Re: [PATCH v2 1/2] Add project-wide .vimrc configuration

From: Jeff King <peff@peff.net>
Date: Dec 15, 2020, 05:28 UTC
Message-ID: <X9hJenCYkwTmxNjA@coredump.intra.peff.net>
In-Reply-To: <5fd8279ce0696_d7c482087@natae.notmuch>

On Mon, Dec 14, 2020 at 09:03:56PM -0600, Felipe Contreras wrote:

Show 21 quoted lines

> Jeff King wrote:
> > On Fri, Dec 11, 2020 at 02:56:22AM +0000, brian m. carlson wrote:
> 
> >   - it doesn't suggest any actions that might be bad practices. I agree
> >     that the instructions for auto-loading this .vimrc are more
> >     complicated than necessary and might have security implications.
> >     Carrying a file in contrib/vim that says "copy this to ~/.vim/foo"
> >     or even "copy these lines to your ~/.vimrc" seems a lot safer. And
> >     it makes it easier for people who prefer to adapt the config to
> >     their own setup.
> > 
> > So I'm not opposed to carrying some vim config, but I think it's best to
> > focus on simplicity and providing human-readable instructions, rather
> > than ad-hoc plugin infrastructure.
> 
> Generally I would agree, but do you know what such instructions would look like?
> 
> In particular what instructions would look like for a person that
> contributes to more than 3 projects with different C code-style.
> 
> I can assure they are anything but human-readable.

Mostly what I'm suggesting is asking the user to copy the settings they want, rather than sourcing a file in the repository that may contain arbitrary options. So something like:

  " Settings to match Git's style/indentation preferences.
  "
  " You can put these straight in your .vimrc if you want to use
  " them all the time. Or if you want to use them only inside
  " certain directories, wrap them like this:
  "   if match(getcwd(), "/path/to/your/git/repo")
  "      au Filetype c setl ...etc...
  "   endif
  "
  au FileType c setl ...etc...

That means they won't automatically pick up new options if they change, but that's the point. They should be inspecting and deciding which options they want to take.

The conditional above definitely has some flaws. It relies on the working directory rather than the location of the file (which is the same as your plugin; yours is just picking it up implicitly from calling git). And once the autoloaders are set up, I think they'd trigger for any C file, even outside the repository directory.

Ideally we'd combine the autoloader for BufRead and FileType, but it seems non-trivial to do so. I think:

  au BufNewFile,BufRead /path/to/git/* if &filetype == "c" | setl ... | endif

works, though it's a little clunky, as each line would need to repeat it. There might be a better way. I'm not that familiar with doing tricky things with vim's autoloading. But my point is mostly that the value in the information is saying "here are some useful vim settings you might want to use". I don't think we need to solve "here's how to trigger some settings for some directories" for everyone. We should let them integrate the settings as they see fit.

-Peff