git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Jakub Narębski" <jnareb@gmail.com>
To: git@vger.kernel.org
Cc: Stefan Beller <sbeller@google.com>, Jeff King <peff@peff.net>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Jacob Keller <jacob.keller@gmail.com>
Subject: Re: [PATCH] diff compaction heuristic: favor shortest neighboring blank lines
Date: Mon, 4 Jul 2016 16:33:48 +0200	[thread overview]
Message-ID: <577A73CC.1050200@gmail.com> (raw)
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:

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



      reply	other threads:[~2016-07-04 14:34 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-16 17:46 [PATCH] diff compaction heuristic: favor shortest neighboring blank lines Stefan Beller
2016-06-16 20:27 ` Junio C Hamano
2016-06-16 21:06   ` Stefan Beller
2016-06-16 21:10 ` Michael Haggerty
2016-06-16 21:36   ` Stefan Beller
2016-06-17 15:36 ` Jeff King
2016-06-17 16:09   ` Stefan Beller
2016-06-23 17:10     ` Michael Haggerty
2016-06-23 17:25       ` Stefan Beller
2016-06-23 17:37       ` Junio C Hamano
2016-06-23 20:13         ` Michael Haggerty
2016-06-30 13:54       ` Michael Haggerty
2016-07-01 17:04         ` diff heuristics dramatically improved by considering line indentation and " Michael Haggerty
2016-07-01 18:01         ` [PATCH] diff compaction heuristic: favor shortest neighboring " Junio C Hamano
2016-07-04 14:33           ` Jakub Narębski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=577A73CC.1050200@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jacob.keller@gmail.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).