Jeff King <peff@xxxxxxxx> writes:
>> 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()?
Perhaps. I think all mention of "$HOME expansion" should be
replaced with "tilde expansion", though. I first thought we are
expanding any environment variable and $HOME is merely an example of
it when I read the title and the log message, before seeing that the
patch just adds a call to expand_user_path().
Other than that, looks good. Thanks for a quick enhancement and a
review.
>> 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
