rokucommunity / brighterscript

A superset of Roku's BrightScript language
MIT License
152 stars 47 forks source link

additional language support functions: BS_LINENUM(), BS_FILE(), BS_FUNCTION() #13

Closed georgejecook closed 4 years ago

georgejecook commented 5 years ago

wouldn't it be great if we could drop these into the code at any time, like macros, so that they could be resolved at compile time. These are super handy for debugging, and currently require the use of third party tools.

Suggestion

print "unknown error occurred " ; BS_FILE() ; "." BS_FUNCTION() ; BS_LINENUM()

Would compile to print "unknown error occurred pkg:/path/ToFile.brs.LoadStuff(23)"

triwav commented 5 years ago

The downside of this is you’d have to add it at the call site. If you have third party you can inject based off some regex for log statements which is easier (with a higher setup threshold) if this could be expanded that if it’s included as default value of a function it’s actually the line, function, file that called that function

TwitchBronBron commented 5 years ago

BrightScript already supports LINE_NUM (Roku docs reference), so we don't need to add that. LINE_NUM is resolved at runtime as well, which is how it should work, in my opinion. I'm not a fan of injecting static line numbers at compile time, when 3d party preprocessors could potentially inject new lines/remove lines and potentially offset those static line numbers. Is there any benefit to having those line numbers be static?

I'm definitely open to adding constants for file name and function name though. How about following the same naming convention as LINE_NUM?

The one question I have is, what should we show for FUNCTION_NAME for unnamed functions (see my setTimeout example below)?

Syntax

function Main()
    print LINE_NUM
    print FUNCTION_NAME
    print FILE_PATH
    print FILE_NAME

    person = {
        sayHello: function()
            print FUNCTION_NAME
        end function
    }

    setTimeout(function()
        print function_name
    end function, 100)
end function

Output

function Main()
    print LINE_NUM ' no change because this is resolved at runtime by Roku
    print "Main"
    print "pkg:/source/Main.brs"
    print "Main.brs"

    person = {
        sayHello: function()
            print "sayHello"
        end function
    }

    setTimeout(function()
        print "anonymous" 'should this be empty? Say "<anonymous>"? 
    end function, 100)
end function
triwav commented 5 years ago

So the main issue is that the built in won’t give the line it was called from which again is what I want. Perhaps something like this that requires the files not be modified after could go in another step in the pipeline right before zip but after our callback to allow the user to make changes.

georgejecook commented 5 years ago

I agree; I didn't specifically think of this feature for me though - there are (unfortunately) many who don't even consider a proper logging solution, who will benefit from this.

Also agree on your changes @TwitchBronBron I didn't suggest those as I was worried about naming collisions with existing code; but if you are fine with it, I prefer the consistency too

TwitchBronBron commented 5 years ago

@triwav, are you saying you want the line number of the source brightscript file, NOT the line number of the actual runtime line number?

triwav commented 5 years ago

@TwitchBronBron What I actually want is say something like this in fileB at line 20: logWarn(message as String, ..., fileName = FILE_NAME as String, functionName = FUNCTION_NAME as String, lineNumber = LINE_NUM as Integer)

to when called from fileA at line 10 in init() to return fileName as fileA, line number as 10 and function name as init

TwitchBronBron commented 4 years ago

@triwav I believe @georgejecook is asking for current line number, file name, function name, etc. What you're asking for is previous information. , so perhaps create a separate github issue to track this, since what you're asking for is quite a bit more difficult. We would need to implement some type of tracking or set a global variable for every scope, and update it before every function call.

triwav commented 4 years ago

@TwitchBronBron totally get that. At least in my logging setup the current discussion doesn't have much value to me so thought I'd chime. To be clear, everything with what I'm saying would be done when we convert from brighterscript to brightscript. It would basically take those placeholders and do the replacement at the call site instead of inside the function. Hope that makes sense.

georgejecook commented 4 years ago

It seems to me that @TwitchBronBron is reading my original intent well. I wanted simply the line number, file and function for the current file. If we're asking for some kind of other (far more complicated, and much more involved) stack trace mechanism, then I think we should discuss that in it's own issue. Of course, I have opinions on that too; but I don't believe this issue is the place for them :)

TwitchBronBron commented 4 years ago

@georgejecook just to be clear, you are not asking for source line number, right? You want the runtime line number? Do you see any value in supporting a SOURCE_LINE_NUMBER variable that statically inserts the line number of the current source file?

georgejecook commented 4 years ago

oh - yes - the line number should be the line number in the file at processing time not at runtime (in case it changed due to code restructuring)