Re: [PATCH v1 6/7] correct blame for files commited with CRLF

Junio C Hamano [off-list ref] writes:

Junio C Hamano [off-list ref] writes:

quoted

Torsten Bögershausen [off-list ref] writes:

quoted

If we had made the CRLF -> LF conversion, yes. But we don't do that.
crlf_to_git() returns without touching the line endings.

That sounds quite broken.  How would a user ever fix broken data in
the index then?  I know the commit that often appears in these
discussions claims to give us "safer CRLF" handling, but I have a
suspicion that perhaps we should rethink if that claim is really
true.  Isn't it giving us more problems than it is worth?

Having said all that, within the context of the current codebase
where autocrlf refuses to do the conversion, I agree that teaching
blame to follow the same logic makes sense.  Let me review the
series up to 6/7 with fresh eyes.

And for the same reason 7/7 may make sense (I didn't check the
implementation, but I think I understand your motivation well enough
to comment)--if crlf-to-git returns without actually converting upon
next "git add $path", in order to serve a preview of what change you
would be checking in and/or committing when you do "git add $path"
and/or "git commit $path", the _to_git() conversion "git diff" and
"git diff HEAD" do on the contents taken from the working tree
should follow the same logic.

"git diff $tree-ish" (of which "git diff HEAD" is a special case) is
a bit tricky to reason about, but I think using the "does index have
CR to cause us refuse conversion?" logic is a sensible thing to do
even in that case.  It is asking what difference you would have if
you committed the state in the working tree right now, and the "does
index have CR?" logic to kick in, even though the contents of the
index may not be something that was derived from the unrelated
$tree-ish, would kick in when you make the hypothetical commit to be
compared against $tree-ish.

Again, let me review 7/7 as well with fresh eyes.

Thanks.