On Tue, Jun 13, 2017 at 2:54 PM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
> On Wed, 24 May 2017 14:40:23 -0700
> Stefan Beller <sbeller@xxxxxxxxxx> wrote:
>
>> Currently, diff output is written either through the emit_line_0
>> function or through the FILE * in struct diff_options directly. To
>> make it easier to teach diff to buffer its output (which will be done
>> in a subsequent commit), introduce a more flexible emit_line() function.
>> In this commit, direct usages of emit_line_0() are replaced with
>> emit_line(); subsequent commits will also replace usages of the
>> FILE * with emit().
>
> Check the names of the functions in this paragraph.
Only the very last word needed replacement of /s/emit/emit_line/.
>
>> diff --git a/diff.c b/diff.c
>> index 2f9722b382..3569857818 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -516,36 +516,30 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
>> ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
>> }
>>
>> -static void emit_line_0(struct diff_options *o, const char *set, const char *reset,
>> - int first, const char *line, int len)
>> +static void emit_line(struct diff_options *o, const char *set, const char *reset,
>> + int add_line_prefix, int sign, const char *line, int len)
>
> In the future, this function is going to be used even to emit partial
> lines
Yes.
> - could this be called emit() instead?
Despite having good IDEs available some (including me)
very much like working with raw text, and then having a function
named as a common string doesn't help.
After this patch
$ git grep emit_line |wc -l
16
# not all are this function, there is
emit_line_checked as well. But 16 is not too much.
But if renamed to emit():
$ git grep emit -- diff.c |wc -l
60
You could argue I'd just have to grep
for "emit (" instead, but that then I would have
rely on correct whitespacing or use a regex already.
Complexity which I would not like.
So I am not sure if this is helping a reader. (Not the casual
reader, but the one grepping for this function)
Maybe we can settle on a different name though,
such as emit_string which is not a prefix of a dozen
different other functions?
Thanks,
Stefan