search

zsh-users / zsh-syntax-highlighting

Fish shell like syntax highlighting for Zsh.
github.com/zsh-users/zsh-syntax-highlighting
BSD 3-Clause "New" or "Revised" License
20.2k stars 1.33k forks source link

pattern: user interface change #304

Open phy1729 opened 8 years ago

phy1729 commented 8 years ago

341a3ae introduced the _zsh_highlight_add_highlight function and changed all highlighters except pattern to use it. For the pattern highlighter to use _zsh_highlight_add_highlight, the interface would have to look something like

# To have commands starting with `rm -rf` in red:
ZSH_HIGHLIGHT_PATTERNS+=('rm -rf *' rm-rf)
ZSH_HIGHLIGHT_STYLES[rm-rf]='fg=white,bold,bg=red'

that is the values in ZSH_HIGHLIGHT_PATTERNS are keys for ZSH_HIGHLIGHT_STYLES. This isn't a trivial change as we're requiring people to do one of the two hard things[1] in computer science: naming things.

This wouldn't be so bad if a dozen or so people used the pattern highlighter, but a quick search shows the number is closer to 500. Further, users don't gain much (if anything) from this change aside from additional lines in their config files.

A few workarounds have been suggested. The first warns the user and drops the style from the user's running configuration which breaks existing dotfiles. Having the else branch silently promote the style in ZSH_HIGHLIGHT_PATTERNS to the key and value in ZSH_HIGHLIGHT_STYLES would not break existing dotfiles, but would not behave correctly under the injective-test branch and feels like a temporary measure that will never be removed. A ZSH_HIGHLIGHT_PATTERNS2 could be introduced with the new semantics, but converting back to ZSH_HIGHLIGHT_PATTERNS would introduce another interface change.

Finally, since the style names for ZSH_HIGHLIGHT_PATTERNS are user defined and refer to keys in ZSH_HIGHLIGHT_STYLES, it is impossible to guarantee forward compatibility. From a compatibility standpoint, it's best for user defined patterns to stay contained in their own variable where they can't collide with future ZSH_HIGHLIGHT_STYLES keys.

For that reason I'm now of the opinion that the pattern highlighter should stay as is even if it mean it will be the odd one out.

[1]: The other of course being cache invalidation and off by one errors.

danielshahaf commented 8 years ago

Not using _zsh_highlight_add_highlight, and being different from all other highlighters, currently only affects the test suite. However, it also represents technical debt.

I propose we accept this technical debt: if down the road this legacy behaviour makes some future change more complicated when it should be, we can then fix this issue. This issue shouldn't be any more complicated to fix then than now, since we have to be backwards-compatible anyway.