tpope / vim-vinegar

vinegar.vim: Combine with netrw to create a delicious salad dressing
https://www.vim.org/scripts/script.php?script_id=5671
2.23k stars 87 forks source link

Keep custom g:netrw_list_hide intact #103

Open fedorenchik opened 6 years ago

fedorenchik commented 6 years ago

Fixes issue #102.

fedorenchik commented 6 years ago

Ping?

tpope commented 6 years ago

You can't just smash these two things together without adding a comma in between.

fedorenchik commented 6 years ago

Fixed, thanks for review.

kiryph commented 6 years ago

I just want to remind what tpope already said about g:netrw_list_hide:

Yes I just said I'm deliberately overriding you, and asked why you want to fight it. Vinegar's stance is that you should ignore files you never want to edit with 'wildignore' and leave everything else; otherwise inconsistencies arise. #29 g:netrw_list_hide in .vimrc is ignored

The variable g:netrw_list_hide is discussed in numerous issues:

There has been a pull request as well

fedorenchik commented 6 years ago

Well, I'm not changing 'wildignore' behaviour, you can check the patch. I'm just adding support for custom hide list. But maybe change 'wildignore' is better option after all. Will need to look into it.

kiryph commented 6 years ago

I know that your patch will still add 'wildignore' to g:netrw_list_hide but your patch removes the part

I'm deliberately overriding you.

I struggled with this myself and would not mind if vinegar lets people decide which way they want to configure their ignore list g:netrw_list_hide. The variable g:netrw_list_hide is documented in netrw

  *g:netrw_list_hide*       comma separated pattern list for hiding files
                Patterns are regular expressions (see |regexp|)
                There's some special support for git-ignore
                files: you may add the output from the helper
                function 'netrw_gitignore#Hide() automatically
                hiding all gitignored files.
                For more details see |netrw-gitignore|.

                Examples:
                 let g:netrw_list_hide= '.*\.swp$'
                 let g:netrw_list_hide= netrw_gitignore#Hide().'.*\.swp$'
                default: ""

Therefore user expect that they can use it this way. However, vinegar changes the use of this variable considerably: the only valid value is the magic string defined in s:dotfiles.

Sometimes I appreciate tpope's standpoint and active measures that users of his plugins are more or less forced to take his standpoint. However, he should properly document this. This would reduce the number of issues and pull requests.

Update Also I would have implemented the current behavior in vinegar more explicitly with a new boolean variable g:vinegar_hide_dot_files. Additionally documented that g:netrw_list_hide cannot be customized by the user anymore and printed a warning message if vinegar overwrites a non-empty g:netrw_list_hide.

tpope commented 6 years ago

Thank you @kiryph for tracking all those down, that was my next step.

One thing I can't stand is trying out a new Vim plugin and being rewarded by getting yelled at on every Vim startup about some stupid crap I don't care about. I also like that you can have a basic netrw config you keep in your vimrc and have it "enhanced" on machines with vinegar without getting screwed when running on a server or something without your full setup. Maybe there's some sort of middle ground here but I think the first step is indeed documentation, as you point out.

@fedorenchik do report back if you find a reason 'wildignore' won't work for you.

kiryph commented 6 years ago

Thank you @kiryph for tracking all those down, that was my next step.

you are welcome.

I also like that you can have a basic netrw config you keep in your vimrc and have it "enhanced" on machines with vinegar without getting screwed when running on a server or something without your full setup.

Now I understand your motivation for the implementation. But it could still be made more explicit with a check if g:netrw_list_hide is either empty or equal to s:dotfiles and if not, report to the user that the custom value will be dismissed and should use 'wildignore'.