search

smashwilson / merge-conflicts

Resolve git merge conflicts in Atom
https://atom.io/packages/merge-conflicts
MIT License
432 stars 42 forks source link

Conflict resolution controls doesn't appear with 1.4.x #227

Closed emileber closed 8 years ago

emileber commented 8 years ago

I was experiencing problem since a couple days with this plugin. Detection works. It renders the listing correctly and a clicked file appears. It's then that I don't see the resolution overlay. It's just the raw un-merged file.

I tried disabling every other plugin, then restarted Atom, same problem.

I reverted to 1.3.7 and it worked.

I'm on windows with Atom version 1.7.1

batjko commented 8 years ago

Same here. It lists the conflict, but clicking on a file does not result in the conflict areas being highlighted to work on. No idea how long it's been in this state, as I haven't used the package in a while.

smashwilson commented 8 years ago

This is likely a result of the ConflictParser changes that were made recently to recognize diff3 output. I think we're outgrowing the hacky mechanism that I'd been using to parse conflict markers. I've got a branch locally where I've been refactoring that into something more maintainable; hopefully I'll catch the problem here along the way.

A few questions to clarify what's going on:

batjko commented 8 years ago

Happened to me with two different projects, one javascript, one custom file type. Windows 10. Unfortunately, I have already gone through corrections. Next time I encounter this, I'll save a copy and post it.

smashwilson commented 8 years ago

Sounds good. I'm most suspicious about those \r\n endings right now; the local smoke tests I do don't cover those yet.

batjko commented 8 years ago

Well I can tell you that my files are both written and committed with Windows line endings, e.g. something like this: image

But that's always been the case and it used to work when I first tried this package.

jolsen71 commented 8 years ago

I just tried to use this module for the first time with a merge conflict yesterday. It detected the file that had conflicts, but when I opened it, it simply displayed the file with the normal git markers in it. I am running Atom 1.8.0-b2 on Windows 10 but the file in question has on LF line endings (everything we do is LF-only). I haven't been able to test it more than this one particular merge conflict which was only a single file. I found this existing reported issue when researching the problem. Unfortunately, though, I didn't keep a sample of the conflict marker area. Anyhow, I just wanted to chime in and say at least for me, I had the problem with just LF endings on a Windows platform.

smashwilson commented 8 years ago

But that's always been the case and it used to work when I first tried this package.

Yeah, a recent change that modified that regular expression which is a likely culprit; regular expressions are notoriously tricky and this one's growing in complexity.

I am running Atom 1.8.0-b2 on Windows 10 but the file in question has on LF line endings (everything we do is LF-only).

Oh weird. I'll have to do some testing downstairs on the Windows machine to see if I can replicate that one. Thanks for the data point :100:

emileber commented 8 years ago

I'm on windows and I know for sure that I'm using LF line endings on my projects as I'm enforcing it through git, and Atom confirms that. With version 1.3.7, both line endings work.

I encountered the problem with different projects and different file types, I remember mostly js.

I made a test:

Aaaaannnd it worked well...

So I made a second test, this time with a js file, and I got the problem.

atom-merge-conflicts-bug

emileber commented 8 years ago

I also tried disabling all other installed community packages and the problem was still present.

jolsen71 commented 8 years ago

I just tried with 1.4.2 (uninstalled and installed a fresh merge-conflict package). Atom 1.8.0-beta2. Unfortunately, it is still failing to detect the markers or something. This test file is UTF-8, LF endings, and Ruby language.

screenshot 5

jolsen71 commented 8 years ago

Possible duplicate of #223 and #196

smashwilson commented 8 years ago

I just tried with 1.4.2 (uninstalled and installed a fresh merge-conflict package). Atom 1.8.0-beta2. Unfortunately, it is still failing to detect the markers or something.

Nuts, I was hoping that #229 would take care of this without needing more complicated diagnostics :smile:

I just gave this a shot on my Windows 10 machine downstairs and... couldn't replicate it. I'm not using Atom Beta there yet, though. I'll give that a try later and see if there's a regression there.