git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: tboegi@web.de
Cc: git@vger.kernel.org
Subject: Re: [PATCH v1 0/1] t6038-merge-text-auto.sh
Date: Mon, 30 May 2016 11:48:21 -0700	[thread overview]
Message-ID: <xmqqy46r8jre.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqzir7o286.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Mon, 30 May 2016 11:00:25 -0700")

Junio C Hamano <gitster@pobox.com> writes:

> Imagine that you started from a history where somebody recorded a
> text file with CRLF in the blob, unconverted.  Later the project
> decided to express their text with LF to support cross-platform
> development better, and sets up the Auto-CRLF.  Your user is working
> near the tip of that history after the eol correction happened.  Now
> she gets a pull-request of a branch that forked from an old point in
> the history, before the eol correction and full of CRLF.  Yes, to
> integrate the change being proposed, she needs to look at "theirs";
> that's the whole point of a "merge".  Why should she revert the eol
> correction her history has by getting fooled by the fact that the
> update was based on a part of the history before the eol correction?

Thinking aloud along this line a bit further, if you really cared
(and I don't feel very strongly, as I think "safe crlf" is an ugly
workaround that lets users keep their misconfiguration by penalizing
their day-to-day operation), you may want to think about doing a
3-way merge of "eol preference" beween all stages.

That is, if the common ancestor in stage #1 and the current version
in stage #2 both have its text in LF, and the data being merged in
stage #3 is in CRLF, you sayr "do not convert; the change being
brought in wants to have CRLF endings, while our history did not
care".  Similarly, if the common ancestor in stage #1 and the data
being merged in stage #3 both have CRLF, and your version in stage
#2 has LF, you say "We wanted to fix eol since the side branch
forked, and the side branch predates that fix, so we keep the eol
fix we did since we diverged", i.e. "Do convert".

For doing this, you may want to refactor the codepath that decides
"Auto-CRLF usually wants to turn CRLF in text to LF, but should we
disable that logic now, because the user already has CRLF in the
current one?" into a function that takes a single parameter, `path`,
and returns "Yes, do convert CRLF to LF / No, do not convert"
boolean.  Having a low level function that says "What's the blob at
this path in the index" and have the caller run that logic feels
unwieldy if we want to go that route.

  reply	other threads:[~2016-05-30 18:48 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-13 13:49 Bug report: Duplicate CRLF rewrite warnings on commit Adam Dinwoodie
2016-05-13 16:43 ` Junio C Hamano
2016-05-14  5:40   ` Torsten Bögershausen
2016-05-14 11:17     ` Adam Dinwoodie
2016-05-13 18:12 ` Jeff King
2016-05-13 19:46   ` Junio C Hamano
2016-05-13 19:53     ` Jeff King
2016-05-15  6:08 ` [PATCH/RFC v1 0/1] Quickfix ?No duplicate " tboegi
2016-05-15  6:08 ` [PATCH/RFC v1 1/1] No " tboegi
2016-05-15  6:15   ` Eric Sunshine
2016-05-15  6:37 ` [PATCH v1 0/3] CRLF-Handling: bug fix around ce_compare_data() tboegi
2016-05-15  6:38 ` [PATCH v1 1/3] t6038; use crlf on all platforms tboegi
2016-05-15  6:42   ` Eric Sunshine
2016-05-15  6:38 ` [PATCH v1 2/3] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-15  6:45   ` Eric Sunshine
2016-05-15  6:38 ` [PATCH v1 3/3] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-15  6:52   ` Eric Sunshine
2016-05-15 22:14   ` Junio C Hamano
2016-05-16 15:51     ` [PATCH v3 0/1] CRLF-Handling: bug fix around ce_compare_data() tboegi
2016-05-16 16:13       ` Junio C Hamano
2016-05-17  4:08         ` Torsten Bögershausen
2016-05-17 16:09           ` [PATCH v1 1/1] t6038; use crlf on all platforms tboegi
2016-05-17 18:39             ` Junio C Hamano
2016-05-17 16:41           ` [PATCH v4 0/2] CRLF: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-17 16:41           ` [PATCH v4 1/2] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-17 16:41           ` [PATCH v4 2/2] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-17 18:58             ` Junio C Hamano
2016-05-18  4:26               ` Torsten Bögershausen
2016-05-18 15:10                 ` Torsten Bögershausen
2016-05-19 14:21           ` [PATCH v5 0/2] CRLF: " tboegi
2016-05-19 23:10             ` Junio C Hamano
2016-05-19 14:21           ` [PATCH v5 1/2] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-19 14:21           ` [PATCH v5 2/2] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-19 23:03             ` Junio C Hamano
2016-05-20 17:12               ` [PATCH v6 1/2] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-20 17:12               ` [PATCH v6 2/2] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-20 17:46                 ` Junio C Hamano
2016-05-21 10:01               ` [PATCH v7 1/2] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-21 10:01               ` [PATCH v7 2/2] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-24 18:36                 ` Junio C Hamano
2016-05-16 15:51     ` [PATCH v3 1/1] " tboegi
2016-05-30 17:00     ` [PATCH v1 0/1] t6038-merge-text-auto.sh tboegi
2016-05-30 18:00       ` Junio C Hamano
2016-05-30 18:48         ` Junio C Hamano [this message]
2016-05-30 17:00     ` [PATCH v1 1/1] t6038: different eol for "Merge addition of text=auto" tboegi
2016-06-07 15:20     ` [PATCH v2 0/3] unified auto CRLF handling, V2 tboegi
2016-06-07 15:20     ` [PATCH v2 1/3] convert: unify the "auto" handling of CRLF tboegi
2016-06-07 15:20     ` [PATCH v2 2/3] read-cache: factor out get_sha1_from_index() helper tboegi
2016-06-07 15:20     ` [PATCH v2 3/3] Correct ce_compare_data() in a middle of a merge tboegi
2016-05-15 13:02 ` [PATCH v2 0/3] CRLF-Handling: bug fix around ce_compare_data() tboegi
2016-05-15 13:02 ` [PATCH v2 1/3] read-cache: factor out get_sha1_from_index() helper tboegi
2016-05-15 13:02 ` [PATCH v2 2/3] convert: ce_compare_data() checks for a sha1 of a path tboegi
2016-05-15 13:02 ` [PATCH v2 3/3] t6038; use crlf on all platforms tboegi

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=xmqqy46r8jre.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --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.
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).