Delimiter/Separator Highlighting #56
Conversation
|
@a-vrma could you take a look at this? |
|
I meant to ask. Can you clarify how I should probably also give this one more pass since I've gotten more experience writing syntax files while working on the Rust syntax file since submitting the PR. |
|
Pass complete. Sorry about that 😅 |
|
I meant to ask. Can you clarify how `tomlKeyDq` and `Sq` are used?
My understanding is that they each match, respectively, double-quoted and single-quoted TOML keys. If they didn't exist, the keys would be matched as `tomlString`s.
With respect to the change, I don't think it I will accept it. The benefits aren't apparent. It doesn't seem as useful as #52. If you want to pursue full semantic highlighting, using tree-sitter would make a lot more sense than a regex-based Vim plugin.
|
|
The benefit here is twofold:
Treesitter is, in my experience, surprisingly less semantic than Vim syntax files. It will get there in time. If it was a concern, these changes neither impact performance meaningfully nor increase the overhead of maintaining this plugin. |
Iron-E
force-pushed
the
feature/delimiter-highlighting
branch
from
ff8da2f to
e3b5698
Compare
|
@averms Rebased onto master. No conflicts anymore |
Previously, connecting characters such as ',', '{}', '[]', and '=' where
not always highlighted. Further, there were several dedicated highlight
groups where only one comprehensive definition is needed.
For example, the `tomlTableArray` has been replaced with a `tomlTable`
that is wrapped inside of a `tomlArray`.
I believe I put these here because I saw a `,` in the definition, but did not understand it was before the `\zs`.
Iron-E
force-pushed
the
feature/delimiter-highlighting
branch
from
597dc31 to
9455b84
Compare
|
Rebased onto master again. |
|
@averms what is the status of this PR wrt #56 (comment) ? |
|
Sorry that this has just been sitting. From your description of the PR the change makes sense to me. The problem is that it's hard to review. It will take me a while to page back in context about how vim syntax highlighting works, and with the interactions between groups and all the regexes it gets quite subtle. I'm afraid that if I just merge it, we'll find out that there are a bunch of regressions. So I do think that someone needs to do a close review first. @averms any interest in picking it up? Or if someone else can convincingly provide a review, that would be great. Thanks. |
|
I did download and play around with this PR. It seems to mostly work in my tests. Here are some issues that I noticed, though:
|
This PR adds highlighting of separators/delimiters, and takes steps to increase the usage of those tokens in all areas of the syntax file.
This PR also incorporates functionality provided by #52.
masterI know that the maintainer's opinion is that highlighting individual characters isn't all that useful— although, from the perspective of a colorscheme maintainer it is the opposite:
As such, as long as
displayis used where applicable andnextgroup/containsis used to meaningfully provide hints to Vim about the location of syntax groups, the benefits appear to be enough that such changes are worth inclusion.