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.91k stars 300 forks source link

Indenter and Compiler Directives #5411

Open ChrisBrackett opened 4 years ago

ChrisBrackett commented 4 years ago

Justification Two things:

  1. When a procedure declaration is within a compiler directive, the indenter seems to miss the first indent level (see the example below). This may be a bug.

  2. Whether indented or not, compiler directives can make code difficult to read. I propose using the indenter to set compiler directives apart (see below). This is a feature request.

Description of Indenter Change

  1. For compiler directives, use a large indent and add blank rows above and below to set these lines apart.

  2. When a procedure declaration is within a compiler directive, the compiler should indent the code below Sub and above End Sub.

Pre-indenter Code:

#If VBA7 Then
Public Sub DoSomething(ByVal Count As LongPtr)
#Else
Public Sub DoSomething(ByVal Count As Long)
#End If
MsgBox Count
End Sub

Post-indenter Code: With most indenter settings, the indenter did not indent anything in my example code. Unchecking both "Indent compiler directives" and "Force compiler directives to column 1" resulted in the following. It looks like something in this code is throwing off the indenter resulting in inconsistent indents.

#If VBA7 Then
Public Sub DoSomething(ByVal Count As LongPtr)
    #Else
Public Sub DoSomething(ByVal Count As Long)
    #End If
    MsgBox Count
End Sub

Post-indenter Code Assuming New Feature:

                                        #If VBA7 Then

Public Sub DoSomething(ByVal Count As LongPtr)

                                        #Else

Public Sub DoSomething(ByVal Count As Long)

                                        #End If

    MsgBox Count
End Sub
retailcoder commented 4 years ago

I'd call that a bug ;-)

That said intertwined blocks like this, is why we need to pre-process the code files and pipe the "dead code" tokens into a separate lexer channel, so the parser can process it. It's also something to keep in mind when we make the indenter work off parse trees (as is the plan), ...thinking we should have it work off token streams instead.

daFreeMan commented 4 years ago

Ooof.. That post-indenter suggestion is a bit rough. To my eye, the compiler directives look like comments shoved way over to the right like that.

I agree that something nicer should be done with Compiler Directives. My preference would be something like this:

#If VBA7 Then
    Public Sub DoSomething(ByVal Count As LongPtr)
#Else
    Public Sub DoSomething(ByVal Count As Long)
#End If
        MsgBox Count
    End Sub

Maybe include some options so people can pick their own preferences?