On Sat, May 6, 2017 at 1:13 AM, Daniel Ferreira (theiostream)
<bnmvco@xxxxxxxxx> wrote:
> On Fri, May 5, 2017 at 7:30 PM, Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
>>> +static int git_add_interactive_config(const char *var,
>>
>> Not git_add_interactive__helper_config()? ;-)
>
> I don't get if you mean this ironically (because of the verbosity) or
> if you do think this would be a good name ;P
>
>>> + for (i = 0; i < q->nr; i++) {
>>> + struct diff_filepair *p;
>>> + p = q->queue[i];
>>> + diff_flush_stat(p, options, &stat);
>>> + }
>>> +
>>> + for (i = 0; i < stat.nr; i++) {
>>> + int file_index = s->file_count;
>>> + for (j = 0; j < s->file_count; j++) {
>>> + if (!strcmp(s->files[j].path, stat.files[i]->name)) {
>>> + file_index = j;
>>> + break;
>>> + }
>>> + }
>>
>> So basically, this is looking up in a list whether we saw the file in
>> question already, and the reason we have to do that is that we run the
>> entire shebang twice, once with the worktree and once with the index.
>>
>> I wonder whether it would not make sense to switch away s->files from a
>> list to a hashmap.
>> [...]
>> BTW in the first pass, we pretty much know that we only get unique names,
>> so the entire lookup is unnecessary and will just increase the time
>> complexity from O(n) to O(n^2). So let's avoid that.
>>
>> By moving to a hashmap, you can even get the second phase down to an
>> expected O(n).
>
> How would you go about implementing that hashmap (i.e. what should be
> the hash)? Does Git have any interface for it, or is there any example
> I can look after in the codebase?
Git has a hashmap API already. 7d4558c462 is a good example of using it.
>>> + printf(ADD_INTERACTIVE_HEADER_INDENT);
>>> + color_fprintf(stdout, header_color, modified_fmt, _("staged"),
>>> + _("unstaged"), _("path"));
>>
>> I think these _() need to become N_().
>
> I cannot find any call to N_() outside of Perl code. What should that
> even do differently?
N_() is to mark a string for later translation, not to return it. See
my "how to use getopt" patch for an example of that, but this doesn't
look like such a case, since _() is returning the string to
color_fprintf, surely...
$ git grep -P '\bN_\(' -- '*.c'