Stray CR character after replace_modified_part command

Question

Stray CR character after replace_modified_part command

Closed this issue 9 years ago · 24 comments

markusgritsch commented 9 years ago

Unmodified file:

Deleting a line and saving the file:

Invoking the command replace_modified_part:

Note the spurious CR appearing at the end of the reverted line.

I am using Git for Windows 2.5. Changing "default_line_ending" has no effect.

Git settings, but seems to also have no effect:
core.autocrlf = true

Answer 1 · 2015-08-29T14:51:26.000Z

@markusgritsch, can you create a gist with a file (or a part of it), where you see the issue?
Also, please provide your git version.

Thanks.

Answer 2 · 2015-08-29T18:40:05.000Z

I already mentioned that I use Git for Windows 2.5. However, I also had the same behaviour with previous versions of Git.

I think the crucial thing to reproduce the bug is to have a file which is in the repository with Unix (LF) line endings which gets checked out on Windows with Windows (CR LF) line endigs. For example this file
https://github.com/nodemcu/nodemcu-firmware/blob/master/Makefile
has LF line endings in the repo (https://github.com/nodemcu/nodemcu-firmware/raw/master/Makefile) but my local clone of this file has CR LF line endings.

So here is an example with exactly that file:

Now, if I delete line 3

And then press CTRL+ALT+R

the spurious CR appears.

Thanks for looking into this.

Answer 3 · 2015-08-29T18:54:17.000Z

Hehe, I was wondering what is Window 2.5.
I think I know how to fix it. I'll create a fix, then you verify it, because I don't have access to Windows machine right now.

Answer 4 · 2015-08-29T19:00:11.000Z

Great, looking forward to it.

Answer 5 · 2015-08-29T19:09:38.000Z

I just pushed commit to master.

Answer 6 · 2015-08-29T19:29:24.000Z

No, this does not change anything. Note, even if I change join_lines() to return "", a spurious CR appears (of course without any text from the original line):

Answer 7 · 2015-08-29T19:32:39.000Z

Made another fix. Hope that helps.

Answer 8 · 2015-08-30T05:36:15.000Z

Unfortunately, still the same.

Answer 9 · 2015-08-30T06:07:14.000Z

I just made another fix. And I'm pretty sure it will work.

Answer 10 · 2015-08-30T06:35:54.000Z

Still no change. If it helps, when doing
git diff Makefile > tmp.diff
the file tmp.diff has Unix (LF) line endings.

Answer 11 · 2015-08-30T06:40:23.000Z

OK. I'll get back to this when I get access to a Windows machine.

Answer 12 · 2015-08-30T06:41:40.000Z

All right.

Answer 13 · 2015-11-30T11:43:43.000Z

Hi gornostal, any chance you will take another look into this issue?

Answer 14 · 2015-11-30T11:49:52.000Z

Sorry for the delay.
I'll try to find time for this today or tomorrow.

Thanks for the reminder!

Answer 15 · 2015-12-01T17:56:26.000Z

Could not reproduce the bug today.

Here is what I did:

downloaded and installed Git-2.6.3-64.bit.exe on Windows 7
downloaded and installed ST 3
git config --global core.autocrlf true
git checkout https://github.com/nodemcu/nodemcu-firmware
modified a few lines in Makefile
pressed Ctrl+Alt+R
Modific reverted lines just fine, no CR symbols in the end of the line

Answer 16 · 2015-12-02T07:06:09.000Z

Did you delete some lines in the Makefile so that they were added back when pressing Ctrl+Alt+R?

Reverting works fine with modified lines and added lines. The problem appears only when deleting lines from the original file.

Answer 17 · 2015-12-02T07:09:29.000Z

Oh.. I missed that in your description. Will try again today.

Answer 18 · 2015-12-04T18:30:14.000Z

Have you been able to reproduce it?

Answer 19 · 2015-12-04T18:32:07.000Z

Not yet. Got distracted at work that day.
Downloading virtual box...

Answer 20 · 2015-12-04T20:46:08.000Z

I reproduced it and here is what I found out:

CR appears only if I use \r\n to join the lines.
When I change it to \n, it joins them just fine.

Can you find the line with "\r\n" in Modific.py and change that to \n, then let me know if it works for you?

If it does I will simply add a new config option to manually change line endings, because as it turns out, we cannot use windows line endings on windows in some cases like yours.

Answer 21 · 2015-12-05T08:13:19.000Z

Checked in changes to issue-96 branch.
Will merge when @markusgritsch confirms it's working.

Answer 22 · 2015-12-05T20:21:31.000Z

Yes, this seems to solve the issue, thanks!

But why add a "line_ending" configuration option? Who would need to set this to something else than \n?

Answer 23 · 2015-12-05T20:28:39.000Z

It's good to hear it works.

I thought I could break the functionality for someone who needs \r\n, but maybe you are right. I'll use \n.

P.S. Thanks for being insistent :)

Answer 24 · 2015-12-05T20:45:16.000Z

You are welcome ;) I like Modific a lot, and this was the only outstanding issue I had with it. Great that it's fixed now.

Concerning the config option: I think, it is unnecessary. Mind you: The problem with the stray \r characters appeard on a file, which had Windows (\r\n) line endings, and even there the config option has to be set to \n.

Besides: If it would be different from file to file, it would not be practicat to have to change the config option every time.