[RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C

[RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C

[Daniel Ferreira (theiostream)]

Hey there,

So, in the GSoC proposal I sent about porting git-add--interactive to
C[1], I expected I would be able to do a couple of small patches to
git-add to familiarize myself with the Git API and have a better clue
of how the porting process would go by. Due to the unexpected size my
microproject ended up taking (though in a good way, I think) I chose
to focus on that and ended up not sending those anticipated patches.
However, I *did* have time to study how I'd go for the port more
closely, and I'd like some comments on what you think of it. Be I
accepted or not into GSoC some days from now, I think the topic might
interest others that might want to tackle this task.

As I went through the code, I grew to believe that a good way to make
a step-by-step conversion might be to get add--interactive "commands"
(as in status_cmd, update_cmd, add_untracked_cmd etc.) ported one at a
time over a set of patch series. For each series, one X_cmd Perl
subroutine would become a one-line call to
"git-add-interactive--helper X <args>" (a builtin). This would make
understanding a function as complex as list_and_choose() to being
ported to C way easier: for each command, a new functionality would be
added on-demand instead of having the whole function sent over at
once. Afterwards, a final series would likely set up the "interactive
main loop" in C and retire the Perl script.

I tried to apply this philosophy to porting `status_cmd` to C, and
easily got most of the configuration/color logic into C by mimicking
what builtins like clean.c do.

The next issue I came up with (and do not yet have a solution) is how
reproduce git-add--interactive's list_modified(). The Perl script runs
git-diff-index and git-diff-files, does some clever string parsing of
their outputs and "merges" the output of both in a single table to
display the number of added/removed lines in a given file BOTH between
HEAD+index and index+tree.

Reproducing either of these comparisons "natively" would simply
require running run_diff_index() or run_diff_files() with
DIFF_FORMAT_NUMSTAT and tweaking diff_flush() to format appropriately
for the "interactive--add case". Furthermore, the current output
prefix options would manage to cover the eventual case of `update_cmd`
selecting files. To output both comparisons, however, I can't come up
with any simple solution. Would you have any ideas?

-- Daniel.

[1]: https://public-inbox.org/git/CAEA2_RJEf4vjgcaux8a1KWh1-vxLLmv1--Vjf9wiEQoF+gVDtA@mail.gmail.com/

Re: [RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C

[Junio C Hamano]

"Daniel Ferreira (theiostream)" <bnmvco@gmail.com> writes:

> Reproducing either of these comparisons "natively" would simply
> require running run_diff_index() or run_diff_files() with
> DIFF_FORMAT_NUMSTAT and tweaking diff_flush() to format appropriately
> for the "interactive--add case".

A more usual way to drive the diff machinery and react to its
findings is to use DIFF_FORMAT_CALLBACK interface, and walk the
collected diff_queue to work on the paths discovered.  The way
wt-status.c builds "Changes to be committed" list out of the
"diff-index --cached" it internally runs and "Changes not staged for
commit" out of the "diff-files" it internally runs would be a good
example to study.

Re: [RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Daniel Ferreira (theiostream)" <bnmvco@gmail.com> writes:
>
>> Reproducing either of these comparisons "natively" would simply
>> require running run_diff_index() or run_diff_files() with
>> DIFF_FORMAT_NUMSTAT and tweaking diff_flush() to format appropriately
>> for the "interactive--add case".
>
> A more usual way to drive the diff machinery and react to its
> findings is to use DIFF_FORMAT_CALLBACK interface, and walk the
> collected diff_queue to work on the paths discovered.  The way
> wt-status.c builds "Changes to be committed" list out of the
> "diff-index --cached" it internally runs and "Changes not staged for
> commit" out of the "diff-files" it internally runs would be a good
> example to study.

Ahh, you'd also want to show the numstat equivalent and eventually
you'd need to spool the actual textual diff in-core, so that you can
present hunks to the user and have them accepted/rejected
interactively. CALLBACK interface is not a good match for that task.

Of course, using a temporary file to buffer output from diff would
be an obvious and simple workaround (and this is not a performance
critical part of the system---we are talking about end user choosing
hunks interactively after all).  A cleaner may be to spawn diff-index
and diff-files and read from them via pipe into core.