LiveScript lexer

[lkinasiewicz]

LiveScript support added, will close #309

[lkinasiewicz]

Just fixed the demo and also improved numbers recognition

[lkinasiewicz]

Hi, I used subclassing and also did some changes in coffeescript lexer because I found some bugs in it. Hope it's better now. 🙂

[lkinasiewicz]

@jneen ping 🙂

[ice1000]

@jneen ping

[HosseinAgha]

@jneen @pyrmont could you merge this PR? It is much appreciated.
We need LiveScript syntax highlighting in Gitlab and waiting for this to be merged.

[pyrmont]

@HosseinAgha It's not currently in a state where it can be merged but I'll have a look tonight and see what I can do.

[pyrmont]

@HosseinAgha Having had more of a look over the code there's a few things that make this situation complicated.

1. The LiveScript lexer shouldn't subclass the CoffeeScript lexer. I realise that this is the opposite of what @jneen suggested in her review back in 2017 but I think this approach is correct only when one language is a superset of another. While CoffeeScript provided the initial base for LiveScript (as a fork of Coco, itself a fork of CoffeeScript), as best as I can tell, LiveScript is not intended to maintain compatibility with CoffeeScript. In fact, the opposite appears to be true. The LiveScript website has an entire section explaining how to change your CoffeeScript to make it compatible with LiveScript.

2. The CoffeeScript changes should be split out and submitted as a separate PR. We try to follow a 'separation of concerns' approach to PR submissions. This has benefits for maintenance that are worth preserving. Given how long ago this PR was submitted, it seems unrealistic (and a little unfair) to ask @lkinasiewicz to separate out the CoffeeScript changes. For that reason, I have done so myself and submitted these changes as Improve regex and string lexing in CoffeeScript lexer #1441.

3. The JavaScript lexer change should be reverted. I'm not sure why @lkinasiewicz made the changes to the JavaScript lexer that they did. They are not consistent with the JavaScript lexers in comparable syntax highlighters (e.g. Pygments and Chroma). I'm not familiar enough with CoffeeScript or LiveScript to know if they were necessary to ensure those lexers worked correctly. If anyone in the thread has any insight, that would be appreciated.

Once #1441 is dealt with, I can look at rebasing this PR and adjusting it as necessary. Sorry that means this will still be more of a wait :(

[HosseinAgha]

@pyrmont, thank you very much for extensive review. It is good to know that we still have open source projects with such active maintainers. I personally don't know Ruby otherwise I'd be happy to contribute the change. 🙏

[lkinasiewicz]

Hi again. 🙂 Thank you for your interest in this merge request (I thought it would never happen) and splitting out coffescript changes to a separate ticket.

The JavaScript lexer change should be reverted. I'm not sure why @lkinasiewicz made the changes to the JavaScript lexer that they did. They are not consistent with the JavaScript lexers in comparable syntax highlighters (e.g. Pygments and Chroma). I'm not familiar enough with CoffeeScript or LiveScript to know if they were necessary to ensure those lexers worked correctly. If anyone in the thread has any insight, that would be appreciated.

The changes in JS lexer are not required for livescript or coffeescript. I thought that if things such as window, global and even sun are treated as built-ins, then it would be nice to also indicate more obvious console that is available in most of environments (node and browsers). Alert is only specific to browsers (like window) so adding it is questionable.
On the other hand all of the words mentioned above depend on the runtime environment so if we want to stay as close to the language specs as possible, I think they all should be removed from JS lexer and treated like regular variables.

Whatever you think about such changes in JS lexer, the commit should be reverted and handled in a new ticket if necessary. Sorry for inconvenience and touching unrelated code.

@pyrmont If I can help you somehow with livescript lexer, please let me know.

I personally don't know Ruby otherwise I'd be happy to contribute the change

@HosseinAgha Neither do I but this doesn't stop me. 😄

[pyrmont]

Oh, great! You're here, @lkinasiewicz! I've just merged the changes to the CoffeeScript lexer into master. Could you rebase the changes in this PR against master and then update the LiveScript lexer to no longer depend on CoffeeScript? I can do that but I'm not as familiar as you (hopefully still) are. That would be a big help!

[lkinasiewicz]

Sure, I'll try to do it on weekend.

[pyrmont]

@lkinasiewicz Sorry for the delay—will get to this soon!

[pyrmont]

I am very sorry for how long this has taken. Hopefully the comments make sense but if they don't, please do let me know. It's quite late here as I type this so I might have simply made a silly mistake!

[pyrmont]

Sorry it took a few days!

This issue has been resolved

[pyrmont]

@lkinasiewicz Thanks for all your work on this. Three years is a long time but this will finally be part of the next version of Rouge, v3.20.0. That's scheduled for release on Tuesday 9 June 🎉