Re: [PATCH] diff compaction heuristic: favor shortest neighboring blank lines

From: Jakub Narębski <jnareb@gmail.com>
Date: Jul 4, 2016, 14:33 UTC
Message-ID: <577A73CC.1050200@gmail.com>
In-Reply-To: <xmqq8txlz19e.fsf@gitster.mtv.corp.google.com>

W dniu 2016-07-01 o 20:01, Junio C Hamano pisze:

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

Show 17 quoted lines

>> It often fails to get C preprocessor directives right:
>>
>>> a08595f76159b09d57553e37a5123f1091bb13e7:http.c aeff8a61216bf6e0d663c08c583bc8552fa3c344:http.c + 429
>>> vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
>>>                >		curl_easy_setopt(result, CURLOPT_SSLKEY, ssl_key);
>>>                >#endif
>>>                >#if LIBCURL_VERSION_NUM >= 0x070908
>>>                >	if (ssl_capath != NULL)
>>>                >		curl_easy_setopt(result, CURLOPT_CAPATH, ssl_capath);
>>>       -1 |   i >#endif
>>>        0 || ci >#if LIBCURL_VERSION_NUM >= 0x072c00
>>>          || ci >	if (ssl_pinnedkey != NULL)
>>>          || ci >		curl_easy_setopt(result, CURLOPT_PINNEDPUBLICKEY, ssl_pinnedkey);
>>>           | c  >#endif
> 
> Yes, this is "non-human do not know 'end' is likely to be at the end
> of a logical block".

I wonder if taking into account xfuncname match to adjust heuristics
(or to change "slider") would help there.  I think good heuristic
would be to break before xfuncname match (before new section).

Show 17 quoted lines

>> And it gets confused by unusual blank line placement:
>>
>>> ed55169834a3ce16a271def9630c858626ded34d:tools/eslint/node_modules/doctrine/lib/doctrine.js 2d441493a4a46a511ba1bdf93e442c3288fbe92d:tools/eslint/node_modules/doctrine/lib/doctrine.js + 330
>>> vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
>>>                >                        name === 'external' ||
>>>                >                        name === 'event')) {
>>>                >                    name += advance();
>>>                >                    name += scanIdentifier(last);
>>>                >
>>>       -1 |   i >                }
>>>        0 || ci >                if(source.charCodeAt(index) === 0x5B  /* '[' */ && source.charCodeAt(index + 1) === 0x5D  /* ']' */){
>>>          || ci >                    name += advance();
>>>          || ci >                    name += advance();
>>>           | c  >                }
> 
> Likewise, this is showing that a "non-human not knowing } is a closing
> and { is an opening token".

If not encoding heuristic that [,{,( are opening token, and ],},) are closing token into heuristics, perhaps length of the line could be a consideration about where to start a diff chunk?

-- 
Jakub Narębski