Stefan Beller <stefanbel...@gmail.com> writes:

> From: Stefan Beller <sbel...@google.com>
>
> At all call sites of emit_{add, del, context}_line we increment the line
> count, so move it into the respective functions to make the code at the
> calling site a bit clearer.
>
> Signed-off-by: Stefan Beller <sbel...@google.com>
> ---
>  diff.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)

I am mostly in favor of this change, as the calls to these three
functions are always preceded by increment of these fields, but it
is "mostly" exactly because the reverse is not true.  Namely ...

> @@ -1293,16 +1294,12 @@ static void fn_out_consume(void *priv, char *line, 
> unsigned long len)
>  
>       switch (line[0]) {
>       case '+':
> -             ecbdata->lno_in_postimage++;
>               emit_add_line(reset, ecbdata, line + 1, len - 1);
>               break;
>       case '-':
> -             ecbdata->lno_in_preimage++;
>               emit_del_line(reset, ecbdata, line + 1, len - 1);
>               break;
>       case ' ':
> -             ecbdata->lno_in_postimage++;
> -             ecbdata->lno_in_preimage++;
>               emit_context_line(reset, ecbdata, line + 1, len - 1);
>               break;
>       default:

... there still needs an increment in the context lines, not shown
in the patch, just after this "default:".  I think the patch is OK
as the comment after this "default:" (also not shown in the patch)
makes it clear what is going on.

Thanks.


Reply via email to