René Scharfe <l.s.r@xxxxxx> writes: > Am 25.10.2017 um 20:49 schrieb Stefan Beller: >> +/* >> + * Compare the strings l1 with l2 which are of size s1 and s2 respectively. >> + * Returns 1 if the strings are deemed equal, 0 otherwise. >> + * The `flags` given as XDF_WHITESPACE_FLAGS determine how white spaces >> + * are treated for the comparision. >> + */ >> +extern int xdiff_compare_lines(const char *l1, long s1, >> + const char *l2, long s2, long flags); > > With the added comment it's OK here. > > Still, I find the tendency in libxdiff to use the shortest possible > variable names to be hard on the eyes. That math-like notation may have > its place in that external library, but I think we should be careful > lest it spreads. Yeah, I tend to agree. The xdiff-interface is at the (surprise!) interface layer, so we could make it follow the style of either one, but we seem to have picked up the convention of the lower layer, so... By the way, I was looking at the code around this area while reviewing the cr-at-eol thing, and noticed a couple of things: * combine-diff.c special cases only IGNORE_WHITESPACE and IGNORE_WHITESPACE_CHANGE but no other IGNORE_WHITESPACE things; I have a suspicion that this is not because IGNORE_WHITESPACE_AT_EOL does not have to special cased by the codepath, but only because the combine-diff.c refactoring predates the introduction of ws-at-eol thing? The machinery uses its own match_string_spaces() helper; it may make sense to use the same xdiff_compare_lines() in its callers and get rid of this helper function. * diff_setup_done() sets DIFF_FROM_CONTENTS when any of the IGNORE_WHITESPACE bits is true, to allow "git diff -q --ignore-something" would do the right thing. We do not however give a similar special case to XDF_IGNORE_BLANK_LINES. $ echo >>Makefile && git diff $option --ignore-blank-lines Makefile exits with 1 when option=--exit-code and it exits with 0 when option=-q For now I'll leve these as #leftoverbits, but I may come back to the latter soonish. I won't come back to the former until Stefan's refactor hits 'master', though.
