Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOS line endings break hunk operations #20

Closed
baudtack opened this issue Jun 8, 2010 · 15 comments
Closed

DOS line endings break hunk operations #20

baudtack opened this issue Jun 8, 2010 · 15 comments
Labels
bug windows Something isn't working on Windows

Comments

@baudtack
Copy link

baudtack commented Jun 8, 2010

When a file with DOS line endings is changed, operations on hunks no longer work.

Steps to reproduce:
create a git repo
add a file with dos line endings (I did this by creating a file and then using unix2dos to change the line endings)
commit the file
edit the file
try to remove edited hunk (not all edits on the file!)

@philjackson
Copy link
Contributor

It's because there's a mix of unix and DOS newlines. Pretty easy to fix... I think.

*edit: or perhaps not.

@baudtack
Copy link
Author

baudtack commented Jun 9, 2010

I'm going to try to take a look at it tonight if I have time. I'm assuming it's just some sort of escaping issue, but I only looked at the code for about 30 seconds.

@baudtack
Copy link
Author

baudtack commented Jul 2, 2010

I tried to figure this out but couldn't. I tried using shell-quote-argument in magit-insert-current-line but that had no effect. I had difficulty following the flow of the program so that may not be where the problem is. It's seriously driving me insane though.

@baudtack
Copy link
Author

baudtack commented Jul 7, 2010

Someone tagged this as "windows" It's not a windows problem. I'm having this problem on Linux where the original files were created in Windows with dos line endings.

@lindi2
Copy link
Contributor

lindi2 commented Sep 14, 2010

Steps to reproduce:

  1. git init
  2. echo hello > hello.txt
  3. git add hello.txt
  4. git commit -a -m 'hello'
  5. unix2dos hello.txt
  6. emacs
  7. M-x magit-status RET
  8. try to hit "v" at the hunk

Expected results:
8) hunk is reverted

Actual results:
8) magit says "git failed"

More info:

  1. Remarkably the problem stops occuring if I run

strace -p $(pidof emacs) -o emacs.strace -f -s4096

and then hit "v" again. Can you reproduce this?

@lindi2
Copy link
Contributor

lindi2 commented Sep 14, 2010

If I strace only git I see

15060 read(0, "diff --git a/hello.txt b/hello.txt\n", 8192) = 35
15060 read(0, "index ce01362..ef0493b 100644\n", 12277) = 30
15060 read(0, "--- a/hello.txt\n", 12247) = 16
15060 read(0, "+++ b/hello.txt\n", 12231) = 16
15060 read(0, "@@ -1 +1 @@\n", 12215) = 12
15060 read(0, "-hello\n", 12203) = 7
15060 read(0, "+hello\n", 12196) = 7
15060 read(0, "\n", 12189) = 1
15060 read(0, "", 12188)

which obviously lacks the \r. If I strace both emacs and git I see

15279 read(0, "diff --git a/hello.txt b/hello.txt\nindex ce01362..ef0493b 100644\n--- a/hello.txt\n+++ b/hello.txt\n@@ -1 +1 @@\n-hello\n+hello\r\n", 8192) = 124
15279 read(0, "", 12188)

which includes the "\r".

@lindi2
Copy link
Contributor

lindi2 commented Sep 14, 2010

The issue seems to be that the git subprocess has a terminal and stty -F /dev/pts/10 -a shows that "icrnl" is set. This means that \r gets converted to \n.

Why is git run in a terminal?

@lindi2
Copy link
Contributor

lindi2 commented Sep 14, 2010

You can pull a proposed fix from the issue-20 branch at

http://iki.fi/lindi/git/magit.git

From d8c938a812748b638e4ba6c30f7058e3920287b4 Mon Sep 17 00:00:00 2001
From: Timo Juhani Lindfors 
Date: Tue, 14 Sep 2010 22:33:02 +0300
Subject: [PATCH] Don't use a pty with git processes that take input from magit since
 that would set the terminal option icrnl which would modify the
 input. In particular \r would get converted to \n (issue #20).

---
 magit.el |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/magit.el b/magit.el
index 057c9c8..5f271fa 100644
--- a/magit.el
+++ b/magit.el
@@ -1465,7 +1465,10 @@ FUNC should leave point at the end of the modified region"
           (with-current-buffer input
         (setq default-directory dir)
         (setq magit-process
-              (apply 'magit-start-process cmd buf cmd args))
+              ;; Don't use a pty, because it would set icrnl
+              ;; which would modify the input (issue #20).
+              (let ((process-connection-type nil))
+            (apply 'magit-start-process cmd buf cmd args)))
         (set-process-filter magit-process 'magit-process-filter)
         (process-send-region magit-process
                      (point-min) (point-max))
-- 
1.7.1

@baudtack
Copy link
Author

Tested and works great! So awesome. This has been driving me crazy for months but I haven't been able to get around to trying to figure it out. Hope this gets integrated into the mainline soon!

@philjackson
Copy link
Contributor

Thanks for your work, chaps.

@baudtack
Copy link
Author

baudtack commented Oct 4, 2010

Is there any ETA on this patch being merged?

@baudtack
Copy link
Author

Sorry to keep harping on this. Any ETA on merging?

@philjackson
Copy link
Contributor

I've applied lindi's patch. Thanks again.

@baudtack
Copy link
Author

Oh frabjous day! Callooh! Callay! Makes me so happy!

@arvidj
Copy link

arvidj commented Oct 28, 2010

Good job, thank you!

alanfalloon pushed a commit to alanfalloon/magit that referenced this issue Apr 28, 2011
that would set the terminal option icrnl which would modify the
input. In particular \r would get converted to \n (issue magit#20).
tarsius pushed a commit that referenced this issue Nov 17, 2014
`git-commit-skip-magit-header-regexp` is nil before Magit is loaded.  If
Git Commit Mode was loaded first, the variable would be added as `nil`
into the font lock keywords, thus breaking fontification.
kenny-thomas pushed a commit to kenny-thomas/magit that referenced this issue Aug 22, 2017
`git-commit-skip-magit-header-regexp` is nil before Magit is loaded.  If
Git Commit Mode was loaded first, the variable would be added as `nil`
into the font lock keywords, thus breaking fontification.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug windows Something isn't working on Windows
Development

No branches or pull requests

4 participants