Desktop: Resolves #6143: Show installed plugins in Help - About Joplin #7711

julien-me · 2023-02-03T12:49:29Z

Resolves #6143

This PR lists in Help > About Joplin every installed plugin. If more than 20 plugins are installed then only the first 20 plugins are shown followed by .... However when clicking on Copy the full list is always copied.

Here's some screenshots when there are 0, 1, 5, 21 plugins installed.

github-actions · 2023-02-03T12:49:42Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

julien-me · 2023-02-03T12:51:01Z

I have read the CLA Document and I hereby sign the CLA

tessus

Nicely done. Test cases, the fact that ellipsis are shown when more than 20 plugins are enabled, it shows that you did your homework.

Thanks for this PR.

laurent22

Looks very good, thank you!

In general when modifying something under /lib or any shared package, you should also check how it's used across platforms. In that particular case, this function is also used on the CLI app (joplin version command), so could you check please that it's still working?

packages/lib/versionInfo.ts

julien-me · 2023-02-06T08:16:01Z

In general when modifying something under /lib or any shared package, you should also check how it's used across platforms. In that particular case, this function is also used on the CLI app (joplin version command), so could you check please that it's still working?

They are not listed.

I started app-cli then used the command :version.

I can see that app-desktop writes to ~/.config/joplindev-desktop on my computer and app-cli writes to ~/.config/joplindev. I guess that's why plugins installed via the desktop application don't appear in the CLI application.

Is this expected? Or is there a way to install plugins from the CLI application?

laurent22 · 2023-02-06T10:36:18Z

They are not listed.

Sorry what do you mean by this? What is not listed?

And no, plugins are not supported on the CLI app at the moment

Edit: Ok I guess you mean plugins are not listed and that's normal. But does the command work at all? I'm wondering because versionInfo now has a dependency to the plugin service, but plugins are not supported on CLI. (it's half supported because we use it for testing but that's it)

julien-me · 2023-02-06T11:46:28Z

They are not listed.

Sorry what do you mean by this? What is not listed?

And no, plugins are not supported on the CLI app at the moment

Edit: Ok I guess you mean plugins are not listed and that's normal. But does the command work at all? I'm wondering because versionInfo now has a dependency to the plugin service, but plugins are not supported on CLI. (it's half supported because we use it for testing but that's it)

I understand now. Yes it works.

Here is a screenshot:

laurent22 · 2023-02-06T12:01:37Z

Hmm, ok it works by chance then because the plugin service is not setup on CLI, yet it's used in your updated function. Instead of having versionInfo.ts loads PluginService directly, would it be possible to pass the plugins as a dependency to versionInfo() (in the same way the packageInfo is currently passed)? And on CLI you'd pass an empty array for instance.

In the context where versionInfo() is called there's state.pluginService.plugins. Does it already contain the necessary info, and if so can you use this? I think it may have been attempted before and it didn't work, so maybe you could add whatever extra info is needed to this array?

julien-me · 2023-02-06T14:46:05Z

state.pluginService.plugins doesn't seem to contain what we need. But I remember a previous PR attempt that passed PluginService.instance().plugins to versionInfo().

I think that would work.

laurent22 · 2023-02-06T14:52:31Z

But I remember a previous PR attempt that passed PluginService.instance().plugins to versionInfo().

Ok let's do that. That way we can pass an empty array on CLI

julien-me · 2023-02-07T04:37:14Z

I updated the code to pass a Plugins object to versionInfo().

laurent22 · 2023-02-07T11:36:27Z

There's a TypeScript error in ErrorBoundary.tsx

julien-me · 2023-02-07T12:53:33Z

There's a TypeScript error in ErrorBoundary.tsx

Fixed.

laurent22 · 2023-02-07T14:07:57Z

Thanks! For info we removed the TypeScript build from the precommit hook because it's too slow. So it means before committing you need to check the output of yarn run watch to see if there's any error in there

laurent22 · 2023-02-08T14:15:59Z

That's great, thanks @julien-me for implementing this!

laurent22 · 2023-02-16T10:25:16Z

@julien-me, since this change the tests now print the version information. Please could you remove these console statements?

laurent22 · 2023-02-16T10:28:24Z

packages/lib/versionInfo.test.ts

+
+jest.mock('./registry');
+
+const info = jest.spyOn(console, 'info').mockImplementation(() => {});


In fact I'm not sure why this is needed? In general we don't want code to print to the console, as that should only be used for debugging

I added that to remove console logs during the tests. There was no test for versionInfo() until now but as soon as I added tests, the statement in versionInfo() was polluting the test output.

That makes sense, but I guess now it can be removed? Or is versionInfo() still printing something?

Yes it can be removed. versionInfo() doesn't print anything anymore.

Ok great, if you could remove it then and create a pull request with it that would be perfect

laurent22 · 2023-02-16T10:59:53Z

packages/lib/versionInfo.ts

@@ -32,9 +68,11 @@ export default function versionInfo(packageInfo: any) {
 console.info(gitInfo);


I have now removed this statement as part of the "no-console" rule that was just added. I assume it was safe to remove?

I do not know the original intent of that line but it seems to me that it is safe to remove.

Desktop: Show installed plugins in Help - About Joplin (laurent22#6143)

0a9fe90

github-actions bot added a commit that referenced this pull request Feb 3, 2023

@julien-me has signed the CLA from Pull Request #7711

2092110

tessus added the desktop All desktop platforms label Feb 3, 2023

tessus approved these changes Feb 3, 2023

View reviewed changes

laurent22 reviewed Feb 5, 2023

View reviewed changes

packages/lib/versionInfo.ts Outdated Show resolved Hide resolved

laurent22 and others added 3 commits February 5, 2023 11:33

Merge branch 'dev' into show-plugins

8323b2b

Merge branch 'dev' into show-plugins

9c78b5a

Rely on type inference

68e3607

julien-me added 2 commits February 7, 2023 11:52

Pass plugins to versionInfo()

4b13307

Remove unnecessary check

65900e3

Update call to versionInfo() in ErrorBoundary

b895597

laurent22 merged commit 631c41a into laurent22:dev Feb 8, 2023

github-actions bot locked and limited conversation to collaborators Feb 8, 2023

laurent22 reviewed Feb 16, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Desktop: Resolves #6143: Show installed plugins in Help - About Joplin #7711

Desktop: Resolves #6143: Show installed plugins in Help - About Joplin #7711

julien-me commented Feb 3, 2023

github-actions bot commented Feb 3, 2023 •

edited

Loading

julien-me commented Feb 3, 2023

tessus left a comment

laurent22 left a comment

julien-me commented Feb 6, 2023

laurent22 commented Feb 6, 2023 •

edited

Loading

julien-me commented Feb 6, 2023

laurent22 commented Feb 6, 2023

julien-me commented Feb 6, 2023

laurent22 commented Feb 6, 2023

julien-me commented Feb 7, 2023

laurent22 commented Feb 7, 2023

julien-me commented Feb 7, 2023

laurent22 commented Feb 7, 2023

laurent22 commented Feb 8, 2023

laurent22 commented Feb 16, 2023

laurent22 Feb 16, 2023

julien-me Feb 16, 2023

laurent22 Feb 16, 2023

julien-me Feb 16, 2023

laurent22 Feb 16, 2023

laurent22 Feb 16, 2023

julien-me Feb 16, 2023


		jest.mock('./registry');

		const info = jest.spyOn(console, 'info').mockImplementation(() => {});

		@@ -32,9 +68,11 @@ export default function versionInfo(packageInfo: any) {
		console.info(gitInfo);

Desktop: Resolves #6143: Show installed plugins in Help - About Joplin #7711

Desktop: Resolves #6143: Show installed plugins in Help - About Joplin #7711

Conversation

julien-me commented Feb 3, 2023

github-actions bot commented Feb 3, 2023 • edited Loading

julien-me commented Feb 3, 2023

tessus left a comment

Choose a reason for hiding this comment

laurent22 left a comment

Choose a reason for hiding this comment

julien-me commented Feb 6, 2023

laurent22 commented Feb 6, 2023 • edited Loading

julien-me commented Feb 6, 2023

laurent22 commented Feb 6, 2023

julien-me commented Feb 6, 2023

laurent22 commented Feb 6, 2023

julien-me commented Feb 7, 2023

laurent22 commented Feb 7, 2023

julien-me commented Feb 7, 2023

laurent22 commented Feb 7, 2023

laurent22 commented Feb 8, 2023

laurent22 commented Feb 16, 2023

laurent22 Feb 16, 2023

Choose a reason for hiding this comment

julien-me Feb 16, 2023

Choose a reason for hiding this comment

laurent22 Feb 16, 2023

Choose a reason for hiding this comment

julien-me Feb 16, 2023

Choose a reason for hiding this comment

laurent22 Feb 16, 2023

Choose a reason for hiding this comment

laurent22 Feb 16, 2023

Choose a reason for hiding this comment

julien-me Feb 16, 2023

Choose a reason for hiding this comment

github-actions bot commented Feb 3, 2023 •

edited

Loading

laurent22 commented Feb 6, 2023 •

edited

Loading