Sublime-Instincts / BetterJinja

A Sublime Text package that provides enhanced syntax highlighting, completions, snippets & more for Jinja templates.
https://packagecontrol.io/packages/Jinja2
MIT License
15 stars 4 forks source link

[BUG] Parenthesised expressions scoped as tuples. #2

Closed ROpdebee closed 5 months ago

ROpdebee commented 4 years ago

Summary

Parenthesised expressions are being scoped as tuples rather than expressions. This also leads to contents of these expressions not being scoped correctly. For example:

   {{ (1 + 2) }}
// ^^^^^^^^^^^^^ text.html.basic meta.placeholder.jinja
// ^^ punctuation.definition.placeholder.begin.jinja
//    ^^^^^^^ meta.sequence.tuple.jinja
//    ^ punctuation.section.sequence.begin.jinja
//     ^ constant.numeric.integer.jinja
//         ^ constant.numeric.integer.jinja
//          ^ punctuation.section.sequence.end.jinja
//            ^^ punctuation.definition.placeholder.end.jinja

   {{ not (test and false) }}
// ^^^^^^^^^^^^^^^^^^^^^^^^^^ text.html.basic meta.placeholder.jinja
// ^^ punctuation.definition.placeholder.begin.jinja
//    ^^^ keyword.operator.word.jinja
//        ^^^^^^^^^^^^^^^^ meta.sequence.tuple.jinja
//        ^ punctuation.section.sequence.begin.jinja
//                       ^ punctuation.section.sequence.end.jinja
//                         ^^ punctuation.definition.placeholder.end.jinja

Expected Behaviour

   {{ (1 + 2) }}
// ^^^^^^^^^^^^^ text.html.basic meta.placeholder.jinja
// ^^ punctuation.definition.placeholder.begin.jinja
//    ^^^^^^^ a sensible scope for parenthesised expressions
//    ^ punctuation.section.sequence.begin.jinja
//     ^ constant.numeric.integer.jinja
//       ^ keyword.operator.arithmetic.jinja
//         ^ constant.numeric.integer.jinja
//          ^ punctuation.section.sequence.end.jinja
//            ^^ punctuation.definition.placeholder.end.jinja

   {{ not (test and false) }}
// ^^^^^^^^^^^^^^^^^^^^^^^^^^ text.html.basic meta.placeholder.jinja
// ^^ punctuation.definition.placeholder.begin.jinja
//    ^^^ keyword.operator.word.jinja
//        ^ punctuation.section.sequence.begin.jinja
//         ^^^^ variable.other.jinja
//              ^^^ keyword.operator.word.jinja
//                  ^^^^^ constant.language.jinja
//                       ^ punctuation.section.sequence.end.jinja
//                         ^^ punctuation.definition.placeholder.end.jinja

Actual Behaviour

See above.

How to Reproduce

See above.

Problematic Jinja2 template.

{{ (1 + 2) }}
{{ not (test and false) }}

Environment

Additional context

Probably somehow making sure a comma is present before attempting to scope should fix this? Perhaps: https://github.com/sublimehq/Packages/blob/master/Python/Python.sublime-syntax#L925

I just spent a day working on an improved Jinja2 syntax, to then find out you've already done this 10 days prior! Should've done more research 😄

UltraInstinct05 commented 4 years ago

I am aware of this (along with the issue about tuple unpacking you have referenced and a few other places where the syntax doesn't receive an appropriate scope). As an end user, however, it should be of little importance that a () should receive either a meta.sequence or a meta.group scope (which is what the Python syntax uses for scoping parenthesized expression) since color schemes would likely color them the same.

This will eventually be fixed. Do you have any specific use case where you need to make the distinction as I have outlined above ?

ROpdebee commented 4 years ago

I don't immediately have a use case for the scoping issue on the parentheses. However, the main problem that it leads to right now is that the contents of these expressions isn't scoped correctly either. The not (test or false) case (although contrived) leads to no scoping whatsoever on the contents of the expression. Similarly, for (1+2), the + isn't scoped at all.

The contents not being scoped may be more related to #4 though.

UltraInstinct05 commented 4 years ago

Ah, okay. This was fixed for Twig but I have not yet fixed it for Jinja2. It doesn't have to do with #4. It is just an oversight on my part not to have included the specific contexts for the operators and language constants. This will be fixed in v1.0.1

UltraInstinct05 commented 3 years ago

This has been fixed & will be a part of the next release.