git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Michael J Gruber <drmicha@warpmail.net>
Cc: Michael J Gruber <git@drmicha.warpmail.net>,
	David Pisoni <dpisoni@gmail.com>,
	GIt Mailing List <git@vger.kernel.org>,
	Git Maintainer <gitster@pobox.com>
Subject: Re: RFC proposal: set git defaults options from config
Date: Mon, 16 May 2011 07:02:56 -0400	[thread overview]
Message-ID: <20110516110256.GB23889@sigill.intra.peff.net> (raw)
In-Reply-To: <4DCBF01F.9040009@warpmail.net>

On Thu, May 12, 2011 at 04:35:11PM +0200, Michael J Gruber wrote:

> Mechanism
> =========
> 
> I propose the following mechanism for setting default command line
> options from the config:
> 
> options.<cmd> = <value>
> 
> is a "multivar" in git-config speak, i.e. it can appear multiple times.
> When running "git <cmd> <opts>", our wrapper executes
> 
> git <cmd> <values> <opt>
> 
> where <values> is determined by the following rule in pseudocode:
> 
> if $GIT_OPTIONS_<cmd> is unset:
>   <values> := empty
> else:
>   for <value> in $(git config --get-all options.cmd):
>     if <value> matches the regexp in $GIT_OPTIONS_<CMD>:
>       append <value> to <values>

As a user, how would I active this for all commands when not running a
script? I see why you defensively say "if unset, don't enable this
feature at all".  As a user, should I have to set GIT_OPTIONS_CMD for
everything that I want to configure? I hope not.

I think we need one extra variable to say generally "I am in strict
plumbing mode" or "I am in user mode". So you would want something like:

  if $GIT_STRICT is unset:
    <values> := $(git config --get-all options.cmd)
  else if $GIT_OPTIONS_<cmd> is unset:
    <values> := empty
  else:
    [match values by regex as you do]

But then you have a question of when GIT_STRICT gets set. An obvious
place is to set it in the git wrapper, so that "git foo" will have its
subcommands properly strict.

But that doesn't help scripts which are not called from the git wrapper;
they need to set GIT_STRICT themselves, so we need a phase-in period for
them to do so.

> NOTES
> =====
> 
> * This can be done by commit_pager_choice() or by a call right after
> that in those places.

Ah, so reading this, I have a sense that you were intending to make the
equivalent of GIT_STRICT be "am I running a pager" (or "am I outputting
to a terminal)?

Which is somewhat safer, as it is purely something for programs to opt
into. And as a heuristic, it's mostly good. I can come up with examples
where a script might not want to allow some options to be passed, even
though output is to the user, but they are probably stretching (e.g.,
something like "--allow-textconv" in a script that is meant to restrict
the users rights).

> * regexp notation/version to be decided

I think I would just as soon have a list of allowed options. We're
hopefully not doing the regex over the value of the option, like
"--pretty=foo is OK, but --pretty=bar is not". It seems like this
unnecessarily complicate the common case (you don't care what the value
is, but you have to tack on (|=.*) to every option matcher), and the
added flexibility is probably not going to be useful.

So I expect options regex are just going to look like:

  --(foo|bar|baz|bleep)

at which point we might as well just make it a list. And for the sake of
sanity, we may want to provide some default lists for scripts to OK,
like some minimal set of rev limiting options or something, so that
scripts don't end up specifying the same sets over and over.

> * We should probably do this for long options only (and insert
> "--<value>" rather than "<value>" to spare the "--" in config).

Yeah. Anything that doesn't have a long option and is useful enough to
be used in this way should probably get one.

> Taking cover...

I dunno. It's not so bad. But I think we probably want to start with an
environment variable to say "I am a script, be strict", let scripts
start picking that up, and then phase in the ability to turn on options
selectively.

-Peff

  parent reply	other threads:[~2011-05-16 11:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-11 22:57 [PATCH] Adds 'stash.index' configuration option David Pisoni
2011-05-11 23:46 ` Junio C Hamano
2011-05-12  0:26   ` Junio C Hamano
2011-05-12  0:48     ` David Pisoni
2011-05-12  0:54       ` Junio C Hamano
2011-05-12  7:14 ` Michael J Gruber
2011-05-12  8:04   ` Jeff King
2011-05-12  8:14     ` Michael J Gruber
2011-05-12  8:22       ` Jeff King
2011-05-12 14:35         ` RFC proposal: set git defaults options from config Michael J Gruber
2011-05-12 22:36           ` David Pisoni
2011-05-16 11:05             ` Jeff King
2011-05-16 11:02           ` Jeff King [this message]
2011-05-16 12:54             ` Michael J Gruber

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110516110256.GB23889@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=dpisoni@gmail.com \
    --cc=drmicha@warpmail.net \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).