nishtahir / language-kotlin

Textmate language grammar for the Kotlin programming language
Apache License 2.0
12 stars 6 forks source link

Fixes recognition of single-lettered class and enum entry (Fixes #38) #39

Closed Animeshz closed 3 years ago

Animeshz commented 3 years ago

Fixes the single-lettered highlighting of class and enum-entry by OR(ing) with single-lettered capital word which is not before >.

Should be merged after PR #37 because this contains another commit over it πŸ˜….

codecov[bot] commented 3 years ago

Codecov Report

Merging #39 (3214ca7) into master (e2b7595) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #39   +/-   ##
=======================================
  Coverage   68.30%   68.30%           
=======================================
  Files          12       12           
  Lines         467      467           
=======================================
  Hits          319      319           
  Misses        148      148           
Impacted Files Coverage Ξ”
src/ident.YAML-tmLanguage 100.00% <100.00%> (ΓΈ)

Continue to review full report at Codecov.

Legend - Click here to learn more Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data Powered by Codecov. Last update e2b7595...3214ca7. Read the comment docs.

Animeshz commented 3 years ago

Oops this will make it inconsistent where generic type is used elsewhere (which will be coloured), other than <> declarations.

We should probably allow every single-letter keyword even generics to be highlighted similar to a class πŸ‘€. It will be alot more consistent, would it πŸ€”? Atleast single-lettered classes and enum-entries will be highlighted properly, and generics/template are already highlighted similar to class in many other languages like C++ πŸ€”. Need opinions.

nishtahir commented 3 years ago

I've actually had issues matching generics properly in the past (#19 is an example of one of the issues) since at the time I didn't have tests/code coverage setup to prevent regressions. So I think it would be good to take another stab at it. We can take some inspiration from other languages here. I would recommend breaking down how it's currently matched in the Java grammar

nishtahir commented 3 years ago

πŸ€” looking at the official Kotlin grammar it looks like an Identifier can be exactly 1 letter or _ so I think the class-ident match rule is wrong. I think if it were

"match": "\\b([A-Z_]\\w*\\b", 

it should fix your issue

Animeshz commented 3 years ago

Yep, I concluded to do that in the previous comment, it'll make the generics to be highlighted as classes with #6f42c1 which is I guess not in Java and it did properly recognized single-lettered enums as well. Probably they have better rules, need to study.

Edit: Oh well, they use similar technique as in this PR, the toArray function has T (parameter) highlighted as classes, while ones inside <> are not highlighted. :O

Edit2: They are of different color, I guess they properly recognized by angle bracket <> parsing, probably we should better fix that issue before going for this one!

Edit3: Hmm, they are inconsistent as well, somewhere E is highlighted, whereas somewhere not.

Animeshz commented 3 years ago

I guess your suggestion makes sense, it'll make all the generics as well as classes highlighted with same color and things will be a lot more consistent.

nishtahir commented 3 years ago

Superseded by #41. Closing