threads / rfc / 45845

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

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

## tl;dr

3 messages between May 1, 2017 and May 2, 2017.

replies: 2people: 2as markdown or json

Daniel Ferreira (theiostream)· May 1, 2017, 22:46 UTC · lore
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/
Junio C Hamano· May 2, 2017, 00:32 UTC · re: Daniel Ferreira (theiostream) · lore

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

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

Junio C Hamano· May 2, 2017, 04:20 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 14 quoted lines
> "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.

← back to recent threads