git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/2] color-moved: ignore all space changes by default

From
Simon Ruderich <simon@ruderich.org>
Date
Oct 26, 2017, 07:22 UTC
Message-ID
<20171026072213.p6llaqrbdss7nbu4@ruderich.org>
In-Reply-To
<20171025224620.27657-1-sbeller@google.com>
On Wed, Oct 25, 2017 at 03:46:18PM -0700, Stefan Beller wrote:
Show 13 quoted lines
> 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
Previous: Stefan BellerNext: Jacob Keller
Message 4 of 6 in “color-moved: ignore all space changes by default”
  1. 0/2 color-moved: ignore all space changes by defaultStefan Beller, Oct 25, 2017
  2. 1/2 diff: decouple white space treatment for move detection from generic optionStefan Beller, Oct 25, 2017
  3. 2/2 diff.c: ignore all white space changes by default in the move detectionStefan Beller, Oct 25, 2017
  4. Simon RuderichOct 26, 2017
  5. Jacob KellerOct 26, 2017
  6. Stefan BellerOct 26, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.