Closed Burky closed 4 years ago
Thanks for reporting this issue.
We are aware that how the Fix all in... feature works is not optimal. This should change at some point, but do not expect this happen soonish; currently, very few people actually put work into RD.
Regarding your additional points:
@PredeclaredId
annotation, you will get an inspection result for a missing attribute annotation. Using the quick fix to remove the attribute will reset it to its default value.One of the annotations-related inspection has the wrong type - should be under "Rubberduck Opportunities" but ends up buried somewhere under "Code Quality Issues" IIRC. ...I think it's the "missing attribute" one. This should be a trivial fix.
I strongly agree with every single bit of feedback in this issue - thanks for taking the time to write it up!
My suggestion on how to make the fix in Fix
button and the context menu should have two levels. The first level is the choice of scope (fix instance, fix in procedure, fix in module, fix in project, (fix in selection, once we get to that)). These would expand sub-menus containing all applicable quick fixes.
I like the two-level context menu approach. It may be worth considering inverting the menu levels to lead the user to choose what to do -> where/scope rather than where/scope -> what to do. Placing what to do as the first choice is how the 'solution' is described above, which leads me to wonder if it may be the more intuitive presentation.
My suggestion on how to make the fix in more intuitive is to remove the static links entirely. Instead, the
Fix
button and the context menu should have two levels. The first level is the choice of scope (fix instance, fix in procedure, fix in module, fix in project, (fix in selection, once we get to that)). These would expand sub-menus containing all applicable quick fixes.
Good Point! There are many approaches one could take. I like yours!
The rule of thumb should be: Any quick-fix should always consider: 1.) If one of the attribute/annotation-pair members is missing, then it is added. 2.) If there are differences between attributes and annotations, then the annotation takes precedence! => A Quick-fix always changes the underlying attribute.
One other "improvement" in UX would be: Use single words/terms (expressions) for attributes and annotations (as I do it here in my writing:-) and never use them together denoting one of them. IMHO, it is not intuitive to read and make the right distinction between: "Adjust attribute annotation" and "Adjust attribute value(s)" when looking at the Code Inspection context menu! In those days, I asked myself: "What the heck is an attribute annotation in contrast to its value? And why there is that plural '(s)'?" I mean, the attribute is the very reverse of the annotation when it comes to how to edit both. Putting those two words together into one single menu option like above must be the source of some confusion. Thus, make it crystal clear: "Adjust annotation" "Adjust attribute" Then everybody knows: "Yeah, the annotations are the remarks I modify in my VBE-editor and the attributes are the hidden tokens, Rubberduck takes care of :-)
Hope, that makes sense!
@retailcoder, @MDoerner 👍 You're right, there is a quick-fix for removing a VB_PredeclaredID attribute. I switched the Code Inspections **Grouping** to By Location and then was able to spot it! Thanks again!
Code Inspections If I get one warning,>>The attribute value(s) for attribute VB_Description ("....") are out of sync with the Description annotation.<< I regularly right-click that inspection and select either >>Adjust attribute annotation<< (1st option), or >>Adjust attribute value(s)<< (2nd option). So far, so good.
Why Normally, I get a lot of above warnings all over the place, because I re-generated (hundreds of) code modules and classes and, as part of that regen, assigned modified descriptions. Now, when I select "Fix all occurrences in the project" and run the fix, Rubberduck replaces all my new annotations (I urgently want to keep) with the old attribute values from the file (that I cannot touch directly)! That's no fix, that's a nightmare!
Ambiguity It is, alt least not for RubberNewbies, not clear, what the fix will be if there is more than one option beneath "Ignore once." For me, after some testing, the first option in the right-click popup is the fix. But I haven't found any documentation about that to prove that.
Quick fixes I'm not suggesting a particular new Quick-Fix, but an overall improvement: Instead of offering static hyperlink-like selectors "Fix all occurrences ... " (which in most cases is ambiguous what exactly the FIX will do ), one should be more precise and alter the hyperlink caption based on the selected issue, like so: "Adjust all attribute annotation in procedure", "Adjust all attribute annotation in module", "Adjust all attribute annotation in the project", a.s.o. And, in addition to that, there also should be the alternative options available, like "Adjust all attribute value(s) in procedure", "Adjust all attribute value(s) in module", "Adjust all attribute value(s) in the project", a.s.o.
Other distracting Oddities There are more (at least to me) distracting oddities, I would like to mention here. Maybe it would be better to create a new topic, but I do not have the time. So, please forgive me :-)
1st) It is possible to multi-select issue-lines in the Code Inspections pane. Nice! But what are they good for? There should be an additional line of options: "Adjust SELECTED attribute annotation", or just "Adjust SELECTION"
2nd) When I was new to RubberDuck, I used the default setup and Window/Pane layout. On my local development system (MacBook Pro coupled with iPadPro using Parallels VM and Duet), the lower Code Inspections Pane was not tall enough to display the "Fix all occurrences..." hyperlinks because they are fixed to the bottom of the lower pane. Thus, I never saw them, until I found it out myself when dragging up the horizontal pane gripper, by change. My suggestion, therefore, is: Put them up above the explanation section. Make the action-links most paramount!
3rd) Until today, I did not find a Quick-Fix to remove a VB_PredeclaredID once I've set it. There should be an Inspection that offers me that option. But maybe I've just overseen it? No, I don't think so.