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

Add SuperCollider language support #749

Merged
merged 20 commits into from Jun 6, 2019

Conversation

mossheim
Copy link
Contributor

This adds support for the SuperCollider language.

The visual sample contains a near-complete rundown of general language cases, including corner cases that I've found as a maintainer of the language's own backend lexer. The highlighting is mostly adapted from the project's IDE's lexing regexes, which I've found to be the most solid set among all the lexers out there for the language.

Let me know if I missed anything. Thanks!

@mossheim
Copy link
Contributor Author

Hm. It looks like the Python lexer also says *.sc is one of its filetypes. Is that right? I can't find anything to confirm that. The sc extension is one of SuperCollider's only two extensions; maybe we could have this one?

https://fileinfo.com/extension/sc
http://dcjtech.info/topic/python-file-extensions/

@jneen
Copy link
Member

jneen commented Aug 14, 2017

.sc is for http://scons.org/ if I remember correctly. I would support changing it to supercollider.

@jneen
Copy link
Member

jneen commented Aug 14, 2017

In general, I'd like to refactor the guessers to use more specific disambiguators, like GitHub's linguist project: https://github.com/github/linguist/blob/master/lib/linguist/heuristics.rb

@mossheim
Copy link
Contributor Author

ahh I see! well, I can push a change for that if you want. I don't want to break things that are already in use.

.sc files in SuperCollider have a predictable scheme: a few lines of comments followed by some form of class or class extension declaration. could I make use of that somehow with the existing guesser implementation?

@mossheim
Copy link
Contributor Author

btw, I'm already using this branch to generate HTML for my github-pages blog. thanks for making this tool so easy to use & extend!

@jneen
Copy link
Member

jneen commented Aug 15, 2017

I'm happy just to give the extension to supercollider for now, and revisit the guessers later. Though if you want to make an attempt at it in a separate MR that would be super helpful!

@mossheim
Copy link
Contributor Author

great! ok, pushed the change and tests now pass.

@mossheim
Copy link
Contributor Author

ping. could this please be merged? if there's anything else I should change please let me know.

Thanks!

@jneen
Copy link
Member

jneen commented Oct 13, 2017

Yes! And now we can use the specific disambiguators to solve the guessing problem.

@jneen
Copy link
Member

jneen commented Oct 13, 2017

@gfx this lgtm. we should add a disambiguator before the next release, though.

@mossheim
Copy link
Contributor Author

mossheim commented Nov 6, 2017

ping - any updates?

@mossheim
Copy link
Contributor Author

mossheim commented Feb 9, 2018

still waiting on an update. is there any plan to integrate this?

@mossheim
Copy link
Contributor Author

The SuperCollider project is redoing its website at the moment which has revived discussion about this PR. Is there any plan to merge this, or should I and my collaborators just preprocess our code blocks ourselves?

@geoffroymontel
Copy link

+1
It would be nice if you could merge it, as Github Pages does whitelist the rouge gem repository, we can not specify our own.

Copy link
Contributor

@vidarh vidarh left a comment

Choose a reason for hiding this comment

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

Hi, I'm trying to help triage old pull requests so we can try to work them down. Can you please take a look at the comments.

lib/rouge/lexers/supercollider.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/supercollider.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/supercollider.rb Show resolved Hide resolved
spec/lexers/supercollider_spec.rb Show resolved Hide resolved
lib/rouge/lexers/python.rb Outdated Show resolved Hide resolved
@mossheim
Copy link
Contributor Author

@vidarh Thanks for the comments; I've made the rest of the changes.

@mossheim
Copy link
Contributor Author

mossheim commented Jun 4, 2019

What's the status on this?

@pyrmont
Copy link
Contributor

pyrmont commented Jun 4, 2019

@brianlheim Hi, I started as a maintainer at the end of last month and am working through the backlog of outstanding PRs. I'm sorry this has been in the queue for so long :(

I realise this will feel like you're being jerked around even more but I'll have some additional comments on the lexer I'll send as a review. To explain, there are a couple of things that we're trying to change with lexers for performance reasons and, while there are existing lexers not doing these things (fixing these is on the list), we're tying to ensure new lexers are all good in this regard.

The other point is with respect to the .sc file extension. I don't think it's fair to put that on your plate at such a late stage of the game so I will write the actual disambiguation code but could you explain a bit more what you meant in this comment? That would be very helpful.

@mossheim
Copy link
Contributor Author

mossheim commented Jun 4, 2019

if you can guarantee a clear timeline for when this will get merged, i'll gladly help out. this is the third time someone has promised a merge if i only change a few things.

lib/rouge/lexers/supercollider.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/supercollider.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/supercollider.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/supercollider.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/supercollider.rb Outdated Show resolved Hide resolved
@pyrmont
Copy link
Contributor

pyrmont commented Jun 4, 2019

@brianlheim Yeah, I understand. Honestly, I wouldn't blame you if you wanted to wash your hands of this and walk away. I can only point at the commit history since I came on board.

I have committing rights and will be ready to merge this PR once: (1) the changes are made in the review just submitted; and (2) I have a disambiguation rule written (I'll get this done as soon as possible).

@pyrmont pyrmont added the author-action The PR has been reviewed but action by the author is needed label Jun 4, 2019
@mossheim
Copy link
Contributor Author

mossheim commented Jun 4, 2019

i'll wait for (2) then, thanks.

@pyrmont
Copy link
Contributor

pyrmont commented Jun 4, 2019

@brianlheim That's fair.

Can you explain what you meant in the comment I referenced earlier? Basically, I need to write code that will distinguish between Python and SuperCollider. You can see how this works for existing disambiguation in the Disambiguation class:

disambiguate '*.pl' do
next Perl if contains?('my $')
next Prolog if contains?(':-')
next Prolog if matches?(/\A\w+(\(\w+\,\s*\w+\))*\./)
end
disambiguate '*.h' do
next ObjectiveC if matches?(/@(end|implementation|protocol|property)\b/)
next ObjectiveC if contains?('@"')
C
end
disambiguate '*.m' do
next ObjectiveC if matches?(/@(end|implementation|protocol|property)\b/)
next ObjectiveC if contains?('@"')
next Mathematica if contains?('(*')
next Mathematica if contains?(':=')
next Matlab if matches?(/^\s*?%/)
end
disambiguate '*.php' do
# PHP always takes precedence over Hack
PHP
end
disambiguate '*.hh' do
next Cpp if matches?(/^\s*#include/)
next Hack if matches?(/^<\?hh/)
next Hack if matches?(/(\(|, ?)\$\$/)
Cpp
end
disambiguate '*.plist' do
next XML if matches?(/\A<\?xml\b/)
Plist
end

Without having thought too deeply about it, my initial idea is that the ~ character is a differentiator. Ideally, what I'd like is to find some syntax that's only in Python and then if that's not there default to SuperCollider.

@pyrmont pyrmont added maintainer-action The PR has been reviewed but action by a maintainer is required and removed author-action The PR has been reviewed but action by the author is needed labels Jun 4, 2019
@mossheim
Copy link
Contributor Author

mossheim commented Jun 5, 2019

@pyrmont Thanks! I've pushed a commit that removes local variables and another to update the visual sample. Not sure I understand your other comments. IIRC I copied this one mostly from the JavaScript lexer. Re this comment:

Is there anything special that's perhaps at the beginning of the file?

You're using a :bol state which is usually what that's for but it looked to me like the actual content is just the ordinary comments that are captured by the rules you mix in at the beginning of the :root state.

nothing special at the beginning of a file. if you think I can remove some of this logic, just let me know and I'll do that.

@pyrmont
Copy link
Contributor

pyrmont commented Jun 5, 2019

@brianlheim It's probably easiest if I make some edits to the file and then submit them to you again as a PR. That'll make the thoughts clearer, I think. I'll try to do that sometime today.

@mossheim
Copy link
Contributor Author

mossheim commented Jun 5, 2019

@pyrmont Got it! I'll look at it tomorrow evening. Thanks so much.

@pyrmont
Copy link
Contributor

pyrmont commented Jun 5, 2019

I've submitted a simplified lexer that produces the same output (or at least seems to based on the visual sample).

@mossheim
Copy link
Contributor Author

mossheim commented Jun 6, 2019

@pyrmont thanks! is there anything else you need now?

@pyrmont
Copy link
Contributor

pyrmont commented Jun 6, 2019

@brianlheim My first pass is that it looks right! I'll take a look at the visual sample during lunch (in Tokyo) and if it's all good, should be able to merge! :)

@mossheim
Copy link
Contributor Author

mossheim commented Jun 6, 2019

fantastic!

@pyrmont pyrmont merged commit f494b53 into rouge-ruby:master Jun 6, 2019
@pyrmont pyrmont removed the author-action The PR has been reviewed but action by the author is needed label Jun 6, 2019
@pyrmont
Copy link
Contributor

pyrmont commented Jun 6, 2019

Merged it in now. Sorry it took so long. I'm discussing with the other maintainers at the moment when the next release of the Rouge gem will be but it should be soon.

Thanks for contributing!

@mossheim mossheim deleted the lexer/supercollider branch June 6, 2019 11:08
@mossheim
Copy link
Contributor Author

mossheim commented Jun 6, 2019

Thanks so much @pyrmont. To be honest, I was quite skeptical when you revived this thread. But, I have to say that this PR since you took it up has been the best experience I've had contributing to a 3rd party OSS project. You're doing a really great job as a maintainer. :)

@pyrmont
Copy link
Contributor

pyrmont commented Jun 6, 2019

Thanks @brianlheim :) That's nice of you to say. It was a very unfortunate sequence of events really but I think we're back on track now!

@mossheim
Copy link
Contributor Author

mossheim commented Jun 6, 2019

Wish you all the best! Where can I go to be informed of your release schedule? I'm really looking forward to being able to use this in github-pages!

@ashmaroli
Copy link
Contributor

@brianlheim I don't intend to squash your bubble, but please do not expect to see the next release on GitHub Pages anytime soon.. They've their own way of managing dependencies.

The latest release is still locked to rouge-2.2.1

@mossheim
Copy link
Contributor Author

mossheim commented Jun 6, 2019

no worries, i understand there are two release cycles to deal with here :) still, it would at least allow people to easily grab this project and use it to colorize their own code.

@pyrmont
Copy link
Contributor

pyrmont commented Jun 7, 2019

@brianlheim We haven't formalised anything in terms of release schedules yet.

My initial preference while we clear the backlog is to try to do a new release each month but that's not something I've discussed yet with the other maintainers. Projects like GitLab use Rouge and they may prefer to see more rapid updates since a number of the outstanding PRs were generated by their users.

To keep track of future releases, I'd suggest turning on notifications from the front page of this project. You can choose there to be notified about releases only :)

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

Successfully merging this pull request may close these issues.

None yet

6 participants