-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Desktop: Fix missing plugin file error and missing setting key error in dev mode #6827
Desktop: Fix missing plugin file error and missing setting key error in dev mode #6827
Conversation
Also, I think this will be needed in normal mode when there is no default plugin directory as it will also throw same error. In my mind, I was thinking that bundling script will be ran before we update the app with functionality to install default plugins. But after this PR is merged, we won't need to worry about updating the app with functionality to install default plugins without default plugin directory. |
@@ -16,7 +16,10 @@ export function checkPreInstalledDefaultPlugins(defaultPluginsId: string[],plugi | |||
} | |||
|
|||
export async function installDefaultPlugins(service: PluginService, pluginsDir: string, defaultPluginsId: string[], pluginSettings: PluginSettings): Promise<PluginSettings> { | |||
if (!await shim.fsDriver().exists(pluginsDir)) return pluginSettings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was so confused with this code because the variables are so poorly named. Rename "pluginsDir" to "defaultPluginDir". In app.ts too, also rename this "pluginsDir" to defaultPluginDir.
Because we also have a "pluginDir" setting which is the actual plugin directory, so you need to use a clearer name for your directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it. I will take care from next time when naming variables.
const defaultPluginsPaths = await shim.fsDriver().readDirStats(pluginsDir); | ||
if (defaultPluginsPaths.length <= 0) return pluginSettings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again please add a message here logger.info('Could not find any default plugin to install - skipping installation.')
See for example useOnInstallHandler.ts
to see how to setup a logger for your file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
const defaultPluginsPaths = await shim.fsDriver().readDirStats(pluginsDir); | ||
export async function installDefaultPlugins(service: PluginService, defaultPluginsDir: string, defaultPluginsId: string[], pluginSettings: PluginSettings): Promise<PluginSettings> { | ||
if (!await shim.fsDriver().exists(defaultPluginsDir)) { | ||
logger.info('Could not find default plugins\' directory - skipping installation.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the path of directory in the message: Could not find default plugins' directory ${defaultPluginsDir} - skipping installation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
} | ||
const defaultPluginsPaths = await shim.fsDriver().readDirStats(defaultPluginsDir); | ||
if (defaultPluginsPaths.length <= 0) { | ||
logger.info('Default plugins\' directory is empty - skipping installation.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add the path of the directory to the message
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
This PR adds a check to see if the default plugin directory exists or not. Also, before applying default plugin settings, we check if setting key exists in Settings.
This should fix following errors which are currently present:
ENOENT: no such file or directory, scandir '***/src/joplin/packages/app-desktop/build/defaultPlugins'
Setting.ts:1702 Uncaught Error: Unknown key: plugin-io.github.jackgruber.backup.path
AppImage to test this can be found here