All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Schindelin <johannes.schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Beat Bolli" <dev+git@drbeat.li>,
	"Jeff King" <peff@peff.org>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Torsten Bögershausen" <tboegi@web.de>,
	"Ramsay Jones" <ramsay@ramsayjones.plus.com>
Subject: [PATCH v5 0/2] Let merge-file write out conflict markers with correct EOLs
Date: Wed, 27 Jan 2016 17:37:25 +0100 (CET)	[thread overview]
Message-ID: <cover.1453912583.git.johannes.schindelin@gmx.de> (raw)
In-Reply-To: <cover.1453819314.git.johannes.schindelin@gmx.de>

The original patch was sent by Beat Bolli in
http://thread.gmane.org/gmane.comp.version-control.git/281600

My suggestion to extend it to respect gitattributes led to
changes that broke the original patch. And they were misguided
to begin with (see below).

Since there have been a couple of "What's cooking" mails
containing unheard calls for updates on this patch, I took it
on myself to fix things.

Junio's comment that we might look at blobs containing CR/LF
line endings made me rethink the entire approach, and I am now
convinced that we need to abandon the entire idea to make the
conflict markers depend on config settings or attributes:
after all, I introduced `git merge-file` as a replacement for
GNU merge that can be used *outside* of any repository, by design.

The crucial idea hit me yesterday when I took a step back: all
we need to do is to ensure that the end-of-line style is matched
when *all* input files are LF-only, or when they are all CR/LF.
In all other cases, we have mixed line endings anyway.

And to do that, it is sufficient to look at *one single line
ending* in the context. Technically, it does not even have to be
the context, but the first line endings of the first file would
be enough, however it is so much more pleasant if the conflict
marker's eol matches the one of the preceding line.

To prevent my future self from repeating mistakes, I added a
little bit of background to the first commit message.

Triggered by a comment by Junio, I realized that a second patch is
needed: we need to make sure that the conflicting sections are also
augmented by the appropriate line endings if they lack any.

The change relative to v4 is that I now test the correct conflict
markers: the merge-file call we add to t6023 actually produces four
conflict markers because a previous test switched the conflict style
to `diff3` mode. This is only a side effect, though, therefore I
really wanted to avoid testing for it. However, I managed to test
an *incorrect* subset of three, but this is now fixed. Thanks go
to Ramsay Jones for poking my nose onto that issue.


Johannes Schindelin (2):
  merge-file: let conflict markers match end-of-line style of the
    context
  merge-file: ensure that conflict sections match eol style

 t/t6023-merge-file.sh | 13 +++++++
 xdiff/xmerge.c        | 98 +++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 93 insertions(+), 18 deletions(-)

Interdiff vs v4:

 diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh
 index 1390548..20aee43 100755
 --- a/t/t6023-merge-file.sh
 +++ b/t/t6023-merge-file.sh
 @@ -352,11 +352,11 @@ test_expect_success 'conflict sections match existing line endings' '
  	printf "1\\r\\n2\\r\\n5" >crlf-diff2.txt &&
  	test_must_fail git -c core.eol=crlf merge-file -p \
  		crlf-diff1.txt crlf-orig.txt crlf-diff2.txt >crlf.txt &&
 -	test $(tr "\015" Q <crlf.txt | grep "\\.txtQ$" | wc -l) = 3 &&
 +	test $(tr "\015" Q <crlf.txt | grep "^[<=>].*Q$" | wc -l) = 3 &&
  	test $(tr "\015" Q <crlf.txt | grep "[345]Q$" | wc -l) = 3 &&
  	test_must_fail git -c core.eol=crlf merge-file -p \
  		nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >nolf.txt &&
 -	test $(tr "\015" Q <nolf.txt | grep "\\.txtQ$" | wc -l) = 0
 +	test $(tr "\015" Q <nolf.txt | grep "^[<=>].*Q$" | wc -l) = 0
  '
  
  test_done

-- 
2.7.0.windows.1.7.g55a05c8

  parent reply	other threads:[~2016-01-27 16:37 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-22 17:01 [PATCH 0/2] Let merge-file write out conflict markers with correct EOLs Johannes Schindelin
2016-01-22 17:01 ` [PATCH 1/2] convert: add a helper to determine the correct EOL for a given path Johannes Schindelin
2016-01-22 18:47   ` Junio C Hamano
2016-01-22 19:04     ` Johannes Schindelin
2016-01-23  7:05       ` Torsten Bögershausen
2016-01-24 10:42       ` Johannes Schindelin
2016-01-23  6:12   ` Torsten Bögershausen
2016-01-24 10:41     ` Johannes Schindelin
2016-01-22 17:01 ` [PATCH 2/2] merge-file: consider core.crlf when writing merge markers Johannes Schindelin
2016-01-22 18:15   ` Junio C Hamano
2016-01-22 18:47     ` Johannes Schindelin
2016-01-22 19:08       ` Junio C Hamano
2016-01-24 10:44         ` Johannes Schindelin
2016-01-22 19:50   ` Eric Sunshine
2016-01-22 19:52     ` Eric Sunshine
2016-01-24 10:37       ` Johannes Schindelin
2016-01-24 18:26         ` Eric Sunshine
2016-01-25  7:02           ` Johannes Schindelin
2016-01-23  6:31   ` Torsten Bögershausen
2016-01-24 10:39     ` Johannes Schindelin
2016-01-22 17:52 ` [PATCH 0/2] Let merge-file write out conflict markers with correct EOLs Beat Bolli
2016-01-24 10:48 ` [PATCH v2 0/1] " Johannes Schindelin
2016-01-24 10:48   ` [PATCH v2 1/1] merge-file: let conflict markers match end-of-line style of the context Johannes Schindelin
2016-01-24 16:27     ` Torsten Bögershausen
2016-01-24 16:36       ` Torsten Bögershausen
2016-01-25  6:53       ` Johannes Schindelin
2016-01-25 19:45         ` Ramsay Jones
2016-01-26  8:54           ` Johannes Schindelin
2016-01-26 16:43             ` Ramsay Jones
2016-01-26 16:49               ` Johannes Schindelin
2016-01-24 22:09   ` [PATCH v2 0/1] Let merge-file write out conflict markers with correct EOLs Junio C Hamano
2016-01-25  7:25     ` Johannes Schindelin
2016-01-25  8:06   ` [PATCH v3 0/2] " Johannes Schindelin
2016-01-25  8:07     ` [PATCH v3 1/2] merge-file: let conflict markers match end-of-line style of the context Johannes Schindelin
2016-01-25  8:32       ` Torsten Bögershausen
2016-01-25  8:59         ` Johannes Schindelin
2016-01-25 20:12       ` Junio C Hamano
2016-01-26  9:04         ` Johannes Schindelin
2016-01-26 17:22           ` Junio C Hamano
2016-01-25  8:07     ` [PATCH v3 2/2] merge-file: ensure that conflict sections match eol style Johannes Schindelin
2016-01-25  9:20       ` Johannes Schindelin
2016-01-26 14:42     ` [PATCH v4 0/2] Let merge-file write out conflict markers with correct EOLs Johannes Schindelin
2016-01-26 14:42       ` [PATCH v4 1/2] merge-file: let conflict markers match end-of-line style of the context Johannes Schindelin
2016-01-26 18:23         ` Junio C Hamano
2016-01-26 21:14           ` Junio C Hamano
2016-01-27  7:58             ` Johannes Schindelin
2016-01-27 18:19               ` Junio C Hamano
2016-01-27 19:12                 ` Johannes Schindelin
2016-01-27 19:32                   ` Junio C Hamano
2016-01-28  7:55                     ` Johannes Schindelin
2016-01-26 14:42       ` [PATCH v4 2/2] merge-file: ensure that conflict sections match eol style Johannes Schindelin
2016-01-27 16:37       ` Johannes Schindelin [this message]
2016-01-27 16:37         ` [PATCH v5 1/2] merge-file: let conflict markers match end-of-line style of the context Johannes Schindelin
2016-01-27 16:37         ` [PATCH v5 2/2] merge-file: ensure that conflict sections match eol style Johannes Schindelin
2016-01-27 20:22         ` [PATCH v5 0/2] Let merge-file write out conflict markers with correct EOLs Junio C Hamano

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

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

  git send-email \
    --in-reply-to=cover.1453912583.git.johannes.schindelin@gmx.de \
    --to=johannes.schindelin@gmx.de \
    --cc=dev+git@drbeat.li \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.org \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sunshine@sunshineco.com \
    --cc=tboegi@web.de \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.