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

From: Jeff King <peff@peff.net>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: "brian m. carlson" <sandals@crustytoothpaste.net>,
	Git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
	Emily Shaffer <emilyshaffer@google.com>,
	Aaron Schrab <aaron@schrab.com>,
	Denton Liu <liu.denton@gmail.com>,
	Christian Couder <christian.couder@gmail.com>
Subject: Re: [PATCH v2 1/2] Add project-wide .vimrc configuration
Date: Tue, 15 Dec 2020 00:28:26 -0500	[thread overview]
Message-ID: <X9hJenCYkwTmxNjA@coredump.intra.peff.net> (raw)
In-Reply-To: <5fd8279ce0696_d7c482087@natae.notmuch>

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

> 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