Skip to content

Conversation

@Moyf
Copy link
Contributor

@Moyf Moyf commented Mar 7, 2025

To avoid the "0 character added, 0 character removed" notice

@pjkaufman
Copy link
Collaborator

Hey @Moyf , thanks for creating the PR. I am a little torn on whether to merge it or not. I may have the wrong use case here for why I would keep the notice since I use it to let me know if things were linted and nothing happened. But maybe that is just something I use it for that most users do not want.

I am thinking about this and hope to come to a decision on it.

@Moyf
Copy link
Contributor Author

Moyf commented May 11, 2025

Hey @Moyf , thanks for creating the PR. I am a little torn on whether to merge it or not. I may have the wrong use case here for why I would keep the notice since I use it to let me know if things were linted and nothing happened. But maybe that is just something I use it for that most users do not want.

I am thinking about this and hope to come to a decision on it.

Oh hey! I think this PR is actually related to how I personally use Linter—I bind "Linter formatting" and "saving" together (and assign it to Ctrl+S), so my trigger frequency might be significantly higher than that of a typical user (who manually performs Lint operations).
For cases where Lint operations are performed manually, yes, even "0 changes" is necessary feedback to let them know that their command has indeed been executed.

Perhaps (if you don't mind adding further settings) "Do not show Notice when modifications are empty" could be made an optional setting (defaulting to False)?

Also, strangely, I seem to recall that there was a setting toggle for "whether to show Notice" itself, but I just tried to find it and couldn't... Did I misremember? 🤔

@pjkaufman
Copy link
Collaborator

@Moyf , that setting addition would be fine with me. Just note that the setting would go under the general tab and would need to only show if the Linter has enabled the notice for the changes made. There are some similar examples in that file here:

let displayLintOnActiveFileChangeSetting: ToggleSetting = null;
tempDiv = this.contentEl.createDiv();
const lintOnActiveFileChangeSetting = new ToggleSetting(tempDiv, 'tabs.general.lint-on-file-change.name', 'tabs.general.lint-on-file-change.description', 'lintOnFileChange', this.plugin, (value: boolean) => {
if (value) {
displayLintOnActiveFileChangeSetting.unhide();
} else {
displayLintOnActiveFileChangeSetting.hide();
}
});
this.addSettingSearchInfoForGeneralSettings(lintOnActiveFileChangeSetting);
tempDiv = this.contentEl.createDiv();
displayLintOnActiveFileChangeSetting = new ToggleSetting(tempDiv, 'tabs.general.display-lint-on-file-change-message.name', 'tabs.general.display-lint-on-file-change-message.description', 'displayLintOnFileChangeNotice', this.plugin);
this.addSettingSearchInfoForGeneralSettings(displayLintOnActiveFileChangeSetting);
if (!lintOnActiveFileChangeSetting.getBoolean()) {
displayLintOnActiveFileChangeSetting.hide();
}

@Moyf
Copy link
Contributor Author

Moyf commented May 12, 2025

@Moyf , that setting addition would be fine with me. Just note that the setting would go under the general tab and would need to only show if the Linter has enabled the notice for the changes made. There are some similar examples in that file here:

I noticed that currently the Display Lint on File Change Message only appears when the Lint on Focused File Change option is enabled.
image

However, regardless of whether I enable or disable this option, the notice always appears when I execute the lint current file command.
Can we conclude that there is currently no global "Notice switch" for all changes, but only a switch specifically for Focused File Change? Should the new option also be based on the existing one?

Personally, I hope it can affect the global notice - hiding empty messages regardless of how the lint process is triggered.

@pjkaufman
Copy link
Collaborator

I believe there are two notices. There is one for lint on file change and there is one for lint on save. But then again I could be wrong and need to double check that logic.

@Moyf
Copy link
Contributor Author

Moyf commented May 13, 2025

I believe there are two notices. There is one for lint on file change and there is one for lint on save. But then again I could be wrong and need to double check that logic.

Yes, you're right, there are two options for notice.
image

What confused me before is that even when I turn off the Lint on save option (I use the Commander plugin to manually bind Save, Lint and some of my scripts together), the option still works.

In other words, that option is not only applied to the Lint on save option; even when I manually use Lint the current file command, it still follows this setting to show notice.
So I think it shouldn't only display when the Lint on save option is enabled. I wonder if my understanding is correct? 🤔

@pjkaufman
Copy link
Collaborator

Yes, the setting should actually not be hidden by Lint on Save. I incorrectly added the hiding logic.

As for how I think that the new setting would work, I think it would show if either display message on lint or display lint on file change message is enabled. That way it can be a global setting on the notices without needing multiple versions of the settings.

@Moyf
Copy link
Contributor Author

Moyf commented May 13, 2025

Yes, the setting should actually not be hidden by Lint on Save. I incorrectly added the hiding logic.

As for how I think that the new setting would work, I think it would show if either display message on lint or display lint on file change message is enabled. That way it can be a global setting on the notices without needing multiple versions of the settings.

Yes, I agree with you 🫡
When I got time, I'll add the new changes to this PR : )

@Moyf
Copy link
Contributor Author

Moyf commented Sep 2, 2025

Yes, the setting should actually not be hidden by Lint on Save. I incorrectly added the hiding logic.

As for how I think that the new setting would work, I think it would show if either display message on lint or display lint on file change message is enabled. That way it can be a global setting on the notices without needing multiple versions of the settings.

@pjkaufman Hi, long time no see! I've finally completed the version of this PR with settings, opened at #1378.

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.

2 participants