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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 4, 2016, 19:52 UTC
Message-ID: <xmqq37mk2txa.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <7b0680ed7a10fc13acd8d7816a75ed05a5f9e28c.1470259583.git.mhagger@alum.mit.edu>

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

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

Show 8 quoted lines

> +#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 ...

Show 9 quoted lines

> +        } 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.