Subject: Re: [PATCH v5 1/4] git: make super-prefix option

From: Junio C Hamano <[email-masked]>
Date: Wed, 28 Sep 2016 15:19:22 -0700

Brandon Williams <[email-masked]> writes:

> Add a super-prefix environment variable 'GIT_INTERNAL_SUPER_PREFIX'
> which can be used to specify a path from above a repository down to its
> root.  The immediate use of this option is by commands which have a
> --recurse-submodule option in order to give context to submodules about
> how they were invoked.  This option is currently only allowed for
> builtins which support a super-prefix.

Yes, it can be used to specify that, but it is unclear (1) how such
a path thusly specified is used, (2) how it affects the outcome of
the operations, and (3) what it means (the same applies to the
documentation update).

It would help future readers of "git log", "tig blame", etc. to have
a few sentences after the first sentence above.  Perhaps

    ... specify a path from above ... down to its root.  When such a
    super-prefix is specified, the paths reported by Git are
    prefixed with it to make them relative to that directory
    "above".  The paths given by the users on the command line
    (e.g. "git subcmd --output-file=path/to/a/file" and pathspecs)
    are taken relative to that directory "above" to match.

or something like that?

> @@ -468,6 +469,7 @@ extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);
>  extern int get_common_dir(struct strbuf *sb, const char *gitdir);
>  extern const char *get_git_namespace(void);
>  extern const char *strip_namespace(const char *namespaced_ref);
> +extern const char *get_super_prefix();

Nice, but 

    extern const char *get_super_prefix(void);

> +const char *get_super_prefix()

Likewise.

> +{
> +	if (!super_prefix)
> +		super_prefix = getenv(GIT_SUPER_PREFIX_ENVIRONMENT);
> +	return super_prefix;
> +}

Good.

>  	commit_pager_choice();
>  
> +	if (!help && get_super_prefix()) {
> +		if (!(p->option & SUPPORT_SUPER_PREFIX))
> +			die("%s doesn't support --super-prefix", p->cmd);
> +		if (prefix)
> +			die("can't have both a prefix and super-prefix");
> +	}

Nice.  I'd phrase the latter as "can't use--super-prefix from a
subdirectory", though.