Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

Add support for the conditional compile elements #11

Merged
merged 12 commits into from
Oct 21, 2018

Conversation

evgygor
Copy link
Contributor

@evgygor evgygor commented Oct 7, 2018

No description provided.

Copy link
Member

@TwitchBronBron TwitchBronBron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, thanks for contributing!

A few things:

  1. There was a lot of whitespace change in your BrightScriptFormatter.ts file. Can you update the file to match the formatting of this project so that only the actual code differences show up in the git diff? This project uses 4 spaces per tab, not 2. My guess is that you have that setting overridden somewhere in your editor? I just pushed a new commit to master that with some new formatting settings in the vscode config that will help enforce this at development time in the future.
  2. Can you add some more tests? I would like to see a few more complex statements be tested. (I put the proper amount of spaces in my markdown, but it doesn't look like it's rendering them.) See my other tests for examples. I usually just write my program using the default formatting options, write the code the way it is supposed to be formatted, and then format it. If it came out the same way, then the formatter works.
    • "#if isDebug\n doSomething()\n#end if"
    • "#if isDebug\n doSomething()\n#else\n doSomethingElse"
    • "#if isDebug\n doSomething()\n#else if isPartialDebug\n doSomethingElse()\n#else\n doFinalThing()\n#end if"
      -"#if isDebug\n if true then\n doSomething()\n end if\n#end if"
    • "if true then\n #if isDebug\n doSomething()\n #end if"

@evgygor
Copy link
Contributor Author

evgygor commented Oct 9, 2018

Thanks for your comments and remarks.
Starting to fix those issues.

@evgygor
Copy link
Contributor Author

evgygor commented Oct 18, 2018

I fixed the whitespace issues and added tests for conditional compile blocks.

@evgygor
Copy link
Contributor Author

evgygor commented Oct 21, 2018

@TwitchBronBron, can you please review the request?

Thanks

@TwitchBronBron TwitchBronBron merged commit 5ff9b04 into rokucommunity:master Oct 21, 2018
@TwitchBronBron
Copy link
Member

Done. There were still a ton of whitespace issues in the main file (i.e. they were still indented with 2 spaces instead of 4), but I'll just fix them. Thanks for your contribution.

@evgygor
Copy link
Contributor Author

evgygor commented Oct 21, 2018

Done. There were still a ton of whitespace issues in the main file (i.e. they were still indented with 2 spaces instead of 4), but I'll just fix them. Thanks for your contribution.

Strange...
My default indentation is 4 spaces in all projects. Maybe is a formatter issue.
I'll check after the merge.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants