Re: [PATCH 1/2] parse-options: expand $HOME on filename options

From: Jeff King <peff@peff.net>
Date: Feb 14, 2018, 14:05 UTC
Message-ID: <20180214140557.GA27850@sigill.intra.peff.net>
In-Reply-To: <20180214105149.28896-1-pclouds@gmail.com>

On Wed, Feb 14, 2018 at 05:51:48PM +0700, Nguyễn Thái Ngọc Duy wrote:

Show 8 quoted lines

> When you specify "--path ~/foo", the shell will automatically expand
> ~/foo to $HOME/foo before it's passed to git. The expansion is not done
> on "--path=~/foo". An experienced user sees the difference but it could
> still be confusing for others (especially when tab-completion still
> works on --path=~/foo).
> 
> Support $HOME expansion for all filename options. There are about seven
> of them.

I think this probably makes sense.

>  parse-options.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Should this be mentioned in the comment documenting OPT_FILENAME()?

Show 19 quoted lines

> diff --git a/parse-options.c b/parse-options.c
> index d265a756b5..c33f14c74e 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -38,10 +38,13 @@ static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
>  
>  static void fix_filename(const char *prefix, const char **file)
>  {
> -	if (!file || !*file || !prefix || is_absolute_path(*file)
> -	    || !strcmp("-", *file))
> +	if (!file || !*file || is_absolute_path(*file) ||
> +	    !strcmp("-", *file))
>  		return;
> -	*file = prefix_filename(prefix, *file);
> +	if (**file == '~')
> +		*file = expand_user_path(*file, 0);
> +	else if (prefix)
> +		*file = prefix_filename(prefix, *file);
>  }

I thought at first this needed a final "else" clause, because we don't assign to *file if we have neither a prefix nor a user-path. But that's what the callers expect (and we are similarly a noop if we hit the first conditional). So this looks right.

-Peff