Re: [PATCH 1/2] git.c: introduce --working-tree superseding --work-tree

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mar 20, 2017, 19:58 UTC
Message-ID: <20170320195818.GV26789@aiede.mtv.corp.google.com>
In-Reply-To: <20170320192225.18928-2-sbeller@google.com>

Hi,

Stefan Beller wrote:

Show 5 quoted lines

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  Documentation/git.txt | 12 ++++++------
>  git.c                 |  5 +++--
>  2 files changed, 9 insertions(+), 8 deletions(-)

I think this is a step in the right direction.  Thanks for writing it.

Nits:

- tests are still using --work-tree --- this patch didn't add any tests
  for --working-tree.  If --working-tree is what we prefer, it may make
  sense to update tests to use --working-tree and add a test or two to
  make sure the existing --work-tree synonym still works.

- this patch updated the argv[i] == "--work-tree" case but forgot to
  update the argv[i].has_prefix("--work-tree=") case

- since this is a feature used for scripting, I don't think we can
  pretend the name change never happened.  We think we need to
  document both option names and say what version introduced the new
  one so script authors can make an informed decision about which to
  use.  Later we can make the --work-tree synonym more obscure, but in
  the short term I suspect it is what most script authors will still
  want to use.

> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt

[...]

Show 7 quoted lines

> @@ -892,7 +892,7 @@ Git so take care if using a foreign front-end.
>  
>  `GIT_WORK_TREE`::
>  	Set the path to the root of the working tree.
> -	This can also be controlled by the `--work-tree` command-line
> +	This can also be controlled by the `--working-tree` command-line
>  	option and the core.worktree configuration variable.

I suspect we don't want to rename GIT_WORK_TREE --- it's not user-facing in the same way as --work-tree is, scripts make direct use of it (and they unset it when appropriate!), and dealing with the permutations of what to do if some subset of environment variables is set seems very complicated.

For comparison, core.worktree is user-facing. Is it also in scope for this change?

Thanks and hope that helps, Jonathan