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

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

Michael Haggerty [off-list ref] writes:

quoted

It often fails to get C preprocessor directives right:

quoted

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).
 

quoted

And it gets confused by unusual blank line placement:

quoted

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