# [RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C 3 messages from 2017-05-01 to 2017-05-02. Participants: Daniel Ferreira (theiostream), Junio C Hamano. Thread: https://gitlist.dev/t/45845 ## Daniel Ferreira (theiostream), 2017-05-01 22:46 Subject: [RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C Message-ID: URL: https://gitlist.dev/e/CAEA2_RLvjRDytE03hfe84TTqvCQm324BXV%3DGvTU3UVqNWJUXiQ%40mail.gmail.com ``` 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 " (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, 2017-05-02 00:32 Subject: Re: [RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C Message-ID: URL: https://gitlist.dev/e/xmqqo9vcf68f.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` "Daniel Ferreira (theiostream)" 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, 2017-05-02 04:20 Subject: Re: [RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C Message-ID: URL: https://gitlist.dev/e/xmqqvapjdh3x.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Junio C Hamano writes: > "Daniel Ferreira (theiostream)" 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. ```