Skip to content
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

Add support for 'XML' comments to code explorer #818

Open
AndrewM- opened this Issue Oct 8, 2015 · 37 comments

Comments

Projects
None yet
10 participants
@AndrewM-
Copy link

AndrewM- commented Oct 8, 2015

In applications with hundreds of procedures, it gets hard to remember exactly what they do and how they relate to each other. To make my life easier, I have been putting C# style 'XML' comments in my procedures. I wonder if these could be shown as tool tips in code explorer as in the attached mock-up. This would make it far easier to find existing functions suitable for purpose.

xml_comments

I currently generate an API based on these comments that is displayed in a web browser. The API is generated with VBA code. A screen clip is shown below.

api

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Oct 8, 2015

Consider this on the roadmap, I love the idea!

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Oct 8, 2015

How about supporting actual XML comments? I mean, the .net XML comments are well documented, we could very well have an implementation for the VBE, and we could very well export an actual .xml file that could be processed by the exact same tools that generate .net documentation (e.g. SandCastle)... and the <summary> tag could be used in the code explorer as tooltips, but also as a pop-up window in the code pane - you could right-click a method (declaration and usage), select "about this member" from the context menu, and RD could display a window that not only displays the documentation, but also lets you edit it... and the summary could also be used as a description attribute once we get the attribute support up, and then it would show up in the object browser (F2)...

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Oct 8, 2015

Now I'm really starting to wish we could hijack IntelliSense...

@Ethan-Bierlein

This comment has been minimized.

Copy link

Ethan-Bierlein commented Oct 8, 2015

Nice idea! I feel like using '/// for XML comments is a little tacky though. Maybe ''' would work? Just an idea..

@Hosch250

This comment has been minimized.

Copy link
Member

Hosch250 commented Oct 8, 2015

''' is what VB.NET uses.

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Oct 8, 2015

@Hosch250 So, it could look like this then:

'''<summary>Calls functions that create photo hashes for each photo that does not yet have one.</summary>
'''<param name="s">A description for parameter <c>s</c>.</param>
'''<param name="VSN">A description for parameter <c>VSN</c>.</param>
'''<returns>Returns a <see cref="FunctReturn"/> object encapsulating the result.</returns>
Public Function ReadDrivePhotoHashes(s As TypeSQL, VSN As String) As FunctReturn
    '...
End Function
@AndrewM-

This comment has been minimized.

Copy link
Author

AndrewM- commented Oct 8, 2015

I would not use ''' as they do in VB.net because ms-access has a comment block tool on the Edit Toolbar in the VBE. If you have commented out a line, then commented out the rest of the block with the comment block tool, you are already have ''. Sometimes I globally replace On Error with 'On Error and over time this builds up to '''On Error. This is probably bad practice on my part but VBA is cowboy land. VB.net also seems to be not that popular (dumped from #Develop) and I would prefer to use the practices of C# if that is possible as I see C# as the upgrade path not VB.net.

I only found the comment block tool recently as the Edit toolbar is hidden by default in the VBE and there it is below next to the hand.

commentblock

@AndrewM-

This comment has been minimized.

Copy link
Author

AndrewM- commented Oct 8, 2015

I am happy to use XML as per visual studio but there should be a way of handling comments that are just plain text as well. There you go, I just pressed the close button to find out what it does.

@AndrewM- AndrewM- closed this Oct 8, 2015

@AndrewM- AndrewM- reopened this Oct 8, 2015

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Oct 8, 2015

I agree that C#'s syntax looks better than VB.NET's, but C# doesn't use a single-quote to denote a comment, '/// feels somewhat like a bastardization; I don't like the idea of mixing up syntaxes, especially since we're already using Java-like "@" annotations.

The argument about the 'On Error building into '''On Error doesn't hold, and wouldn't affect XML comments unless the instruction was sitting between an opening and a closing XML tag... if we exclude picking up standalone/stray comments which I believe needs to be a firm requirement, especially if this feature can serve as an introduction to .net's XML comment syntax.

@AndrewM-

This comment has been minimized.

Copy link
Author

AndrewM- commented Oct 8, 2015

If XML comments must contain actual XML, then there is no need for having a different notation for these comments, I am happy with that.

@rubberduck203

This comment has been minimized.

Copy link
Member

rubberduck203 commented Oct 8, 2015

You're not the only one wishing we could hijack intellisense @retailcoder! This is a great feature request @AndrewM!

Support for XML doc comments is on the roadmap already (somewhere, I'll need to find the issue later and link back to it), but I never considered a tool tip. Most excellent suggestion!

I am going to second that we use the triple apostrophe notation of VB.Net though. It's going to make life much simpler in the long run as far as an implementation goes. I really do bet that we can actually find one more or less pre-built for us in the Roslyn code base.

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Oct 8, 2015

@ckuhn203 ooh it didn't even occur to me that we could use Roslyn for that! Wouldn't that be yet another license nightmare though?

@rubberduck203

This comment has been minimized.

Copy link
Member

rubberduck203 commented Oct 8, 2015

Possibly Probably @retailcoder. Probably...

@Hosch250

This comment has been minimized.

Copy link
Member

Hosch250 commented Oct 8, 2015

@ckuhn203 @retailcoder You bet you can use Roslyn for this (I think you might need C# 6, though?). SyntaxFactory (VisualBasic version), then just create an XML doc comment.

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Oct 8, 2015

@Hosch250 except Roslyn is under an APL license and we can't just grab a random part of its source code and embed it into ours.. and referencing the whole project, even without considering the license restrictions, would be overkill.

@AndrewM-

This comment has been minimized.

Copy link
Author

AndrewM- commented Oct 14, 2015

Rather than ''' for XML comments, we could use REM as that is already part of VBA. In the process of commenting out code for testing I often builds up to '''. Typically this happens when I am adding something to a production database and I can't finish and test the new code before I have use the database to do some work. In ms-access live applications evolve and the development and production environment are often the same. I know that is not best practice but it is the reality for almost every ms-access project that I do. The reason why is that I am a consultant and each client has a completely different problem that needs a database. The don't get the database but the database is necessary for my ability to create are report for the client. Examples include adding up the damage bill from a cyclone for a whole local government area or recording landholder consultation for a major power line project. A typical one project database application is developed in about 3 days and lives for about 1-6 months. It is this type of environment were ms-access has no competitors and this is where the non-professional developers cook up applications that give ms-access a bad name with the IT department. Panel beating rubbish data and rotten processes is what I do. I just hope that Rubberduck will be the master of this untidy environment as well as being an infusion of clean modern development styles.

@Hosch250

This comment has been minimized.

Copy link
Member

Hosch250 commented Oct 14, 2015

I said this in chat a while ago, but I might as well state it here as well. We could not (for more than just licensing reasons!) just insert Roslyn code in RD, but we could potentially use Roslyn to generate an XML comment syntax node, extract the text string, and insert that.

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Oct 14, 2015

@AndrewM- REM would trigger a code inspection result for ObsoleteCommentSyntaxInspection .. all that really matters is that the XML is in comments - could be a single quote for all I care. I think part of the reason for the triple quotes is really just for Visual Studio to pick up and automatically insert the XML boilerplate.

@rubberduck203

This comment has been minimized.

Copy link
Member

rubberduck203 commented Oct 14, 2015

@AndrewM we're not going to use REM as we have an inspection that specifically looks for that obsolete keyword. We're going with the VB.Net triple apostrophe syntax, as that's what most people who use RD would be comfortable with. Many of our users are looking for features that are available in Visual Studio, but not the VBE. We try to stay consistent with Visual Studio wherever possible so we can provide a reasonably consistent development experience for VB devs.

I hate to play the "benevolent dictator", but the implementation of the feature is no longer up for discussion. The matter is decided.

@rubberduck203

This comment has been minimized.

Copy link
Member

rubberduck203 commented Oct 14, 2015

@hosch, can we just add a reference to the NuGet package and leverage Roslyn that way? Is that what you're recommending?

@Hosch250

This comment has been minimized.

Copy link
Member

Hosch250 commented Oct 15, 2015

@ckuhn203 Definitely yes, if it we do not have to use C# 6 for that.

@rubberduck203

This comment has been minimized.

Copy link
Member

rubberduck203 commented Oct 15, 2015

I'm reasonably confident that we can reference the NuGet package in VS 2013, but it does change the compiler to C#6.

@daFreeMan

This comment has been minimized.

Copy link
Contributor

daFreeMan commented Nov 6, 2015

I think @retailcoder almost wet his pants a little bit here! :)

Though I have to agree, I think this is a great idea, and I might just start doing a much better job of documenting my code!

@manfredk

This comment has been minimized.

Copy link

manfredk commented Nov 10, 2015

I started a full time engagement as a vba developer some days ago and I confess that it really would make me hot! hot! hot! to see this implemented. Great idea, guys ! 👍

@rubberduck203

This comment has been minimized.

Copy link
Member

rubberduck203 commented Nov 11, 2015

We've got some things to get done first, but there's obviously a lot of demand for this feature.

@Vogel612

This comment has been minimized.

Copy link
Member

Vogel612 commented Nov 11, 2015

Alternative Syntax idea, just bouncing it off here:

Leverage annotations. Annotations are already in the works for ignoring specific Inspections for different scopes. As I understand the documentation feature will only be useable through rubberduck.
Let's keep it simple and not introduce another type of syntax that the Parser needs to recognize. Instead using something similar-ish to javadocs might be more interesting:

'''
@description This method will calculate the lenght of the longest side of a 
      rectangular triangle using the pythagorean theorem.

      The description continues until the next annotation is encountered or the comment ends
@param a One shorter leg
@param b The other shorter leg
@return The length of the hypotenuse
'''

This also somewhat eliminates the redundancy I see in XML-Comments. It's just an idea, though. Thoughts?

@rubberduck203

This comment has been minimized.

Copy link
Member

rubberduck203 commented Nov 11, 2015

It's a good point. The annotations were originally inspired by the @todo annotation from my Java days. I really wish I could go back in time and implement the TODO markers differently now. I had no idea that it would influence the design so much.

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Nov 11, 2015

Comments aren't handled by the ANTLR parser, we grab them with our own code. While Javadoc-style does look neat, they would have to be prefixed with a single quote since VBA doesn't have a syntax for comment blocks; the result being that we'll need a way to unambiguously recognize these annotations as such, with C# code (because we can't add a XmlComment grammar rule until comments are handled by the ANTLR grammar).

One nice thing with xml comments is that they can be easily written to an actual .xml file, in a format that's well-specified and usable by other tools to generate entire websites (MSDN-style). The above annotation format looks nicer indeed, but is much less flexible, too - we'll only have one shot at getting this right, coming up with a proprietary annotations system to use within Rubberduck will lock the documentation within Rubberduck; by implementing the existing VB.net xml comment syntax, we remain open to export the docs out of RD and make this data usable by 3rd-party tools we're not going to have to implement ourselves.

Let's not reinvent the wheel :-)

@Vogel612

This comment has been minimized.

Copy link
Member

Vogel612 commented Nov 11, 2015

agreed, makes sense 👍

@Hosch250

This comment has been minimized.

Copy link
Member

Hosch250 commented Dec 28, 2015

I'm willing to tackle this for 2.0 if I get the green light.

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Dec 28, 2015

It's a pretty big feature on its own, not sure we should shoot for 2.0 for that one.

@retailcoder retailcoder added this to the v2.1 milestone Dec 28, 2015

@retailcoder retailcoder modified the milestones: Version 3.0, v2.1 Apr 9, 2016

@retailcoder

This comment has been minimized.

Copy link
Member

retailcoder commented Jan 13, 2017

as much as I love this feature, it needs to be deemed out of scope.

@daFreeMan

This comment has been minimized.

Copy link
Contributor

daFreeMan commented Jan 13, 2017

@retailcoder Booooooooo!!!!!!!

@ThunderFrame

This comment has been minimized.

Copy link
Member

ThunderFrame commented Nov 16, 2017

With AvalonEdit coming, this can surely be deemed back in scope, as we'll have intellisense.

Also, rather than trying to coerce '/// or ''' into bastardized comments, why not use our annotation parser (#2851) to implement as @documentation VB attributes of the procedure?

Sub Foo(Bar As String)
    '@description = "This does the Foo with the Bar"
    '@documentation = "Foo is a decoy, it just beeps, don't tell"
    Beep
End Sub

We could utilise VB_Ext_KEY attributes (which, iirc, are module level, but can be keyed), but have the annotation appear at the procedure level, where the key be defined as ProcName_XML_Comment, and the value is stored in .NET XML comment syntax.

That keeps our annotation syntax and reuses our annotation parser, and allows for an inspection for missing XML comments. And the comments can be parsed by Roslyn or whatever else.

@zspitz

This comment has been minimized.

Copy link

zspitz commented Jul 16, 2018

Extending this, how about blocks of Markdown?

Sub Foo(Bar As String)
    '@description = "This does the Foo with the Bar"
    '@documentation = "
    '    ### Remarks
    '
    '    Not with a Bar, but a Beep
    '"
    Beep
End Sub

This assumes the possibility of multi-line annotations.

@ThunderFrame

This comment has been minimized.

Copy link
Member

ThunderFrame commented Jul 16, 2018

@zspitz, I guess it's just an extra Interface implementation for each description text format/renderer. There's a risk that we're jumping the shark by offering too many formats, and given the limitations of the VBA codepane and character-sets, I'd be more inclined to stick with XML as at least special characters can be represented with escaped entities, can be readily transformed, is in keeping with .NET documentation, and building snippets will be easier.

@rubberduck203

This comment has been minimized.

Copy link
Member

rubberduck203 commented Jul 16, 2018

We don’t want to reinvent anything here, assuming this 3 yr old feature request ever gets implemented. VB.Net style doc comments so the existing Sandcastle tooling can be leveraged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.