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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Derrick Stolee <derrickstolee@xxxxxxxxxx> 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.

> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux