Keyword Indentation Error

[heart-of-code]

Hello,

I am using vanilla brightscript (not brighterscript) and came across an indentation issue while creating some functions for an app.

The terms enum and class trigger indentation that applies to the remainder of the file after the keyword, even though these terms don't mean anything and aren't reserved in brightscript.

class:

enum:

The indentation affects the entire file..

Could we distinguish formatting needs for Brightscript vs Brighterscript?

[TwitchBronBron]

Currently our formatter has some fairly very primitive handling, it just looks for keywords and formats based on that. This is definitely a bug, and it should be fixed. It shouldn't be indenting that in brightscript OR brighterscript.

Thanks for the issue. No guarantees, but I'll try and fit in a fix sometime in the next month or so.

[TwitchBronBron]

Let's make sure that all of these keywords can be properly used as functions and not cause indentation issues:

https://github.com/rokucommunity/brighterscript/blob/ec0341b6e54343d4efcb928b8630b0b3c326270d/src/lexer/TokenKind.ts#L149C3-L165C27

[philanderson888]

I can replicate the issue by

npm install brighterscript-formatter -g
bsfmt main.brs

I see the following output when I start off with the file like this

the output is this

so yes it's affecting the output very clearly

for sure!

I have rebuilt the github repositories required from scratch again using

https://rokucommunity.github.io/vscode-brightscript-language/contributing.html#the-easy-way

so thanks @TwitchBronBron for those instructions again.

I will get the vscode extension running again in developer mode (it's been a while!) then have a look to debug the issue.

[philanderson888]

I have generated a work in progress pull request

#96

code is super messy as I was just playing around with all variables to see what works.

fix does appear to hold though as I am still getting all existing tests passing

and all of the following appears to be indented correctly after formatting

sub init()
    m.top.setFocus(true)
    m.myLabel = m.top.findNode("myLabel")
    m.myLabel.font.size = 80
    m.myLabel.color = "0xCCCCCC"
    m.myLabel.text = "Hello World 01!"
end sub

function class()
end function

function enum()
end function

function sub2(args)
    print "sub"
    print args
end function

function for2(args)
    print "for"
    print args
end function

function function2(args)
    print "function"
    print args
end function

function if2(args)
    print "if"
    print args
end function

function while2(args)
    print "while"
    print args
end function

function forEach(args)
    print "forEach"
    print args
end function

function leftCurlyBrace(args)
    print "leftCurlyBrace"
    print args
end function

function leftSquareBracket(args)
    print "leftSquareBracket"
    print args
end function

function questionLeftSquare(args)
    print "questionLeftSquare"
    print args
end function

function hashIf(args)
    print "hashIf"
    print args
end function

function interface(args)
    print "interface"
    print args
end function

function namespace(args)
    print "namespace"
    print args
end function

function try(args)
    print "try"
    print args
end function

function testFunction()
    print "testFunction"
end function

this is after my new code is in place eg

I see that the syntax

function enum() is highlighted red by the compiler

should not function class() also be so?

that would be a better fix - prevent the compiler accepting such nonsensical function names - they should be banned like

function sub()
function function()
function class()
function enum() 

i guess the latter two are brighterscript concepts so are not naturally blocked in brightscript?

anyway if you can check out my super messy code just for initial comments that would be great - then i will proceed to generate tests and neat fix, thanks.

Super Summary

1. I have generated a very messy fix - are you able to have a super quick look and just give some super instant feedback please? Once you feed back I will generate neat fix and tests to go with it.

2. My question is - why are we fixing this at all ? These should be caught as compiler errors in the first place. I never heard of anything so nonsensical as function class() being allowed or function enum() ... I guess they are permitted under natural Brightscript as the words 'class' and 'enum' are not in that language so are not reserved ... interesting conundrum!

[philanderson888]

@chrisdp @markwpearce @TwitchBronBron @heart-of-code

[addison-adler]

@philanderson888 re:

My question is - why are we fixing this at all ? These should be caught as compiler errors in the first place. I never heard of anything so nonsensical as function class() being allowed or function enum() ... I guess they are permitted under natural Brightscript as the words 'class' and 'enum' are not in that language so are not reserved ... interesting conundrum!

This was a problem for me when I was a not-brighterscript user, who was using the formatter/language features from the VSCode Brightscript Language Extension and I was implementing my own version of classes. I also had my own enums function for constructing enums since the Roku OS doesn't include them either!

big world out there, not everybody trying to get language/syntax support is using brighterscript 😄

and I'd figure they aren't compile errors because Roku OS/Brightscript doesn't have any concept of a "class" or a "enum," which is why we end up polyfilling them

[philanderson888]

Hello @addison-adler thank you so much for explaining it in that way - that makes total and complete and utter sense!!!! Myself humbled completely for my statement!

But now I see a clear and compelling reason to fix this, which really helps!

Thank you

:)

[addison-adler]

absolutely, context is always helpful/important to have!

[philanderson888]

OK @TwitchBronBron looks like

• code is working
• tests are passing

cc

@chrisdp @markwpearce @heart-of-code @addison-adler

#96

[addison-adler]

@philanderson888 I have another example of why this fix is helpful 😄

I don't know if interface got into the PR, but either way I appreciate that you took the time to do the work 🙌🙌🙌

I tend to create a lot of namespaces like this because it makes the code read more plainly / like English, and namespaces don't have the runtime cost that Classes do
(note: this is not the only function I'm adding into this namespace, otherwise I'd just write function HasInterface)