On Sun, Dec 6, 2020 at 5:58 PM brian m. carlson
<sandals@xxxxxxxxxxxxxxxxxxxx> wrote:
> [...]
> To help out the scripter, let's provide an option which turns most of
> the paths printed by git rev-parse to be either relative to the current
> working directory or absolute and canonical. Document which options are
> affected and which are not so that users are not confused.
> [...]
> Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx>
> ---
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> @@ -212,6 +212,15 @@ Options for Files
> +--path-format=(absolute|relative)::
> + Controls the behavior of certain other options following it on the command
> + line. If specified as absolute, the paths printed by those options will be
> + absolute and canonical. If specified as relative, the paths will be relative
> + to the current working directory if that is possible. The default is option
> + specific.
I read the commit message and looked at the implementation so I know
that this option can be specified multiple times, but this
documentation only vaguely hints at it by saying "options following
it". We could do a better job of imparting that knowledge to the
reader by saying so explicitly, perhaps like this:
Controls the behavior of some path-printing options. If
'absolute', [...]. If 'relative', [...]. May be specified multiple
times, each time affecting the path-printing options which
follow it. The default path format is option-specific.
Since this option can be specified multiple times, should it also
recognize `default` to request the default behavior in addition to
`absolute` and `relative`? (Genuine question. Maybe real-world
use-cases wouldn't need it, but it would be easy to support and make
it functionally complete.)
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> @@ -583,6 +583,73 @@ static void handle_ref_opt(const char *pattern, const char *prefix)
> +static void print_path(const char *path, const char *prefix, enum format_type format, enum default_type def)
> +{
> + struct strbuf buf = STRBUF_INIT, realbuf = STRBUF_INIT, prefixbuf = STRBUF_INIT;
> + if (!is_absolute_path(path)) {
> + strbuf_realpath_missing(&realbuf, path, 1, 1);
> + path = realbuf.buf;
> + }
> + if (!is_absolute_path(prefix)) {
> + strbuf_realpath_missing(&prefixbuf, prefix, 1, 1);
> + prefix = prefixbuf.buf;
> + }
> + puts(relative_path(path, prefix, &buf));
> + strbuf_release(&buf);
Leaking `realbuf` and `prefixbuf` here.
