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

fix: allow overwrite plugin that don't have plugin-priority #8800

Merged
merged 3 commits into from
Dec 5, 2024

Conversation

linonetwo
Copy link
Contributor

@linonetwo linonetwo commented Dec 4, 2024

@pmario 's palette switcher plugin contains default dark/light palette setting, I hope I can overwrite it in my plugin. My plugin is a config-pack plugin for TidGi, to make sure user can upgrade basic UX through plugin, instead of having to upgrade TidGi app.

But palette switcher plugin don't have a plugin-priority field, in current logic it means its priority is Infinity, no one can overwrite it, I think this is incorrect. If an author forget to add plugin-priority field, this should means its priority is 0 (or maybe 1, so it is load later than the core). This make sure any plugin author can patch other author's plugin.

BTW: @pmario could you please add this field?

Copy link

github-actions bot commented Dec 4, 2024

Confirmed: linonetwo has already signed the Contributor License Agreement (see contributing.md)

Copy link

netlify bot commented Dec 4, 2024

Deploy Preview for tiddlywiki-previews ready!

Name Link
🔨 Latest commit 6cd2283
🔍 Latest deploy log https://app.netlify.com/sites/tiddlywiki-previews/deploys/675048439b48b80009637f9c
😎 Deploy Preview https://deploy-preview-8800--tiddlywiki-previews.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pmario
Copy link
Member

pmario commented Dec 4, 2024

I think we can not change that behaviour in a backwards compatible way. Every existing plugin relies on the current behaviour. Sorting by name is the default.

@linonetwo
Copy link
Contributor Author

linonetwo commented Dec 4, 2024

We only have 800+ plugins, why not fix this before we have as much as Obsidian.

Also, fallback priority to 1 won't change much, when they all have 1, they are still compared by name. Have a default value is important, otherwise I have to consider change my name to zenonetwo 🤣 , in China many people prefix their name with AAA so they are show in first place in wechat app.

@pmario
Copy link
Member

pmario commented Dec 4, 2024

@Jermolene -- What do you think?

@Jermolene
Copy link
Member

Hi @linonetwo @pmario the inability to override a plugin that lacks a "plugin-priority" field would appear to be a bug, and I'd be happy to see it fixed. Merging this would be the best way to discover any unintended side effects of the fix.

@pmario
Copy link
Member

pmario commented Dec 5, 2024

@Jermolene -- OK, then this PR should be merged.


@linonetwo

@pmario 's palette switcher plugin contains default dark/light palette setting,

BTW: @pmario could you please add this field?

I think if this PR is merged, it should not be necessary anymore to add a plugin-priority field. Right?

@Jermolene
Copy link
Member

Thanks @linonetwo

@Jermolene Jermolene merged commit 673a0f5 into TiddlyWiki:master Dec 5, 2024
7 checks passed
@linonetwo
Copy link
Contributor Author

@pmario Yes, and thanks for making that plugin, it protects my eyes in the night.

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.

3 participants