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 support for the Jolie language #1014

Merged
merged 9 commits into from
Nov 9, 2016
Merged

Conversation

thesave
Copy link
Contributor

@thesave thesave commented Aug 19, 2016

No description provided.

@@ -0,0 +1,16 @@
Prism.languages.jolie = Prism.languages.extend('java', {
'keyword': /(<-|=>)|\b(is_defined|undef|include|main|outputPort|inputPort|Location|Protocol|RequestResponse|throw|OneWay|interface|any|long|type|void|sequential|raw|scope|forward|install|execution|single|concurrent|Interfaces|cset|csets|double|global|linkIn|linkOut|string|bool|int|synchronized|courier|extender|throws|this|new|Interfaces|nullprocess|Redirects|embedded|extender|Aggregates|spawn|constants|with|foreach|instanceof|<-|over|define)\b/g,
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 the arrow <- near the end of the regexp was not intended to be there anymore.

@Golmote
Copy link
Contributor

Golmote commented Aug 19, 2016

Thanks for contributing. Please take a look at the few comments I made on the code.

Also, it would be truly awesome if you could:

'builtin': /\b(string|int|long|Byte|bool|double|float|char|any)\b/g,
'number': /\b0x[\da-f]*\.?[\da-f\-]+\b|\b\d*\.?\d+[e]?[\d]*[dfl]?\b/gi,
'operator': /[-+]{1,2}|!|<=?|>=?|={1,3}|&{1,2}|\|\||\*|\//g,
'punctuation': /[{}[\]()\.:]/g,
Copy link
Contributor

Choose a reason for hiding this comment

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

The dot . does not need escaping inside a character class.

- revised jolie component
- added tests
- added code example
- minified with `gulp`
@thesave
Copy link
Contributor Author

thesave commented Aug 21, 2016

Hi, thanks for the very useful comments :)
I took the time to extend the support for the Jolie language (also following you advises).
As suggested, I added the examples, tests, and built with gulp.

Prism.languages.jolie = Prism.languages.extend('clike', {
'keyword': /\b(?:include|define|is_defined|undef|main|init|outputPort|inputPort|Location|Protocol|Interfaces|RequestResponse|OneWay|type|interface|extender|throws|cset|csets|forward|Aggregates|Redirects|embedded|courier|extender|execution|sequential|concurrent|single|scope|install|throw|comp|cH|default|global|linkIn|linkOut|synchronized|this|new|for|if|else|while|in|Jolie|Java|Javascript|nullProcess|spawn|constants|with|provide|until|exit|foreach|instanceof|over|service)\b/g,
'builtin': /\b(?:undefined|string|int|void|long|Byte|bool|double|float|char|any)\b/,
'number': /\b(?:\b\d*\.?\d*(e[+-]?\d*)?[l]?)\b/i,
Copy link
Contributor

Choose a reason for hiding this comment

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

There is not need for the square brackets around [l]. Also, the second \b seems redundant to me. Finally, it looks like your regexp may match nothing at all, since everything is optional. Please replace one of the two first \d* occurrences with \d+. I also suspect this part e[+-]?\d* should be e[+-]?\d+ since numbers after the exponent symbol are usually not optional.

@Golmote
Copy link
Contributor

Golmote commented Aug 29, 2016

I reviewed your additions and made a bunch of comments about them. Please take a look when you can.

Those additions would also need test files ;)

@thesave
Copy link
Contributor Author

thesave commented Sep 1, 2016

Hi, thank you very much for your review, I really appreciate it :)
I tried to follow all your suggestions and added tests for the new features.

Prism.languages.jolie = Prism.languages.extend('clike', {
'keyword': /\b(?:include|define|is_defined|undef|main|init|outputPort|inputPort|Location|Protocol|Interfaces|RequestResponse|OneWay|type|interface|extender|throws|cset|csets|forward|Aggregates|Redirects|embedded|courier|extender|execution|sequential|concurrent|single|scope|install|throw|comp|cH|default|global|linkIn|linkOut|synchronized|this|new|for|if|else|while|in|Jolie|Java|Javascript|nullProcess|spawn|constants|with|provide|until|exit|foreach|instanceof|over|service)\b/g,
'builtin': /\b(?:undefined|string|int|void|long|Byte|bool|double|float|char|any)\b/,
'number': /\b(?:\d*\.?\d+(e[+-]?\d+)?l?)\b/i,
Copy link
Contributor

@Golmote Golmote Sep 1, 2016

Choose a reason for hiding this comment

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

The outer parentheses are not needed here. And the inner ones should be made non-capturing.

/\b\d*\.?\d+(?:e[+-]?\d+)?l?\b/i

lookbehind: true
},
'aggregates': {
pattern: /(\bAggregates\s*:\s*)(?:\w+(\s+with\s+\w+)?\s*,\s*)*\w+(?:\s+with\s+\w+)?/,
Copy link
Contributor

Choose a reason for hiding this comment

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

There is still a group here (\s+with\s+\w+)? that should be made non-capturing.

@Golmote
Copy link
Contributor

Golmote commented Sep 1, 2016

Great! Almost there!

* PrismJS/gh-pages:
  Autoloader plugin: don't try to load "none" component. Fix PrismJS#1000
  Optimized block regexps to prevent struggling on large files. Fixes
PrismJS#1001
  Revert PrismJS#998 + run normalize-whitespace and remove-initial-line-feed
plugins in the before-sanity-check hook. Fix PrismJS#1018 (see issue for
discussion)
  Run gulp

# Conflicts:
#	plugins/autoloader/prism-autoloader.min.js
@thesave
Copy link
Contributor Author

thesave commented Sep 2, 2016

Did the required fixes.
You are right about <= and =<, I added also the support for *= (whilst %= is not supported)
I also moved => from operators to symbols.

@Golmote
Copy link
Contributor

Golmote commented Sep 2, 2016

The code looks quite good to me now.

I pulled your branch to test it a bit, and I noticed your examples for strings and numbers aren't exactly working well, though. Most of those cases don't appear to be handled by the current regexps. Either we need to handle them or they should not appear in the examples. Can you take a look?

prism-jolie-broken-examples

Also, in the full example, I'm curious about the color of the parentheses and commas. They probably should go inside the punctuation rule, shouldn't they?

* PrismJS/gh-pages:
  update patterns (PrismJS#1032)
  Test suite: fixed missing diff in error message
  Reverse prism markup min
  JSON: Fixed issues with properties and strings + added tests. Fix
PrismJS#1025
  Fix grammar in Readme
  Update CHANGELOG
  Update APL minified file + update test for iota underbar function
  Add iota underbar (PrismJS#1024)
  Fix typo `Fload` to `Float` in prism-ruby.js (PrismJS#1023)
  update min file and reverse core
  add prefix feature for custom class plugin
  add plugin custom class name

# Conflicts:
#	plugins/autoloader/prism-autoloader.min.js
@thesave
Copy link
Contributor Author

thesave commented Oct 14, 2016

Sorry for the late reply. Work swamped me :)
I fixed the example file and the errors on strings and punctuations.

@Golmote Golmote merged commit dfc1941 into PrismJS:gh-pages Nov 9, 2016
@Golmote
Copy link
Contributor

Golmote commented Nov 9, 2016

Sorry for the late answer too. It's merged now.

@thesave
Copy link
Contributor Author

thesave commented Nov 9, 2016

Thank you very much for your time :)

thesave added a commit to thesave/prism that referenced this pull request Nov 9, 2016
* PrismJS/gh-pages:
  Plugins: Toolbar & Copy to Clipboard (PrismJS#891)
  Ini: Fix test after  PrismJS#1047
  Add support for the Jolie language (PrismJS#1014)
  Fix order of decoding entities in groovy (PrismJS#1049) (PrismJS#1050)
  Ruby: Make strings greedy. Fixes PrismJS#1048
  Ini: Remove newline at end of minified file.
  Ruby: Fix test after PrismJS#1023
  Remove important token in ini definition (PrismJS#1047)
  Add missing `from` keyword to typescript & set `ts` as alias. (PrismJS#1042)
  Fix greedy-flag bug
  Add yarn.lock (PrismJS#1035)
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.

2 participants