Fix bookmark plugin file monitoring issue

[VictoriousRaptor]

Fixes #2120 and maybe fixes #2138.

Changes

• Don't load bookmarks or monitor bookmark files when plugin is disabled
• Remove LastAccessed filter to avoid unwanted RegisterBookmarkFile() calls

Tests

• Loading bookmark won't create unwanted recursive calls. Previously because LastAccessed filter of the file watcher, an attempt to load bookmarks invokes a second call to load.
• Manual data reload creates file watchers and bookmark changes are correctly monitored.
• Bookmarks are loaded on query when plugin gets enabled later.

[VictoriousRaptor]

We shouldn't call init() when plugin is disabled.

[taooceros]

true, let's add a flag to indicate whether a plugin is initialized

The issue for now is that if user re-enable the plugin, it won't be initialized

[VictoriousRaptor]

Yeah but it's not the point of this PR.

[taooceros]

should we call init every time plugin is reenabled?

[VictoriousRaptor]

should we call init every time plugin is reenabled?

Technically we should but some plugins may need to update. like if we don't dispose file watchers when disabling plugins, creating new ones after re enabling may lead to some issues.

[JohnTheGr8]

I agree with @taooceros that the plugin manager should keep track of what plugins have been initialized or not (with a flag). This allows us to skip initializing disabled plugins on start-up. When a plugin is enabled, we can check the flag to determine if its Init method should be called or not.

This check does not belong in the Init of individual plugins...

[VictoriousRaptor]

I agree with @taooceros that the plugin manager should keep track of what plugins have been initialized or not (with a flag). This allows us to skip initializing disabled plugins on start-up. When a plugin is enabled, we can check the flag to determine if its Init method should be called or not.

This check does not belong in the Init of individual plugins...

Then we need to change all plugins. init method passes context object to plugin, which is necessary for localized plugin names and descriptions in settings window.

[JohnTheGr8]

Then we need to change all plugins.

Why do we?

[VictoriousRaptor]

Then we need to change all plugins.

Why do we?

i18n plugins uses plugincontext.API to retrieve localized plugin names. if we simply stop invoking init method of disabled plugins the plugin context is null and raises errors.

[JohnTheGr8]

i18n plugins uses plugincontext.API to retrieve localized plugin names. if we simply stop invoking init method of disabled plugins the plugin context is null and raises errors.

What I said was that we should invoke their Init method if they get enabled later on...

[VictoriousRaptor]

i18n plugins uses plugincontext.API to retrieve localized plugin names. if we simply stop invoking init method of disabled plugins the plugin context is null and raises errors.

What I said was that we should invoke their Init method if they get enabled later on...

Yeah. Anyway should I remove the check in this PR? It's going off-topic and checking it in Init is not a good idea.

[JohnTheGr8]

Yeah. Anyway should I remove the check in this PR? It's going off-topic and checking it in Init is not a good idea.

This isn't off-topic, you have made a change that introduces a different issue (when the plugin is enabled later, as you noted) 😉

Anyway, I'm okay with keeping this as a temporary solution, but let's add a quick flag inside the bookmarks plugin that tracks if bookmarks have been loaded or not. At the top of Query check this flag and load bookmarks if necessary...

[JohnTheGr8]

@VictoriousRaptor let's remove that delay and get the branch up-to-date with dev

[JohnTheGr8]

Let's get another pair of eyes on this before merging

[github-actions]

@check-spelling-bot Report

🔴 Please review

See the 📂 files view or the 📜action log for details.

Unrecognized words (1)

Firefox

To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the [email protected]:VictoriousRaptor/Flow.Launcher.git repository
on the FirefoxBookmark branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/main/apply.pl' |
perl - 'https://github.com/Flow-Launcher/Flow.Launcher/actions/runs/5407282387/attempts/1'

If the flagged items are 🤯 false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

• well-formed pattern.

  If you can write a pattern that would match it,
  try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.