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

vi-mode: visual mode: no background for commands #165

Closed
blueyed opened this issue Apr 13, 2015 · 8 comments
Closed

vi-mode: visual mode: no background for commands #165

blueyed opened this issue Apr 13, 2015 · 8 comments
Labels

Comments

@blueyed
Copy link

blueyed commented Apr 13, 2015

When using vi-keybindings and entering visual mode (v), the visual selection gets a background color / standout mode, but this does not work with some parts when using zsh-syntax-highlighting, e.g. commands and invalid command:

In the below screenshot, the whole line is visually selected, but it is not visible for ls and lss:

selection_065

@rosshadden
Copy link

👍

@danielshahaf
Copy link
Member

In your example, printing $region_highlight from a widget shows:

region_highlight=( '0 2 fg=green' '3 6 underline' '7 10 none' '11 12 none' '13 16 fg=red,bold' )

Combining that with your screenshot shows reverse video only applies to the portions that have an fg=… attribute.

I can reproduce this without zsh-syntax-highlighting:

$ zsh -f
% bindkey -v
% f() { region_highlight=('0 2 fg=green' '2 4 underline') }
% zle -N f
% bindkey -a ^T f
% abcdef<Esc>v0<^T>    # cdef in reverse video, ab in green on normal-background-color

Should zsh turn the fg=green to bg=green? Or should everyone who sets $region_highlight check MARK and CURSOR and swap bg and fg? I would guess zsh itself should take care of applying reverse video. @blueyed @rosshadden Do you know? Maybe check with zsh-users@?

@blueyed
Copy link
Author

blueyed commented Aug 31, 2015

Should zsh turn the fg=green to bg=green?

Seems sensible in the case of reverse?!

@danielshahaf
Copy link
Member

Seems sensible in the case of reverse?!

It seems both you and I expect $region_highlight to be applied first, and the MARK..CURSOR region's highlighting to be applied on top of that. (Yes, the names are confusing.) However, it would not be unreasonable of zle to do things the other way around: first to apply the MARK..CURSOR region's highlighting and then to apply $region_highlight on top of that, so widgets can override the MARK..CURSOR region's highlighting.

The MARK..CURSOR's region highlighting, which defaults to standout (i.e., reverse video), is specified by $zle_highlight.

The man page is silent on the interaction between $zle_highlight and $region_highlight. I think that should be clarified by upstream. Once it is clarified whose responsibility it is to reverse the video of the ls and lss (in your example), that party — either zle or zsh-syntax-highlighting — can implement a fix.

Makes sense?

@danielshahaf
Copy link
Member

workers/29425 also discusses the interaction between $zle_highlight and $region_highlight.

@danielshahaf
Copy link
Member

jimmijj@ece762e fixes this for the special case that $zle_highlight contains an explicit region:… element. Therefore, a workaround is:

  1. Set the default explicitly: zle_highlight+=( region:standout )
  2. Cherry-pick jimmijj@ece762e.

So, I think the way forward is:

  1. Work with zsh-workers to document whether $region_highlight takes precedence over $zle_highlight (current behaviour: it does).
  2. Assuming the current behaviour becomes documented upstream, merge @jimmijj's patch, and improve it to have the code do the right thing even when $zle_highlight contains no region:… element, as is the default. (That patch is part of PR colors for files, command_prefix + more minor modifications #152.)

[ Update: started a zsh-workers thread. ]
[ Update: upstream has documented the behaviour in question. ]

@danielshahaf
Copy link
Member

Proposed fix: danielshahaf@0e31d6e

(It's the i165-region-v1 branch in my repository.)

@danielshahaf
Copy link
Member

Merged in 51102bf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants