Ping Yin <pkufranky@xxxxxxxxx> writes:
> With --color-words, following example will show deleted word "bar" at
> another line. (<r> represents red)
> ------------------
> $ git diff
> - foo bar
> + foo
> $ git diff --color-words
> foo
> <r>bar</r>
> ------------------
Minor style nit, but commit log is not asiidoc and the horizontal bars are
distracting. Just use a blank line either ends to separate the example
from the descriptive text, and indent the example by a few places.
> This wrong behaviour is a bug in fn_out_diff_words_aux which always
"This wrong behaviour is a bug" is a very roundabout way to say it. "This
is caused by a bug in ..."?
> Instead, we always supress the newline when using print_word, and in
> fn_out_diff_words_aux, a newline is shown only in following cases:
>
> - true minus.suppressed_newline followd by a line beginning with
> '-', ' ' or '@' (i.e. not '+')
> - true plus.suppressed_newline followd by a line beginning with
> '+', ' ' or '@' (i.e. not '-')
Hmm, that may describe _what_ the updated code _does_, but does not
explain why/how it is an improvement.
For this kind of change we really would need tests to illustrate various
inputs and desired output for them.
> diff --git a/diff.c b/diff.c
> index b5f7141..11316fe 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -409,6 +409,7 @@ static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, in
> static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
> {
> struct diff_words_data *diff_words;
> + char cm;
> struct diff_words_buffer *dm, *dp;
> FILE *df;
These nondescriptive two letter variable names make the logic very hard to
follow. What does cm represent in the new code? Let's see.... (spends a
few minutes to follow the code)... Ah, it is just a randomly named
temporary variable of "char" type and do not have long-term meaning of any
significance, and could have been named "c" or "ch" or whatever. Heck,
the code fooled me because it looked like it was named similarly to the dm
and dp nearby.
Not a demonstration of "cm" being named poorly, but this wasted few
minutes shows that dm and dp are named very poorly.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
