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

Jeff King [off-list ref] writes:

quoted

Support $HOME expansion for all filename options. There are about seven
of them.

I think this probably makes sense.

quoted

 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.

quoted

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