whatyouhide / vim-lengthmatters

Highlight the flooding part of an overly long line 📐
Do What The F*ck You Want To Public License
83 stars 12 forks source link

Not working inside specific buffers alone #14

Closed oblitum closed 8 years ago

oblitum commented 9 years ago

Per docs I expected activation was per buffer but whenever I open a file that activates it and then switch to a buffer where it wasn't previously activated, it stays active.

My configuration on .vimrc is this:

" lengthmatters Setup {{{
let g:lengthmatters_on_by_default = 0
call lengthmatters#highlight_link_to('Visual')
au FileType c,cpp,go :LengthmattersEnable
" }}}

Is there any way to have it activated solely for those file types?

Strangely this configuration enables lenghmatters even when opening a file whose file type is not in that list. This happens for example, when I open a .swift file it's disabled, then I open my .vimrc and it gets enabled.

UPDATE

My configuration is like this now instead:

" lengthmatters Setup {{{
let g:lengthmatters_excluded = [
    \ '', 'qf', 'help', 'unite', 'tagbar', 'gundo', 'vimshell', 'vimfiler',
    \ 'quickrun', 'python', 'html', 'javascript', 'css', 'scss', 'latex',
    \ 'haskell', 'purescript', 'elm'
    \ ]
" }}}
whatyouhide commented 9 years ago

Hey @oblitum, the docs appear to be out of date actually. If you go through the README, it says that the functionality operates on a per-window basis, not a per-buffer one. I haven't worked on this plugin for a long time and moved to Emacs a while back, so I don't remember the reason why I chose windows over buffers but I remember making this decision for a reason. A PR updating the docs would be very welcome. Wrt the feature you're asking (filetype-based activation) I'm afraid it won't be implemented by me for quite some time (a lot of other priorities and again, I'm not using vim anymore so it would take some additional time). A PR is very very welcome on this as well :) Let me know if I can help in any other way!

oblitum commented 9 years ago

Thanks for the response. Sad news, I believe I'll move to another option since this one has no maintainer anymore ( priorities, priorities, ... also for me =/ ). Thanks anyway.

oblitum commented 8 years ago

@whatyouhide I have turned this issue into a pull request. It seems it was a minor bug and the provided change fixed it.

oblitum commented 8 years ago

Ah, I've noticed with this fix :LengthmattersDisable is not respected when switching buffers.

oblitum commented 8 years ago

To clarify the problem with Lengthmatters I'll quote part of a conversation I was having with @scps950707:

To have a better understanding of the issue you may take a look at my Lengthmatters configuration:

" lengthmatters Setup {{{
let g:lengthmatters_match_name = 'Visual'
let g:lengthmatters_excluded = [
    \ '', 'qf', 'unite', 'tagbar', 'gundo', 'vimshell', 'vimfiler', 'help',
    \ 'python', 'html', 'javascript', 'css', 'scss', 'latex', 'haskell',
    \ 'purescript', 'elm'
    \ ]
" }}}

Notice that I have several files where I don't want Lengthmatters enabled, but this is not respected by original Lengthmatters.

If I launch Vim by opening one file of type in that list, Lengthmatters will correctly not be active, but then, with Vim still open, if I open another file that is not in lengthmatters_excluded, Lengthmatters will stay innactive for that file too, which is incorrect.

If I do the contrary, start Vim by opening one file of type that is not in that list, Lengthmatters will be correctly active, but then, with Vim still open, if I open a file to be excluded by Lengthmatters, Lengthmatters incorrectly stays active.

oblitum commented 8 years ago

The behaviour now is fixed. The remaining test not passing on Travis is now in an inconsistent state because now highlighting is disabled for g:lengthmatters_excluded files on s:AutocmdTrigger() event but w:lengthmatters_active is left in previous state on purpose. This is done because now w:lengthmatters_active is being used to better track disabling by user interaction through :LengthmattersDisable/:LengthmattersEnable.

w:lengthmatters_active is a flag conveying whether Lengthmatters is activated, regardless whether there's no highlighting inside the current buffer because of its presence on g:lengthmatters_excluded.

Not sure what to do about this test. Maybe a better rewriting of the code instead? I'm not sure but now at last it's working as I wanted.

oblitum commented 8 years ago

Also regarding

so I don't remember the reason why I chose windows over buffers but I remember making this decision for a reason

I suspect you have chosen so because matchadd is a command to create matches for the window, and this constrained the surrounding code to work in the context of windows. Because of this suspicion, I've tried not to look for a fix that changed variable types.

whatyouhide commented 8 years ago

@oblitum hey there, thanks for looking into this. As I said above I haven't looked at this plugin in a long time and I haven't got it "loaded" in my mind, so I'm a bit lost as for what's going on. But since you seem to know what you're doing, I'll trust you when you'll feel this is ready to merge so let me know :)

oblitum commented 8 years ago

@whatyouhide the solution of this issue is a bit difficult to "get", but as far as I've tested, it's fixed. The only thing left is to remove that test because now, no-highlight is not the same thing as not-active, it can be active while still having excluded files not highlighted. So, if you wish I can remove that test, so all the other remaining tests will still pass, and you can merge the pull.

whatyouhide commented 8 years ago

@oblitum sounds like a good plan 👍

oblitum commented 8 years ago

@whatyouhide ok, it's ready.

whatyouhide commented 8 years ago

Thanks @oblitum 💟

oblitum commented 8 years ago

@whatyouhide 👍

Weird how GitHub merged this. Stashed to a single commit & destroyed my commit comments.

whatyouhide commented 8 years ago

@oblitum I squashed, not GitHub :) I'm pretty picky about commits and I feel this change is fine as just one commit, sorry if it caused any harm :)

oblitum commented 8 years ago

@whatyouhide ok np then.