glynnforrest / salt-mode

Emacs major mode for Salt States
GNU General Public License v3.0
22 stars 5 forks source link

Provide more information via font-lock. #6

Closed joewreschnig closed 7 years ago

joewreschnig commented 7 years ago

This adds special faces for Salt highstate keywords, requisite properties, state functions, and top-level state IDs.

Because parsing SLS files is hard, it doesn’t yet handle all cases. Most notably state IDs in “extend:” declarations are not highlighted correctly. However, it can stop you from typing “requires” where you meant “require:”.

joewreschnig commented 7 years ago

Before: before

After: after

(This is after loading then disabling my custom theme, so I have no idea what state the active faces are really in...)

glynnforrest commented 7 years ago

Thank you @joewreschnig for another great contribution. This is a great improvement to the non-existent font-locking we have currently.

An issue to consider with salt-mode-state-id-face is the alternative state syntax, for example where the managed in file.managed goes to another line:

alternative syntax

A tricky problem to solve I think, given how flexible the yaml state syntax can be. I personally don't use or like this style, but I have seen people write states that way before.

Do you have any thoughts about detecting pillar, top, and orchestrate files? Perhaps the mode could become aware of the filesystem layout. By default it could assume that if the current file matches the path pillar/, top.sls, reactor/, etc, it will use different font locking.

I've added you to the list of contributors in the README, let me know if you want to put a link to somewhere other than your github profile.

joewreschnig commented 7 years ago

SLS file syntax is really quite a mess between YAML and Jinja, and to make matters worse yaml-mode offers us very little help. I think path/filename matching is probably "good enough." I also think top files are "good enough" for now, the names of the faces don't match the real top file semantics but we also don't provide a real syntax table anyway so eh. Pillars may just be best with these new faces disabled, since they're basically straight maps. I don't have enough experience with orch/reactor files to say what's ideal for those; maybe in a few months I'll have an opinion.

Splitting the function name seems to be more an accident of the highstate compilation process than a feature anyone is really happy with. It would really complicate other additions I'm working on (e.g. eldoc hints), and I'd rather explicitly not support it.

glynnforrest commented 7 years ago

Agreed on all points. I think incremental improvements to the highlighting is the way to go. Happy to not support splitting the function name.

What other features are you working on, aside from eldoc hints? I'd like to add some functionality myself, but only if it won't duplicate your efforts.

joewreschnig commented 7 years ago

My rough plan (based on my perception of where I waste time making SLS files) is:

But I'm pressed for time beyond the eldoc work, so if you have ideas go ahead.