From: Simon Ruderich <simon@ruderich.org>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/2] color-moved: ignore all space changes by default
Date: Thu, 26 Oct 2017 09:22:13 +0200 [thread overview]
Message-ID: <20171026072213.p6llaqrbdss7nbu4@ruderich.org> (raw)
In-Reply-To: <20171025224620.27657-1-sbeller@google.com>
On Wed, Oct 25, 2017 at 03:46:18PM -0700, Stefan Beller wrote:
> On Mon, Oct 23, 2017 at 7:52 PM, Stefan Beller wrote[1]:
>> On Mon, Oct 23, 2017 at 6:54 PM, Junio C Hamano wrote:
>>>
>>> * As moved-lines display is mostly a presentation thing, I wonder
>>> if it makes sense to always match loosely wrt whitespace
>>> differences.
>>
>> Well, sometimes the user wants to know if it is byte-for-byte identical
>> (unlikely to be code, but maybe column oriented data for input;
>> think of all our FORTRAN users. ;)
>
> ... and this is the implementation and the flip of the default setting
> to ignore all white space for the move detection.
Hello,
I'm not sure if this is a good default. I think it's not obvious
that moved code gets treated differently than regular changes. I
wouldn't expect git diff to ignore whitespace changes (without me
telling it to) and so when I see moved code I expect they were
moved as is.
And there are languages where indentation is relevant (e.g.
Python, YAML) and as color-moved is also treated as review tool
to detect unwanted changes this new default can be dangerous.
The new options sound like a good addition but I don't think the
defaults should change. However unrelated to this decision,
please add config settings in addition to these new options so
users can globally configure the behavior they want.
Regards
Simon
--
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
next prev parent reply other threads:[~2017-10-26 7:22 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-25 22:46 [PATCH 0/2] color-moved: ignore all space changes by default Stefan Beller
2017-10-25 22:46 ` [PATCH 1/2] diff: decouple white space treatment for move detection from generic option Stefan Beller
2017-10-25 22:46 ` [PATCH 2/2] diff.c: ignore all white space changes by default in the move detection Stefan Beller
2017-10-26 7:22 ` Simon Ruderich [this message]
2017-10-26 8:42 ` [PATCH 0/2] color-moved: ignore all space changes by default Jacob Keller
2017-10-26 17:59 ` Stefan Beller
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=20171026072213.p6llaqrbdss7nbu4@ruderich.org \
--to=simon@ruderich.org \
--cc=git@vger.kernel.org \
--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).