{"thread":{"id":"45845","subject":"[RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C","startedAt":"2017-05-01T22:47:42Z","lastAt":"2017-05-02T04:20:56Z","messageCount":3,"participants":["Daniel Ferreira (theiostream)","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"318420","messageId":"CAEA2_RLvjRDytE03hfe84TTqvCQm324BXV=GvTU3UVqNWJUXiQ@mail.gmail.com","threadId":"45845","inReplyTo":null,"subject":"[RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C","fromName":"Daniel Ferreira (theiostream)","fromEmail":"bnmvco@gmail.com","sentAt":"2017-05-01T22:46:59Z","receivedAt":"2017-05-01T22:47:42Z","isPatch":false,"sender":{"key":"bnmvco@gmail.com","avatar":"https://avatars.githubusercontent.com/u/735397?v=4"},"body":"Hey there,\n\nSo, in the GSoC proposal I sent about porting git-add--interactive to\nC[1], I expected I would be able to do a couple of small patches to\ngit-add to familiarize myself with the Git API and have a better clue\nof how the porting process would go by. Due to the unexpected size my\nmicroproject ended up taking (though in a good way, I think) I chose\nto focus on that and ended up not sending those anticipated patches.\nHowever, I *did* have time to study how I'd go for the port more\nclosely, and I'd like some comments on what you think of it. Be I\naccepted or not into GSoC some days from now, I think the topic might\ninterest others that might want to tackle this task.\n\nAs I went through the code, I grew to believe that a good way to make\na step-by-step conversion might be to get add--interactive \"commands\"\n(as in status_cmd, update_cmd, add_untracked_cmd etc.) ported one at a\ntime over a set of patch series. For each series, one X_cmd Perl\nsubroutine would become a one-line call to\n\"git-add-interactive--helper X <args>\" (a builtin). This would make\nunderstanding a function as complex as list_and_choose() to being\nported to C way easier: for each command, a new functionality would be\nadded on-demand instead of having the whole function sent over at\nonce. Afterwards, a final series would likely set up the \"interactive\nmain loop\" in C and retire the Perl script.\n\nI tried to apply this philosophy to porting `status_cmd` to C, and\neasily got most of the configuration/color logic into C by mimicking\nwhat builtins like clean.c do.\n\nThe next issue I came up with (and do not yet have a solution) is how\nreproduce git-add--interactive's list_modified(). The Perl script runs\ngit-diff-index and git-diff-files, does some clever string parsing of\ntheir outputs and \"merges\" the output of both in a single table to\ndisplay the number of added/removed lines in a given file BOTH between\nHEAD+index and index+tree.\n\nReproducing either of these comparisons \"natively\" would simply\nrequire running run_diff_index() or run_diff_files() with\nDIFF_FORMAT_NUMSTAT and tweaking diff_flush() to format appropriately\nfor the \"interactive--add case\". Furthermore, the current output\nprefix options would manage to cover the eventual case of `update_cmd`\nselecting files. To output both comparisons, however, I can't come up\nwith any simple solution. Would you have any ideas?\n\n-- Daniel.\n\n[1]: https://public-inbox.org/git/CAEA2_RJEf4vjgcaux8a1KWh1-vxLLmv1--Vjf9wiEQoF+gVDtA@mail.gmail.com/\n"},{"id":"318433","messageId":"xmqqo9vcf68f.fsf@gitster.mtv.corp.google.com","threadId":"45845","inReplyTo":"CAEA2_RLvjRDytE03hfe84TTqvCQm324BXV=GvTU3UVqNWJUXiQ@mail.gmail.com","subject":"Re: [RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-02T00:32:48Z","receivedAt":"2017-05-02T00:32:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Daniel Ferreira (theiostream)\" <bnmvco@gmail.com> writes:\n\n> Reproducing either of these comparisons \"natively\" would simply\n> require running run_diff_index() or run_diff_files() with\n> DIFF_FORMAT_NUMSTAT and tweaking diff_flush() to format appropriately\n> for the \"interactive--add case\".\n\nA more usual way to drive the diff machinery and react to its\nfindings is to use DIFF_FORMAT_CALLBACK interface, and walk the\ncollected diff_queue to work on the paths discovered.  The way\nwt-status.c builds \"Changes to be committed\" list out of the\n\"diff-index --cached\" it internally runs and \"Changes not staged for\ncommit\" out of the \"diff-files\" it internally runs would be a good\nexample to study.\n"},{"id":"318472","messageId":"xmqqvapjdh3x.fsf@gitster.mtv.corp.google.com","threadId":"45845","inReplyTo":"xmqqo9vcf68f.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] [GSoC] Port git-add--interactive.perl:status_cmd to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-02T04:20:50Z","receivedAt":"2017-05-02T04:20:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> \"Daniel Ferreira (theiostream)\" <bnmvco@gmail.com> writes:\n>\n>> Reproducing either of these comparisons \"natively\" would simply\n>> require running run_diff_index() or run_diff_files() with\n>> DIFF_FORMAT_NUMSTAT and tweaking diff_flush() to format appropriately\n>> for the \"interactive--add case\".\n>\n> A more usual way to drive the diff machinery and react to its\n> findings is to use DIFF_FORMAT_CALLBACK interface, and walk the\n> collected diff_queue to work on the paths discovered.  The way\n> wt-status.c builds \"Changes to be committed\" list out of the\n> \"diff-index --cached\" it internally runs and \"Changes not staged for\n> commit\" out of the \"diff-files\" it internally runs would be a good\n> example to study.\n\nAhh, you'd also want to show the numstat equivalent and eventually\nyou'd need to spool the actual textual diff in-core, so that you can\npresent hunks to the user and have them accepted/rejected\ninteractively. CALLBACK interface is not a good match for that task.\n\nOf course, using a temporary file to buffer output from diff would\nbe an obvious and simple workaround (and this is not a performance\ncritical part of the system---we are talking about end user choosing\nhunks interactively after all).  A cleaner may be to spawn diff-index\nand diff-files and read from them via pipe into core.\n\n"}]}