rubberduck-vba / Rubberduck

Every programmer needs a rubberduck. COM add-in for the VBA & VB6 IDE (VBE).
https://rubberduckvba.com
GNU General Public License v3.0
1.92k stars 302 forks source link

disable adding @Ignore comments into code #6225

Open noambrand opened 1 month ago

noambrand commented 1 month ago

What Under options add "disable adding comments into code"

Why I do not want Rubberduck comments in my code, but do want the ability to ignore some inspections (not one by one).
That will help make the list of remaining things to check more focused.

Example '@Ignore ImplicitPublicMember '@Ignore UseMeaningfulName '@Ignore UseMeaningfulName, MultipleDeclarations

retailcoder commented 1 month ago

Hi, thanks for the feedback!

I'm not sure I understand the feature being requested here; the way I'm reading it there would be a new option that, if enabled, would disable or remove the "ignore once" quick-fix from the inspection results' "fix" menu?

Rubberduck does not add comments to your code without you telling it to do exactly that, so I'm not sure how disabling Ignore Once would be functionally any different from not opting to apply the IgnoreOnce quickfix for a given inspection/result? What if an annotation was manually added (they're just comments, they can be typed)?

What is the motivation behind the feature? Is it that the code has a bunch of scattered "ignore" annotations everywhere and it would be nice to have a way to quickly find (and remove?) them?

I do not want Rubberduck comments [...], want the ability to ignore inspections (not one by one).

Not sure if this is referring to inspections or inspection results, but to ignore an inspection you can disable it by setting its severity level to DoNotShow (then it won't run at all, so won't issue any results); to ignore the results of an inspection "one by one" would be what the IgnoreOnce quick-fix does, but then IgnoreInModule silences (or should be silencing anyway) all results in that module, but it does this by adding an @IgnoreModule annotation comment in the declarations section. But that would be a no-go if we don't want any annotation comments anywhere.

Basically I'm all ears, but I'd need to better understand the use case.

There is no way to just "dismiss" a result and then to remember it was dismissed, because each run is entirely independent of any preceding run, and results are associated to parse tree nodes, which are invalidated whenever the code changes in any way: the way Rubberduck can remember to not issue a particular inspection result is by annotating the code being inspected so that an inspection knows to skip a node that's associated with an IgnoreAnnotation.

noambrand commented 1 month ago

Hi,

Thanks for the clarification. What I’m asking for is the ability to apply the "Fix All Occurrences" without adding any IgnoreOnce or IgnoreInModule comments to my code.

Currently, when I use the "Fix All Occurrences" option, it adds comments starting with "Ignore" into my VBA code. However, I do not want Rubberduck to modify my code with these comments.

Instead, I want to be able to ignore all occurrences across the entire project or module without inserting any comments. I’m not referring to disabling specific inspections with DoNotShow, but rather applying fixes or ignoring results without adding comments or annotations in the code.

Is there a way to achieve this or could this feature be considered for future updates?

Thanks!

בתאריך יום א׳, 22 בספט׳ 2024, 19:41, מאת Mathieu Guindon ‏< @.***>:

Hi, thanks for the feedback!

I'm not sure I understand the feature being requested here; the way I'm reading it there would be a new option that, if enabled, would disable or remove the "ignore once" quick-fix from the inspection results' "fix" menu?

Rubberduck does not add comments to your code without you telling it to do exactly that, so I'm not sure how disabling Ignore Once would be functionally any different from not opting to apply the IgnoreOnce quickfix for a given inspection/result? What if an annotation was manually added (they're just comments, they can be typed)?

What is the motivation behind the feature? Is it that the code has a bunch of scattered "ignore" annotations everywhere and it would be nice to have a way to quickly find (and remove?) them?

I do not want Rubberduck comments [...], want the ability to ignore inspections (not one by one).

Not sure if this is referring to inspections or inspection results, but to ignore an inspection you can disable it by setting its severity level to DoNotShow (then it won't run at all, so won't issue any results); to ignore the results of an inspection "one by one" would be what the IgnoreOnce quick-fix does, but then IgnoreInModule silences (or should be silencing anyway) all results in that module, but it can do this by adding an @IgnoreModule annotation comment in the declarations section. But that would be a no-go if we don't want any annotation comments anywhere.

Basically I'm all ears, but I'd need to better understand the use case.

— Reply to this email directly, view it on GitHub https://github.com/rubberduck-vba/Rubberduck/issues/6225#issuecomment-2366865821, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOQHOGPDT6XRLANBQ2DWOH3ZX3XMNAVCNFSM6AAAAABOUP7E3OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRWHA3DKOBSGE . You are receiving this because you authored the thread.Message ID: @.***>

retailcoder commented 1 month ago

Instead, I want to be able to ignore all occurrences across the entire project or module without inserting any comments. I’m not referring to disabling specific inspections with DoNotShow, but rather applying fixes or ignoring results without adding comments or annotations in the code.

We could add a "dismiss result" feature that removes a result (or a bunch!) from the list without annotating the code. It would clean up the toolwindow, ...but only until the inspections run again... which I don't think would be useful.

The problem is that something somewhere has to remember that a given result is to be ignored, and inspection results don't do that because they are tied to a single parse run; as soon as the code changes in any way, they're flushed and regenerated from the new/updated code and parse trees. The thing that associates a result with its location in the source code is the parser tokens, and they are not mutable so there isn't really any way to keep inspection results around and re-link them to updated token locations; if a tree is invalidated, so are all its inspection results. Without an annotation comment to tell an inspection to not issue a result at a given location, there is no way (none that would be reliable anyway) to say "there was a result here before and it was dismissed so don't reissue it now". An inspection doesn't know anything about its previous-run results.

The 2.x approach with inspections dates all the way back to v1, where all inspections would traverse and inspect all parse trees after a successful run, aggregating results in one giant bag. v3.0 will change this by basically eliminating inspections, replacing them with language server diagnostics. Sounds similar, but the LSP paradigm makes everything very different, and diagnostics will be fully decoupled from parse trees and their tokens, making it much simpler to correctly manage. It would not be something possible with 2.x, at least not without fundamentally changing how things work.

noambrand commented 1 month ago

Thanks for your response. I have another suggestion that might address my concern more effectively:

What about adding an option that allows users to delete all the "Ignore" comments from the code in one go? This option would come with a notification, informing the user that doing so will make all previously ignored inspections reappear in the UI.

Currently, finding the option to set an inspection to DoNotShow by name under the settings is not straightforward. This new feature would simplify things by providing a quick and easy way to manage ignored inspections without needing to go through the settings manually.

I hope this suggestion can be considered for future updates.

Thanks!

בתאריך יום א׳, 22 בספט׳ 2024, 21:07, מאת Mathieu Guindon ‏< @.***>:

Instead, I want to be able to ignore all occurrences across the entire project or module without inserting any comments. I’m not referring to disabling specific inspections with DoNotShow, but rather applying fixes or ignoring results without adding comments or annotations in the code.

We could add a "dismiss result" feature that removes a result (or a bunch!) from the list without annotating the code. It would clean up the toolwindow, ...but only until the inspections run again... which I don't think would be useful.

The problem is that something somewhere has to remember that a given result is to be ignored, and inspection results don't do that because they are tied to a single parse run; as soon as the code changes in any way, they're flushed and regenerated from the new/updated code and parse trees. The thing that associates a result with its location in the source code is the parser tokens, and they are not mutable so there isn't really any way to keep inspection results around and re-link them to updated token locations; if a tree is invalidated, so are all its inspection results. Without an annotation comment to tell an inspection to not issue a result at a given location, there is no way (none that would be reliable anyway) to say "there was a result here before and it was dismissed so don't reissue it now". An inspection doesn't know anything about its previous-run results.

The 2.x approach with inspections dates all the way back to v1, where all inspections would traverse and inspect all parse trees after a successful run, aggregating results in one giant bag. v3.0 will change this by basically eliminating inspections, replacing them with language server diagnostics. Sounds similar, but the LSP paradigm makes everything very different, and diagnostics will be fully decoupled from parse trees and their tokens, making it much simpler to correctly manage. It would not be something possible with 2.x, at least not without fundamentally changing how things work.

— Reply to this email directly, view it on GitHub https://github.com/rubberduck-vba/Rubberduck/issues/6225#issuecomment-2366898299, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOQHOGIJGE4JFWM7G4UCMNTZX4BMNAVCNFSM6AAAAABOUP7E3OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRWHA4TQMRZHE . You are receiving this because you authored the thread.Message ID: @.***>