Skip to content
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

Merged
merged 7 commits into from
Sep 12, 2022

Conversation

mak2002
Copy link
Contributor

@mak2002 mak2002 commented Sep 7, 2022

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

@mak2002 mak2002 changed the title Dev mode fix for default plugin Desktop: Fix missing plugin file error and missingb setting key error in dev mode Sep 7, 2022
@mak2002 mak2002 changed the title Desktop: Fix missing plugin file error and missingb setting key error in dev mode Desktop: Fix missing plugin file error and missing setting key error in dev mode Sep 7, 2022
@mak2002
Copy link
Contributor Author

mak2002 commented Sep 7, 2022

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;
Copy link
Owner

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.

Copy link
Contributor Author

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;
Copy link
Owner

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.

Copy link
Contributor Author

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.');
Copy link
Owner

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.

Copy link
Contributor Author

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.');
Copy link
Owner

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

@laurent22 laurent22 merged commit 66c9ee0 into laurent22:dev Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants