Re: [PATCH 8/8] diff: improve positioning of add/delete blocks in diffs

[Junio C Hamano <gitster@pobox.com>]

Michael Haggerty <mhagger@alum.mit.edu> writes:

I agree with Peff about "comment on the voodoo upfront".

> +#define START_OF_FILE_BONUS 9
> +#define END_OF_FILE_BONUS 46
> +#define TOTAL_BLANK_WEIGHT 4
> +#define PRE_BLANK_WEIGHT 16
> +#define RELATIVE_INDENT_BONUS -1
> +#define RELATIVE_INDENT_HAS_BLANK_BONUS 15
> +#define RELATIVE_OUTDENT_BONUS -19
> +#define RELATIVE_OUTDENT_HAS_BLANK_BONUS 2

When I read up to here, I thought "Heh, isn't the opposite of INdent
DEdent?" and then saw this:

> +#define RELATIVE_DEDENT_BONUS -63
> +#define RELATIVE_DEDENT_HAS_BLANK_BONUS 50

It turns out that you mean by OUTdent a line that indents further
(if I am reading the code correctly).  Is that obvious to everybody?

> +	/* Bonuses based on the location of blank lines: */
> +        bonus += TOTAL_BLANK_WEIGHT * total_blanks;
> +	bonus += PRE_BLANK_WEIGHT * m->pre_blank;

This and ...

> +        } else if (indent > m->pre_indent) {
> +		/*
> +		 * The line is indented more than its predecessor. Score it based
> +		 * on the larger indent:
> +		 */
> +		score = indent;
> +		bonus += RELATIVE_INDENT_BONUS;
> +		bonus += RELATIVE_INDENT_HAS_BLANK_BONUS * any_blanks;
> +	} else if (indent < m->pre_indent) {

... this seems to be indented correctly even after getting quoted,
which in turn means most of the lines in the added code share
indent-with-non-tab badness.