Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lexer for Inform 7 #1727

Merged
merged 4 commits into from
Nov 17, 2014
Merged

Lexer for Inform 7 #1727

merged 4 commits into from
Nov 17, 2014

Conversation

pchaigno
Copy link
Contributor

This pull request adds a lexer for Inform 7 as requested in #1725.
The lexer is at: https://github.com/PogiNate/Sublime-Inform.

I opened an issue on the repository to clarify the license situation: PogiNate/Sublime-Inform#2.

@@ -278,7 +278,9 @@ https://github.com/textmate/haskell.tmbundle:
https://github.com/textmate/html.tmbundle:
- text.html.basic
https://github.com/textmate/inform.tmbundle:
- source.inform
- source.inform6
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately just changing the scope name here won't work. script/download-grammars regenerates this file when it runs and will undo this change. (In fact, the best way to add new grammars to this file is to run script/download-grammars --add https://github.com/PogiNate/Sublime-Inform.)

And if we aren't using this grammar anymore we should just remove it.

Choose a reason for hiding this comment

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

Rather than remove it, I suggest adding an entry for Inform 6 to languages.yml.

@aroben
Copy link
Contributor

aroben commented Nov 17, 2014

Thanks for getting this started @pchaigno, and especially for filing that issue about the license!

@pchaigno
Copy link
Contributor Author

@aroben Thanks for the review!
I updated the pull request using script/download-grammars (we should maybe add a note on this in the Contributing part of the README).

@aroben
Copy link
Contributor

aroben commented Nov 17, 2014

we should maybe add a note on this in the Contributing part of the README

Definitely!

@@ -1135,7 +1135,7 @@ Inform 7:
extensions:
- .ni
- .i7x
tm_scope: source.inform
tm_scope: source.inform7
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll need to say source.Inform7 here to match the case used in Sublime-Inform itself.

Running script/prune-grammars should also remove the old inform grammar (after verifying that it's no longer used).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. We should probably have some test of the tm_scope too.
I couldn't run script/prune-grammars (still trying to see why) so I verified manually that it wasn't used anymore.

@pchaigno
Copy link
Contributor Author

@dscorbett I removed the grammar for Inform 6. I couldn't find a lot of Inform 6 files on GitHub (I may not be using the right keywords). If you want to add it, please open an issue with examples of Inform 6 on GitHub, a search result page would be better (we try to only add new languages once they have some usage on GitHub).

@pchaigno
Copy link
Contributor Author

Sublime-Inform is now distributed under the MIT license!
Thanks @PogiNate for the quick answer!

@aroben
Copy link
Contributor

aroben commented Nov 17, 2014

Thanks, this looks great! Did you figure out why prune-grammars doesn't work for you?

@pchaigno
Copy link
Contributor Author

@aroben Nope, I haven't have time to look at it yet. I'll open an issue if I can't figure out what I'm doing wrong.

aroben added a commit that referenced this pull request Nov 17, 2014
@aroben aroben merged commit 970953c into github-linguist:master Nov 17, 2014
@pchaigno pchaigno deleted the lexer-inform7 branch November 17, 2014 19:49
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 18, 2024
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.

3 participants