justinmk / vim-dirvish

Directory viewer for Vim :zap:
Other
1.19k stars 64 forks source link

Fix cosmetic/usability bug with marking paths. #123

Closed lifepillar closed 5 years ago

lifepillar commented 6 years ago

This commit addresses https://github.com/justinmk/vim-dirvish/issues/76 and more specifically, this comment: https://github.com/justinmk/vim-dirvish/issues/76#issuecomment-304442570. It likely needs some cleanup, but it basically works.

A few notes:

lifepillar commented 6 years ago

There likely should be a syntax clear DirvishFullPath DirvishArg before defining those highlight groups, to avoid duplication of rules.

justinmk commented 6 years ago

@lifepillar Thank you! This was very bothersome. I think I can finally tag 1.0 after this.

It seems to work. LMK if you're at a stopping point.

justinmk commented 6 years ago

I am not sure whether \v has any effect in syntax rule, as the manual says that syntax patterns are always magic (:h syn-pattern).

Yeah that's left over from before I knew better. I think \v should not be used.

Syntax rules for DirvishFullPath and DirvishArg must follow the other rules, otherwise they may be overriden (see :h syn-priority).

Good call, should put that comment in the code.

I am not sure what the place for the condition testing for b:current_syntax is. That might need to be changed.

Yes that was to avoid duplicate rules.

lifepillar commented 6 years ago

I've left \v and the current syntax check untouched, 'cause they are not relevant to this PR.

justinmk commented 5 years ago

There likely should be a syntax clear DirvishFullPath DirvishArg before defining those highlight groups, to avoid duplication of rules.

Can't do that (AFAIK) because other windows (showing the same Dirvish buffer) may have their own local arglists which used the existing buffer-local :syntax match rules. That's what the "Define (again) ..." comment refers to.

However, it's not a big problem, a Refresh clears the redundant syntax definitions for example.

I am not sure what the place for the condition testing for b:current_syntax is.

It's to avoid redundant definitions for DirvishPathHead etc. I changed it as follows:

" Define once (per buffer).
if !exists('b:current_syntax')
  exe 'syntax match DirvishPathHead =\v.*\'.s:sep.'\ze[^\'.s:sep.']+\'.s:sep.'?$= conceal'
  exe 'syntax match DirvishPathTail =\v[^\'.s:sep.']+\'.s:sep.'$='
  exe 'syntax match DirvishSuffix   =[^\'.s:sep.']*\%('.join(map(split(&suffixes, ','), s:escape), '\|') . '\)$='
endif
kristijanhusak commented 5 years ago

@justinmk i have one question kinda related to this. Would you agree on support for custom columns/flags/whatever you want to call it, per single path, something like this https://github.com/Xuyuanp/nerdtree-git-plugin? as @lifepillar noted, it's really hard to work with these paths and concealing. In my git plugin for divish i somehow managed to conceal these paths, but it's really messy. It would simplify managing these things, but i assume it would complicate handling paths for other things.

justinmk commented 5 years ago

@kristijanhusak Left-side column would be possible after https://github.com/justinmk/vim-dirvish/issues/70 is addressed, because that will entail adding some whitespace for indentation. Though I don't plan to work on it anytime soon.

Right-side "column" can be achieved with "virtual text" which is supported by Nvim 0.3.2 (prerelease), and I assume Vim will eventually have a similar feature. Anything else will be a mess.

If there's some way to get left-side column support before https://github.com/justinmk/vim-dirvish/issues/70 , without a ton of code, I'd be in favor.

justinmk commented 5 years ago

Merged, thanks @lifepillar . I was also able to simplify it in 9c67fa7729fd.