Add JSP (Java Server Pages) lexer #915
Conversation
There was a problem hiding this comment.
Thanks for the contribution @miparnisari. I have some style comments but otherwise looks good.
| @@ -0,0 +1,116 @@ | |||
| # -*- coding: utf-8 -*- # | |||
|
|
|||
| module Rouge | |||
There was a problem hiding this comment.
Please change all indenting to 2 spaces.
There was a problem hiding this comment.
FWIW, I'm not sure why Rubocop didn't complain. I'll investigate that later.
lib/rouge/lexers/jsp.rb
Outdated
| rule(/<\/[a-zA-Z]*:[a-zA-Z]*/) { token Name::Tag; push :jsp_tag_end } | ||
|
|
||
| rule(/<%[!=]?/) { token Name::Tag; push :jsp_expression2 } | ||
| end |
There was a problem hiding this comment.
Prefer the non-block argument style in the cases above: rule(/.../), Name::Tag, :jsp_directive is equivalent to rule(/.../) { token Name::Tag; push :jsp_directive }.
Also, be consistent with use of parentheses throughout the file - rule /.../ or rule(/.../).
There was a problem hiding this comment.
the parentheses are required in these cases?
There was a problem hiding this comment.
but yes, the non-block style would be better.
lib/rouge/lexers/jsp.rb
Outdated
|
|
||
| state :jsp_expression do | ||
| rule(/<\/jsp:(#{actions.join('|')})>/) { token Name::Tag; pop! } | ||
| rule(/[^<\/]+/) { delegate @java } |
There was a problem hiding this comment.
Although there's technically nothing wrong with this syntax, let's stick to do ... end style blocks. I don't see this style used anywhere else in the code so let's stay consistent.
There was a problem hiding this comment.
??? i use this style all the time...
There was a problem hiding this comment.
Ah, yep. I see it now. Don't know why I wasn't finding these examples earlier.
dblessing
added
the
author-action
|
For template languages like this, it is more appropriate to delegate to the parent language than to subclass it (see other template lexers like ERB for example), since the templating system acts as a preprocessor before the html syntax is interpreted. |
|
Thanks for pointing out the ERB example @jneen. That looks nice and clean. @miparnisari Can you take a look, please, and see how that might work? |
|
Thanks @jneen and @dblessing ! I'll have a look at the ERB example later. |
|
@jneen @dblessing i've updated the lexer to subclass it from |
|
Thanks @miparnisari I'll review shortly. |
There was a problem hiding this comment.
This is really great @miparnisari Thank you. Also, thank you for updating the CHANGELOG, although now it falls in the wrong spot. Would you mind removing that, or if you'd like you can create a section for the next release. But I can also do that along with other changes. Hopefully I have this automated in the future.
Also, if you're able, please squash the commits into one logical commit. This is not a huge issue and if you have trouble with it, that's OK.
|
Both done. |
|
Any chance of releasing this soon in version 3.2.2? @dblessing |
Tested with close to 300 JSP files 😄
