Re: [PATCH v2 1/2] add-menu: added add-menu to lib objects

From: Junio C Hamano <gitster@pobox.com>
Date: May 23, 2022, 20:35 UTC
Message-ID: <xmqq1qwkq96f.fsf@gitster.g>
In-Reply-To: <8d483e1d-1865-c475-cbe3-78fa1f7f8bfb@github.com>

Derrick Stolee <derrickstolee@github.com> writes:

> The biggest test of your new API is whether it can
> support _both_ of these existing interactive menus
> before adding one to 'git stash'.

A great piece of advice.

Show 8 quoted lines

> One problem with this approach of adding the code to this
> new *.c file and then later removing the code from clean is
> that we lose 'git blame' or 'git log -L' history across the
> move. It's much harder to detect copies than to detect moved
> lines of code.
>
> I don't have a good solution in mind right now, but it's
> worth thinking about.

I actually do ;-)

One single preliminary patch can rename the helper functions, update the direct reference to the color configuration table into passing a parameter to the table, doing the same to the menu that defines the shorthand, help text, and implementation of the command, and any other necessary adjustment. Or if these tasks are turns out to be too large to do in a single commit, they can be done in steps. This preliminary refactoring patch (or a series of patches) can be done while everything is still in builtin/clean.c (or we could start by refactoring the one in add-interactive.c instead).

Then the second patch in the series would move the refactored library-ish code into the new interactive-menu.c file, and add the interactive-menu.h header for users to include. The first such user will be builtin/clean.c (or if we started from add-interactive.c, then that one). "blame", "log" and "diff --color-moved" would all notice that an already refactored block of code was lifted from one source file and moved to the new place in this step.

The third patch in the series would convert add-interactive.c (or if we started from it, then builtin/clean.c) to be the second user of the API. There might need a few preliminary steps in the file to be converted before it happens to match the function signatures, etc.

After that, "git stash" will have its own interactive mode that uses the new API from day one.