Add BrightScript syntax
ClosedPublic

Authored by dlevin on Nov 5 2018, 3:23 AM.

Details

Summary

Accurate implementation of BrightScript language, mostly used by Roku
channel developers and documented at Roku SDK portal:

https://sdkdocs.roku.com/display/sdkdoc/Roku+SDK+Documentation

Highlight sytax covers:

  • function and sub definitions, both global and inline
  • comments: ' and REM
  • macros
  • print statement
  • library imports
  • literal type narrowing with: $, %, !, #, &
  • conditionals and loops with: if, for and while
  • built-in and library functions
  • standard Roku Scene Graph keywords: m, top, global
  • array, function calls
  • goto and label statements
  • built-in types used in function/sub arguments and return
  • constants: true, false, invalid
  • distinct assignment and unary/binary ops, e.g.: if a = b c = d ' first = is a comparison, second = is an assignment
  • invalid syntax detection in some cases

No folding is supported at this point.

Diff Detail

Repository
R216 Syntax Highlighting
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dlevin created this revision.Nov 5 2018, 3:23 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptNov 5 2018, 3:23 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
dlevin requested review of this revision.Nov 5 2018, 3:23 AM

Hi, thanks for the contribution.
Could you use the MIT license for the files?

dlevin updated this revision to Diff 44885.Nov 5 2018, 7:35 AM

Changed syntax license to MIT.

cullmann accepted this revision.Nov 5 2018, 8:21 AM

Ok, beside that I am not sure why you use 1-element keyword list instead of StringDetect at some places, I see no issues with that.
If you like, you can change that, otherwise I am fine with that as is.

This revision is now accepted and ready to land.Nov 5 2018, 8:21 AM
dlevin added a comment.Nov 5 2018, 8:45 AM

There are various reasons I was using 1-element keyword lists instead of StringDetect:

  1. keyword properly recognizes value surrounded by non-word characters, StringDetect does not.
  2. BrightScript language is case insensitive, adding insensitive="true" to every single StringDetect is too verbose in my case.
dlevin added a comment.Nov 9 2018, 1:59 AM

Just double checking whether any additional actions required from my side or 'landing' a patch means it will be a part of the next release automatically.
If so then where I could find a release roadmap?
Thanks in advance!

Unless you have a KDE commit account you can not commit yourself. Unfortunately we cannot see on Phabricator whether you have one or not. We can integrate this patch and then it will be in the next KDE Frameworks release, i.e. early next month.

dlevin added a comment.Nov 9 2018, 5:46 PM

I do not have a KDE commit account, so I guess I need your help to integrate this patch.

I can push that for you, just need to fixup my local dev setup again ;=)

dlevin added a comment.Nov 9 2018, 7:50 PM

Thanks a lot for helping me with this! One last question: how do I know that patch was finally merged, by periodically checking the github mirror git repository? https://github.com/KDE/syntax-highlighting.git

The merge should show up here and you should get a notification mail like for any other action.

ngraham added a subscriber: ngraham.Nov 9 2018, 9:46 PM

Unless you have a KDE commit account you can not commit yourself. Unfortunately we cannot see on Phabricator whether you have one or not.

BTW, you can check to see whether someone has a contributor account by trying to find their username on https://websvn.kde.org/trunk/kde-common/accounts?view=markup

@ngraham, so I have to match commit authors in syntax-highlighting Git project against that list? Curious whether all those people have rights to push patches particularly into this project. Still it feels like this is not an easy task to do that manually, so I would rather annoy you guys in this message thread with my push request (-:

@dlevin

That comment was meant for other KDE developers. :) There's nothing you need to do on that front... however, since you submitted the patch using the web interface rather than using arc, your authorship information was not included with the patch and we'll need your full name and preferred email address before we can land the patch. Can you provide that?

That's all fine, its inside the xml syntax file: "Daniel Levin (dendy.ua@gmail.com)" ;=)
I will use that for the commit, otherwise I would have asked ;=)

You can find my credentials at the beginning of the file I added in this patch: data/syntax/brightscript.xml

cullmann requested changes to this revision.Nov 10 2018, 8:20 PM

Hi, I tried to land the patch but the consistency checks of the indexer tell me:

"/home/cullmann/local/kde/src/frameworks/syntax-highlighting/data/syntax/brightscript.xml" Reference of non-existing itemData attributes: QSet("code")

I think the

<context name="sub_code" attribute="code" lineEndContext="#stay">

line needs some fix ;=)

This revision now requires changes to proceed.Nov 10 2018, 8:20 PM
dlevin updated this revision to Diff 45260.Nov 11 2018, 5:45 AM

Replaced missing "code" attribute with "g".

Thanks for catching this. Did re-run tests locally, seems no other issue remaining. Please check.

cullmann accepted this revision.Nov 11 2018, 2:22 PM

Looks OK, will merge, thanks!

This revision is now accepted and ready to land.Nov 11 2018, 2:22 PM
This revision was automatically updated to reflect the committed changes.