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 301 forks source link

Surfacing Annotations #4790

Closed retailcoder closed 4 years ago

retailcoder commented 5 years ago

Annotations are one of our coolest features (especially since the merging of #4686), and yet unless you read about them in the release notes or on the RD News blog, there's no real way for our users to discover them. Let's fix this.

Code Explorer

In the context menu for the Code Explorer, let's add an "Annotate" parent menu that's enabled (visible?) on module and member nodes; one command per legal annotation, as per the selected node.

Code Pane

In the context menu for the code pane, the "Annotate" parent menu would list module-level commands when the Selection is in the module's declarations section, and member-level commands everywhere else.


AnnotateCommand

Every "annotation" command needs to:

Parameterless annotations could be one implementation of some AnnotateCommandBase abstract class, and each parameterized one would probably need its own implementation, overriding the "prompt for any parameter values" part with a function that gets a model to work with.

Thus:

This seems a rather large feature at a glance, but the only new UI we need is a simple check-list of inspections - other parameterized annotations only need a string value for a single parameter. And with this setup (or similar), we could easily add a command later, with different UI requirements.


Thoughts? Other ideas?

bclothier commented 5 years ago

Excellent ideas. Only one little niggle regarding the listing of annotations in the context menu. I personally would prefer that the list of the annotations be static and be simply enabled/disabled based on the selection. This aids the discoverability and avoid the jack-in-the-box effect which I personally despise in any UI context. By extension the Annotate method should be always visible (though I can see disabling it if it's inapplicable).

comintern commented 5 years ago

@bclothier I'm not sure I like the idea of adding more static members to the context menu in the CE. It's already kind of a cluster-duck in there.

This would be much better to do dynamically from a UX standpoint, and isn't horribly difficult. I'd probably want to see this implemented in conjunction with #1479.

comintern commented 5 years ago

Ref #2964, #3745, #4227, #4691. All of these (and likely some that I missed) would be easier to implement with the proposed API set.

mansellan commented 5 years ago

Sorry @bclothier, the downvote is from me. Whilst I agree that consistency is important, I don't think it's worth the price of having greyed-out entries for items that make no sense on the selected node.

Having said that, I do agree the ordering (for any that apply to multiple contexts) should be kept consistent.

SmileyFtW commented 5 years ago

That would be AWESOME!

MDoerner commented 5 years ago

I also like the idea.

The actual rewriting in the code panes will be easy to handle via the IAnnotationUpdater.

One correction though: there are multiple commands taking multiple strings as input, e.g. ModuleAttributeAnnotation. In that case, even the number of arguments is not fixed. (BTW, you can have arbitrarily many of these on the same module.)

SmileyFtW commented 5 years ago

So in the interim where can one find a list of valid annotations and maybe some explanation of how a particular annotation is intended to be used. Some examples of how one is used in an unexpected but useful way would also be nice.

Vogel612 commented 5 years ago

@SmileyFtW The project wiki currently has a page on the VB_Attribute controlling annotations as well as a dedicated page for the Folder Annotation

SmileyFtW commented 5 years ago

Thanks! I had seen that, but I am pretty sure there is a lot more hiding under the hood.

From: Clemens Lieb notifications@github.com Sent: Sunday, May 12, 2019 10:20 AM To: rubberduck-vba/Rubberduck Rubberduck@noreply.github.com Cc: David dgmsmiles@gmail.com; Mention mention@noreply.github.com Subject: Re: [rubberduck-vba/Rubberduck] Surfacing Annotations (#4790)

@SmileyFtW https://github.com/SmileyFtW The project wiki currently has a page on the VB_Attribute controlling annotations https://github.com/rubberduck-vba/Rubberduck/wiki/VB_Attribute-Annotations as well as a dedicated page for the Folder Annotation https://github.com/rubberduck-vba/Rubberduck/wiki/Using-@Folder-Annotations

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rubberduck-vba/Rubberduck/issues/4790#issuecomment-491604646 , or mute the thread https://github.com/notifications/unsubscribe-auth/AKCD6F42N24I5MGQHPS4K4LPVAYR7ANCNFSM4GWN54VA . https://github.com/notifications/beacon/AKCD6FZWYJOG4L435TCFWBLPVAYR7A5CNFSM4GWN54VKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVGUVJQ.gif

MDoerner commented 4 years ago

I would like to propose a further angle on surfacing the annotations. How about adding and extracting XML-docs for the annotation classes the same way we do for inspections?

MDoerner commented 4 years ago

I think we should split this issue into two, one for the CE and one for code panes.

I could pick up the CE one. It is rather easy there to have different contents in the Annotate submenu for members and modules. Moreover, we can make the menu dynamic like the one for templates.

In the code panes, a dynamic menu would be a lot more painful, since it requires dynamically changing the VBE's COM menus.

Moreover, I would advise against having multiple annotate commands, we can simply wrap the AnnotationUpdater into one command that takes both the target and an Annotation as arguments (in a tuple). Then, we pop up a dialog to enter the annotation arguments, provided the annotation has any. I would start with a simple dialog taking a comma-separated list. Later, we could still make it more sophisticated.

I would set this up as a refactoring action with an IRefactoringModel and an IRefactoringUserInteraction. Then, we can use the dialog infrastructure for the refactorings and reuse the dialog for the the code pane command, which will take different parameters.

testingoutgith1 commented 4 years ago

Could the COM part be simplified by just having an AddAnnotationCommandMenuItem with ShowAnnotationOptionsCommand for bringing up dialog of available annotations instead of using a pop-up submenu? That way only need to change one COM control.

MDoerner commented 4 years ago

That was my idea as well. However, in that case, the UI will be entirely different, which is the reason I would like to split the issue.

testingoutgith1 commented 4 years ago

Is a popup submenu better than a dialog box?

MDoerner commented 4 years ago

I think I have an idea how I want to implement this. However, I would like to explicitly limit the functionality to components, methods and module level variables. This will exclude the Ignore annotation, which can be applied to identifier references and general contexts as well.

This restriction allows to find a unique target for the command without the need to guess whether the context at hand, the referenced declaration or the containing method are the intended target. The priority will be the same we employ elsewhere: referenced/selected declaration - > containing method - > containing component.