Closed gavocanov closed 5 years ago
Sorry for the enormous delay! The changes look good, but I'm a bit hesitant about the new ktOperator
syntax group. In my opinion, it adds a bit too much highlighting, which can be annoying on certain color schemes. Also, I couldn't find any mainstream language syntax schemes using similar rules -- all of c.vim
, cpp.vim
, java.vim
, scala.vim
do not include things like comparison and arithmetic operation characters to the syntax group Operator
. Would it be OK for you if I pushed your commit without this part? Thanks!
Hi, Newer syntax files like for ex. javascript and haskell include operators, I like it to dim them usually to reduce noise, but I agree it's a personal preference, np.
Maybe make it a setting? I can do that if you agree.
To clarify, I'd actually agree to turn it on by default if we weren't highlighting things such as .
, :
, single =
and generic angle brackets <
/>
which are very common in Kotlin. With these characters highlighted, any code becomes unnecessarily colorful for my taste. It's not easily possible to distinguish <
/>
in generics vs in comparison operations though. Do you think it would make sense then not to highlight .
, :
and comparison operators (<
, >
because of a clash with generics, and =
because of its usage in assignment statements and because of a symmetry), i.e. only use the following characters for the pattern: -!%&*+/?|
? I've experimented a bit and the results look rather pleasant.
Alternatively, if you'd still like comparison operators to be highlighted too, I'm perfectly fine with hiding this all under an option. In that case, please match the syntax group anyway, under a new name (e.g. ktOperatorSign
), and then only link it with Operator
if the option is provided.
Also, a small comment on the regex: currently it also allows \
and ^
which are not operators in Kotlin, so let's remove them! And the whole regex is enclosed in a group (\(
...\)
), this is not needed I think.
To sum up, this is the resulting regex I propose. What do you think?
syn match ktOperator "[-!%&*+/?|]"
UPD: I've noticed that this rule would make Elvis operator ?:
look strange where ?
is highlighted and :
is not. I propose to support it explicitly. Also maybe support double colon operators. The proposed regex is therefore as follows (\v
is a modifier to turn on Vim's "very magic" regex parsing mode):
syn match ktOperator "\v\?:|::|[-!%&*+/?|]"
Hi, sry for the slight delay, does this look ok now?
Thanks! I've managed to add comparison operators (==
, !=
, ===
, !==
, <
, <=
, >
, >=
) to the ktOperator
group as well, with a heuristic that only works for the latter four operators if they're separated by whitespaces, but that allows not highlighting generic type parameters/arguments, which is good. I've also removed highlighting of ?
but added as?
. The results are looking very good IMHO. I've decided to push this as is by default, and I think an option to enable this behavior is no longer needed.
Regarding the change in ReadMe about another package manager, see https://github.com/udalov/kotlin-vim/pull/5#issuecomment-255043733.
Regarding the changes which add more standard library symbols to ktType
-- I'm actually considering to write a script that pulls all classes from the standard library and adds their names here, to highlight usages of all standard classes. Stay tuned.
I've merged your changes as 0a900d0, 4b86759 and my fixes as 7fc458a, 95848f4. Thanks again for the contrubution!
Using hi def let's user override stuff, in color themes etc., I'm a heavy user of that and it was not working for kotlin.
Also current syntax is commonly defined at the end.
" to ' to make vim linter happy :)
Hope this helps and thnx for Kotlin!