-
Notifications
You must be signed in to change notification settings - Fork 112
Add a 'Suppress message if no change' feature with settings. #1378
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
Conversation
* feat(settings): add option to hide notice when no change Introduce a new setting noNoticeWhenNoChange to control whether the post-lint notice is shown when no characters are added or removed. - Add noNoticeWhenNoChange to settings data and default values, with comments. - Persist and normalize the setting during loadSettings to ensure a boolean default. - Add a ToggleSetting in the General tab UI and include localization entries (en, zh-cn) for name and description. - Update displayChangedMessage to respect the new setting and skip showing the notice when no chars were added/removed. This reduces noise for users who prefer no feedback when linting makes no effective changes. * feat(locale): add more translations - Translated by AI * refactor: rename variable name Update settings, usage and translations to use a clearer name for the option that suppresses messages when there are no actual changes. - Rename settings property in loadSettings from noNoticeWhenNoChange to suppressMessageWhenNoChange and default it to false. - Update displayChangedMessage to read suppressMessageWhenNoChange and skip message emission when chars added + removed is zero. - Update translation keys and copy in ru, tr, de, es locales from no-notice-when-no-change to suppress-message-when-no-change and adjust wording to match the new name. - Remove a now-duplicate UI setting block in general-tab that created the old toggle for noNoticeWhenNoChange (the UI now uses the renamed setting elsewhere). This clarifies intent (suppress message vs. no notice) and consolidates the implementation to the new, consistent setting name.
Adjust the constructor call and remove stray whitespace in the general tab component to match expected parameter formatting. - Remove an extra blank character left after a conditional block. - Add a missing trailing comma for the plugin argument in the ToggleSetting constructor call to ensure consistent argument separation and avoid potential lint/format issues. These small fixes improve code style consistency and prevent minor syntax/formatting problems in the UI linter component.
pjkaufman
left a comment
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.
Other than passing in the string references as any this looks good to me. If you could remove the as any then I can merge this. Otherwise I can remove the as any if I do not hear back or if that is easier.
| } | ||
|
|
||
| tempDiv = this.contentEl.createDiv(); | ||
| const suppressMessageWhenNoChangeSetting = new ToggleSetting( tempDiv, 'tabs.general.suppress-message-when-no-change.name' as any, 'tabs.general.suppress-message-when-no-change.description' as any, 'suppressMessageWhenNoChange', this.plugin); |
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.
Could you explain the use of as any here? The other areas are not using as any. So this implies that it was either an accidental addition or the string provided does not map properly for the English translation.
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.
Could you explain the use of
as anyhere? The other areas are not using as any. So this implies that it was either an accidental addition or the string provided does not map properly for the English translation.
Oh, sorry for late replying! I'll try to update this commit, but I'm worried about causing issues with multi-branch operations (just like last time...).
If I don't succeed, feel free to remove this as any :D
pjkaufman
left a comment
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.
Looks good. Thanks!
It's the new version with settings of #1286
I'm sorry I forgot about this earlier... Since there were some issues with the original PR (my merge operation caused a mess), I'm opening a new one now.
It now has separate settings, disabled by default: