Re: [PATCH v3 0/5] mingw: handle absolute paths in expand_user_path()

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 26, 2021, 17:56 UTC
Message-ID: <xmqqk0ldj6w3.fsf@gitster.g>
In-Reply-To: <pull.66.v3.git.1627164413.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

>  * Since our convention of %(...) interpolation does not involve uppercase
>    keywords, I now use a lowercase one.

Makes sense.

>  * Since this keyword is interpolated to the compiled-in prefix if built
>    without runtime prefix support, I dropped the runtime part of the
>    keyword.

I have this nagging feeling that %(prefix) may be (mistakenly) expected to interporate to $(git rev-parse --show-prefix). Of course, nobody would expect that in paths in the configuration files, but because we are borrowing %(token) convention from elsewhere, it is not outragous to imagine that either "for-each-ref" family or "log" family of placeholders may want to use %(prefix)" for such purpose (for that matter, they may also be helped to have the runtime-prefix information available).

Perhaps %(installPrefix) or something may have less chance of making us regret later. I am just raising this as a possible problem; I personally would not be confused if we settled on the %(prefix).

>  * Renamed the expand_user_path() to interpolate_path(), to remove the
>    distraction as to the implementation detail which things get to be
>    interpolated (because we extend it to interpolate more than just a home
>    directory, which might well be unclear from the former name, anyway).

Great.

>  * Adjusted the code comment above the interpolate_path() to remove a stale
>    part, clarify another part, and to extend it to talk about the prefix
>    expansion, too.

These looked good, too.

Thanks.