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

Highlight the dots in dotted keys #52

Merged
merged 1 commit into from
Oct 9, 2021

Conversation

nastevens
Copy link
Contributor

TOML supports the dotted-key notation for defining keys within subtables as described in the "Keys" section of the spec

This adds a highlight to the dots in dotted-keys by setting the dot's style to Normal. The result of this highlight is subtle, but very helpful for picking out the levels of a dotted-key.

@cespare
Copy link
Owner

cespare commented Jul 7, 2020

Thanks for the PR @nastevens!

It's hard for me to get excited about this change. In general I don't find that highlighting each last little bit of syntax is useful for readability, and it can get busy (and slow, if taken to the extreme). In my experience highlighting these sorts of "namespacing operators" isn't all too common -- do you have other examples you can point to where vanilla vim does special highlighting for similar constructs?

For instance, in this screenshot of C++ code, you can see that neither the / (in the #includes), nor the :: namespace separator, nor the . field/method access separator are highlighted:

screen_20200706170349

If we do make this change, I think that for consistency it should also apply to keys that are the names of tables or table arrays:

screen_20200706170809

@nastevens nastevens force-pushed the highlight-dotted-keys branch from 31e240b to 4d48298 Compare July 7, 2020 19:24
@nastevens
Copy link
Contributor Author

Probably the best example, since it's also primarily a data language, is YAML. It does highlight the : after key names:

Vim YAML highlighting

Rust highlights namespace operators like :: and ->, and highlights function calls:

Vim Rust highlighting

One weird note on the Rust hightlighting - in cmp::min you see cmp:: is one color and min is a different color. The :: is actually a different highlight group though, so this is what I see with my preferred color scheme:

Vim Rust highlighting spacegray

A cursory review of some other languages shows there's not a whole lot of consistency on the subject. Here's the summary:

Namespace highlighting

  • Haskell
  • Erlang
  • Rust
  • Yaml

No namespace highlighting

  • Groovy
  • Java
  • Lua
  • Python

I was running everything on vim 8.1.2269 with vim --clean so I wasn't picking up outside plugins. Not sure if this helps or just makes things muddier 😄

If we do make this change, I think that for consistency it should also apply to keys that are the names of tables or table arrays

Totally agree, I've pushed a change that does this.

@cespare
Copy link
Owner

cespare commented Sep 18, 2021

@a-vrma could you take a look at this?

@averms averms self-assigned this Sep 18, 2021
@averms
Copy link
Collaborator

averms commented Sep 21, 2021

I quite like this. The attached image compares main with this change.

comparision

The color change can help people parse the pi example as a key inside a table rather than a decimal. It also helps with the quoted IP address. I also like that you used the display option so performance shouldn't be affected for people who aren't assigning different colors to tomlKey and tomlDottedKey.

EDIT: I also like this because matching a dot in the key will make it easier to fix #58.

@nastevens nastevens force-pushed the highlight-dotted-keys branch from 4d48298 to 3d10ab4 Compare September 29, 2021 00:31
@averms averms merged commit 25aba7e into cespare:main Oct 9, 2021
@averms
Copy link
Collaborator

averms commented Oct 9, 2021

Thanks for the patch!

@nastevens nastevens deleted the highlight-dotted-keys branch October 9, 2021 16:00
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.

3 participants