Re: [GSoC Patch v6 1/4] path: introduce append_formatted_path() for shared path formatting

From: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
Date: Jun 20, 2026, 16:30 UTC
Message-ID: <CA+rGoLcibxaEs7KzS8a=A9kxV8+3KCqVXOK+zoiFtNvJkVHvCA@mail.gmail.com>
In-Reply-To: <xmqqbjd5guci.fsf@gitster.g>

Hi Junio,

Show 6 quoted lines

> It often, even though not always, is a sign of a bad topic structure
> to have an insertion-only patch without any removal of existing
> code, that adds totally unused code.
>
> If the step is to "extract the core algorithm", shouldn't it be able
> to replace existing code already?

Giving the helper and converting its first caller (`rev-parse`) in the same step proves the implementation avoids leaving unused code lingering in the tree, even temporarily.

Show 10 quoted lines

> We may want to add new features to this helper function near the end
> of the topic, but wouldn't it make sense for the topic to first
> consolidate various path formatting logic already present in the
> existing code into a single helper for ease of extending it (which
> means replacing open-coded logic in existing code paths with a call
> to the new helper, which would have a code that may look very
> similar to the original code that was replaced with a single call to
> the helper function), and then expose the helper for use by new
> callers, and finally further add new features that existing code
> paths wouldn't have needed but the new callers would want?

Consolidating the existing logic first ensures we aren't introducing unnecessary complexity up front. I agree with restructuring the topic this way.

Show 5 quoted lines

> How else can we make sure this new implementation added by the first
> step in the series is (1) capable enough to reproduce what we
> already have in different parts of the system, (2) does not bring in
> what the current codebase does not need, and (3) bug-to-bug
> compatible with the existing code paths?

Introducing the helper and swapping out the `rev-parse` implementation in the same step is the best way to prove bug-to-bug compatibility and demonstrate its immediate utility.

For v7, I will squash patches 1 and 2 together so that the extraction and the replacement happen simultaneously, guaranteeing that the new `append_formatted_path()` perfectly mirrors the old behavior before we introduce the new `path.*` callers.

Thanks for taking the time to explain the rationale!

- K Jayatheerth