Re: [PATCH 2/5] parse: add git_parse_maybe_pathname()
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 11, 2026, 12:13 UTC
- Message-ID
- <aYxyXlH5Z0toWgPj@pks.im>
- In-Reply-To
- <8d3a6a8265714c5e4bae0f2e5a587ea46a6adddc.1770698579.git.gitgitgadget@gmail.com>
On Tue, Feb 10, 2026 at 04:42:56AM +0000, Derrick Stolee via GitGitGadget wrote:
Show 5 quoted lines
> From: Derrick Stolee <stolee@gmail.com> > > This extraction of logic from config.c's git_config_pathname() allows > for parsing a fully-qualified path from a relative path along with > validation of the existence of the path without failing with a die().
That sentence is quite something. I had to read it thrice to understand what it wants to say :)
Show 31 quoted lines
> diff --git a/parse.c b/parse.c
> index 48313571aa..3f37f0b93a 100644
> --- a/parse.c
> +++ b/parse.c
> @@ -209,3 +210,26 @@ unsigned long git_env_ulong(const char *k, unsigned long val)
> die(_("failed to parse %s"), k);
> return val;
> }
> +
> +int git_parse_maybe_pathname(const char *value, char **dest)
> +{
> + bool is_optional;
> + char *path;
> +
> + if (!value)
> + return -1;
> +
> + is_optional = skip_prefix(value, ":(optional)", &value);
> + path = interpolate_path(value, 0);
> + if (!path)
> + return -1;
> +
> + if (is_optional && is_missing_file(path)) {
> + free(path);
> + *dest = NULL;
> + return 0;
> + }
> +
> + *dest = path;
> + return 0;
> +}Okay. So the difference is that this function here doesn't cause us to die in case the path is not marked as optional and missing. Makes sense.
Show 9 quoted lines
> diff --git a/parse.h b/parse.h > index ea32de9a91..4f97c3727a 100644 > --- a/parse.h > +++ b/parse.h > @@ -19,4 +19,6 @@ int git_parse_maybe_bool_text(const char *value); > int git_env_bool(const char *, int); > unsigned long git_env_ulong(const char *, unsigned long); > > +int git_parse_maybe_pathname(const char *value, char **dest);
I think this function could use some explanation what it actually does, as the behaviour is non-trivial:
- I think the ":(optional)" part needs to be documented properly to
say that we return successfully with a NULL string in case the
target path doesn't exist. - We should document that it expands "~" and "%(prefix)" (even though
the latter feels somewhat coincidental to me).- The path is not resolved to an absolute path.
Thanks!
Patrick