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

Cannot see all conflicts #6213

Closed
decabeza opened this issue Nov 16, 2018 · 63 comments
Closed

Cannot see all conflicts #6213

decabeza opened this issue Nov 16, 2018 · 63 comments
Milestone

Comments

@decabeza
Copy link

decabeza commented Nov 16, 2018

Please describe the problem you think should be solved
When you merge a branch with conflicts in previous version you could check all files with conflicts. Now appears a no closable popup with the list of all conflicted files and you have to open one by one on text editor instead of see them on Github Desktop.

Example:

conflicts

Also when you resolve all the conflicts you can't review all changed files before merge 😅

merge

merge

Do you have any potential solutions in mind?
I think this popup should have a close button to hide it an can check all files (as before) without have to open one by one.

@j-f1
Copy link
Contributor

j-f1 commented Nov 16, 2018

The dialog could be dismissed into a banner saying “X conflicts remaining. Resolve →” that could be clicked to reopen the dialog.

@billygriffin
Copy link
Contributor

Thanks @decabeza! One of the things we still want to validate is whether there's some group of people who will prefer to just be able to see the conflicted files differentiated and be able to resolve them however you decide to instead of going through the more guided flow. Your description and the screenshots are really helpful there.

One note is that you can click "Open in Sublime Text" on one of the files and just navigate within Sublime to each of the others and come back to Desktop at the end so you're not having to bounce back and forth. The conflict resolution dialog will update as each of the conflicts is resolved so you don't have to come back to Desktop each time unless you want to.

On your second observation, can you unpack that a bit more for us? Are you saying that you want to be able to see a final diff of each of the files before completing the merge? We heard pretty convincingly from users in our usability tests that people felt like the extra step of viewing the final diff was burdensome and not particularly valuable, but we can obviously revisit that if it's something people are struggling with.

cc: @desktop/design

@billygriffin
Copy link
Contributor

@decabeza We're discussing your issue internally and I wanted to clarify one additional thing. Are the first and second screenshots in the same flow? In other words, after you resolved the conflicts from the first screenshot, did you then see the second screenshot with only one file displayed?

@webmayak
Copy link

webmayak commented Nov 17, 2018

Subcribed. New merge flow is uncomfortable

@manas-kulkarni
Copy link

New merge flow is annoying. I dont have any of the supported merge editors. So I do it through TortoiseMerge using git mergetool from command line. This adds a .ORIG file and I am forced to commit it since I cant review the changes

@chrispday
Copy link

Agree that new merge flow has issues. If anything goes wrong with the merge and can't commit there doesn't seem to be an option to close and manually fix yourself. Only option is to abort which rolls back all merges done so far. Agree with @j-f1 that there should at least be an option to just close the dialog and leave things as is.

@decabeza
Copy link
Author

Hi @billygriffin,

I understand the benefits to see all conflicted files. The main problem I see is for example if you merge two branches there is two types of files: the conflicted ones and other files, I would like can review all files to check the merge is correct.

About your second question, yes the screenshots are in the same flow, I start with 93 conflicted files I was resolving them and the number was going down. When there was only file left first appear the "0 conflictes files" message, after go to Sublime Text and back the message refresh to "All conflicts resolved" but I still can't view the rest of the files before merging.

I attach some screenshot with some ideas about how to improve this flow (I think all are compatible with each other):

1. Make popup closable
1_closable_popup

2. List changed files separate of conflicted ones
2_separate_files

3. Add a link to reopen the popup
3_link_to_conflicted

@billygriffin
Copy link
Contributor

@decabeza Thank you, really helpful and appreciate the thoughtful response.

Separate from the questions of how to view all the files and ensure everything is good before committing the merge (and your explanation makes complete sense), I'm curious about the flow in your initial screenshots. It appears that you had 93 conflicts, but after resolving each one they should all show up with checkmarks next to them to indicate that each file has been resolved. So it should be a scrollable list of resolved files, and I'm concerned that your second screenshot only had config/application.rb in it. I'd just like to verify that you went through the following steps:

  1. Click merge
  2. See a scrollable list of 93 conflicted files.
  3. Go through each file and resolve the conflicts in Sublime.
  4. Come back to Desktop.
  5. See the modal with only one file listed as resolved (config/application.rb) and the options to either abort or commit merge.

If that's right, it's a bug that would be pretty bad. So I just want to verify that separately from the larger questions that we're discussing internally. Thanks again!

@decabeza
Copy link
Author

@billygriffin I think it's not a bug, it's my flow 😅 I mean...

  1. I merge a branch with conflicts
    step_1

  2. I going resolving and all appear as resolved on a scrollable list.
    step_2

  3. I add the file on command line with (for example) git add .hound.yml, so the file disappear from the list
    step_3

I'm used to work between Github Desktop UI and command line, sorry for the confusion 🙏 😌

@billygriffin
Copy link
Contributor

@decabeza Ah! Perfect, thank you for clarifying, that makes me feel better. 😄 We'll keep you updated on how we're thinking about the original problems, and thanks again.

@ampinsk
Copy link
Contributor

ampinsk commented Nov 19, 2018

Hi everyone! Thanks so much for sharing your thoughts. Based on everyone's feedback here, we're considering this as an iteration on the current flow:

We'll add a close button to the modal:

modal

We'll also add a dropdown option to the open buttons:

dropdown-open-options

Closing the modal will take you to this state:

manual state 2x

  • You would reopen the modal from the banner at the top
  • The file list will update with a green check as you remove conflict markers
  • We will always enable the commit button, but we will show a warning dialog if you commit files with conflict markers

Let us know if you have any more feedback! If this feels like a good direction, we'll work on implementing it soon.

@billygriffin
Copy link
Contributor

billygriffin commented Nov 19, 2018

Thanks @ampinsk, looks really nice and hopefully helps to give people an escape hatch if they want to see the diff and/or the additional files that were not conflicted. And it's nice that they can easily get back to it.

A couple considerations in implementation:

  • We'll need to make sure the title stops before the close button, and that you can tab to it for accessibility.
  • If we do go this direction, I'd also like to instrument metrics to find out what % of time people are closing the dialog, and also how frequently people are reopening it. If we implement something like this and a trivially small portion of people are closing the dialog, maybe we re-evaluate. And if a large portion of people are closing it, maybe we reconsider our original happy-path implementation.

@chrispday
Copy link

To try and add some more constructivity, it might also be helpful to add an "Open all in (editor of choice)" button. The dialogue is not very big and I had more than a dozen merge conflicts and scrolling and clicking each could have been a better experience.

The other reasons I would like to close this dialog without commiting (or aborting) is that I like to review my conflicted merges as a diff just before committing to double check I've done it right.

Merging conflicts is probably the hardest thing to do, so I think this effort is a good idea. But it's also where flexibility in workflow is required. Most of the time it can be followed but still provides an escape hatch for when things don't want to work cleanly. Especially when you are doing long and complicated merges where clicking the wrong button could lose a lot of effort.

@j-f1
Copy link
Contributor

j-f1 commented Nov 19, 2018

How about replacing the commit button with a button that re-opens the resolution dialog?

@billygriffin
Copy link
Contributor

To try and add some more constructivity, it might also be helpful to add an "Open all in (editor of choice)" button.

Thanks for the feedback @chrispday! That feels like a somewhat separate issue that we're definitely keeping an eye on the feedback. A couple things stand out. First, our hypothesis is that the vast majority of merge conflicts have only 1-3 files conflicted so we think for most cases this won't be an issue. Additionally, most editors should allow you to navigate to the files themselves and not have to come back to Desktop each time, and when you get back to Desktop at the end you're in better shape. But if there's a dozen or more, I totally get how that could be tough to figure out where they all are and it's helpful to provide an escape hatch.

@billygriffin
Copy link
Contributor

billygriffin commented Nov 19, 2018

How about replacing the commit button with a button that re-opens the resolution dialog?

@j-f1 We don't want to slow people down if that's their preferred workflow to work outside of the guided flow, and they can still commit after resolving the conflicts in the normal Changes pane, right? I'd worry that replacing the commit button would add an extra step if they just wanted to commit the merge via that button. Thoughts?

@j-f1
Copy link
Contributor

j-f1 commented Nov 20, 2018

@billygriffin I was thinking that the button would only be replaced when attempting to commit would generate an error about resolving conflicts.

@billygriffin
Copy link
Contributor

@j-f1 Oh, interesting, that makes sense. Because you can't actually commit until you've resolved them, thanks for the added context. @outofambit Do you happen to know what the state of the commit button will be now if someone closes the modal (as in @ampinsk's designs above) and still has unresolved conflicts?

@MarcusDivine
Copy link

MarcusDivine commented Nov 21, 2018

If the proposed solution is implemented, I do hope that you do not decide to re-evaluate even if it turns out that a trivially small portion of people actually end up closing the dialog. I would echo the sentiment by @chrispday , flexibility is required when dealing with merging conflicts, and options are a good thing.

@outofambit
Copy link
Contributor

outofambit commented Nov 26, 2018

Do you happen to know what the state of the commit button will be now if someone closes the modal (as in @ampinsk's designs above) and still has unresolved conflicts?

@billygriffin there is currently no logic to stop a user from committing conflicted files in that part of the UI. so the button would be active and users could commit whatever files are checked in the changes pane.

@killroy42
Copy link

In absolute desperation, I even installed Atom... still doesn't get me past that modal. It opens Atom alright, but doesn't give me access to the GitHub UI to view the diff to find the conflicts, nor does it give me an option to look at other repos to get my work done or even to help me complete the merge!

@billygriffin
Copy link
Contributor

@killroy42 Thanks for the additional feedback. We found in interviews that most people use an external editor to typically resolve their conflicts, and that's why we made the primary path to open the conflicted file in your editor, resolve it there, and then complete the merge after all conflicts are resolved back in Desktop. There's also an option in the modal to open in command line in order to resolve it in a different fashion (for instance, if you use a merge tool to resolve conflicts).

What do you typically use in your normal workflow to resolve conflicts?

@killroy42
Copy link

@billygriffin At any point in time I have dozens of consoles open as well as several editor windows and multiple panes across three monitors. Your button does nothing for me. But the modal blocking access to GitHub takes away the tools I need. Why am I not allowed to look at my other repos? Why am I not allowed to look at the diff of the conflicted file? What's the reasoning behind this?

Even after installing Atom, just to satisfy your modal, All the button does is open Atom with the file. It does not give me back the diff display inside GitHub, or give me access to my other repos and information I might need.

Even if I taskkill GitHub to restart it an gain access to a different repo (perhaps to do urgent work there), it will immediately get disabled again after starting up by that modal, denying me access to my work.

@billygriffin
Copy link
Contributor

@killroy42 Once the conflict is either resolved or aborted (however you choose to do that), you should no longer see the conflicts modal. Our hypothesis based on interviews is that when users are in a conflicted state, they will want to get into a clean state (either by aborting or resolving) prior to moving onto other work. So that's why the modal is front and center.

How do you typically resolve conflicts currently? Do you use an editor, the command line, or another tool?

@killroy42
Copy link

Well, I USED to use GitHub to look at the diff to see what the conflict was before I would choose how to resolve it, usually with the editor already open that I am working in, or using the console, one of the many already open as well.

Unfortunately, I now no longer can use GitHub as a tool to help me resolve the conflict. I have to use tedious string searches to find the conflict, and if I have to look something up in one of the other, related repos of this project, I have to completely revert the merge, before I am able to view that repo in GitHub, before re-initiating the merge once again on the command line.

What purpose does that modal serve? What single benefit does it give me to accomplish my tasks of writing code and developing applications?

@billygriffin
Copy link
Contributor

@danielb987 Thanks for your thoughts, and sorry you're not feeling heard. While we definitely want to support a continuously improving out-of-the-box experience for newer developers, our goal is to continue to improve the experience for more experienced developers as well - that's why we're exploring support for rebasing and other workflows that some more advanced users prefer.

We recognize in this thread that our initial iteration didn't quite hit the mark for users who didn't need (or didn't want) a more guided editor-centric flow, and that's why we're actively iterating on it based on the feedback we've received here and elsewhere. We're also evaluating metrics over time to help determine what percentage of users are having success resolving conflicts compared to before, so that will help guide our path forward as well.

As editors are concerned, we hope GitHub Desktop will increasingly be able to provide support for more and more editors over time, and we're totally open to open source contributions to add additional supported editors, with documentation on how to do that here: https://github.com/desktop/desktop/blob/master/docs/technical/editor-integration.md

We're not always going to get the first iteration of new features perfect for every subset of users, but we are continually evaluating how we can be more representative in our user interviews and usability testing as we explore how we solve problems in GitHub Desktop. Thanks again for sharing, and I hope the next iteration of this in 1.6 will make the experience less painful for you.

@danielb987
Copy link

@billygriffin : Thanks.

@msftrncs
Copy link
Contributor

msftrncs commented Dec 7, 2018

to all, some ideas regarding the methods for resolution,

I understand the complexity of supporting editors. Compounded by the fact that newer editors support opening folders, its not as easy, amongst all the OS's to just say, 'Hey OS, here is a path, do what you do with it', because most of the magic happens in the shell, not in the OS, and also too many programs fight for the default operations of file extensions or types, and so it can never be clear what the results will be.

(Note, do not ever perform the default action on BAT or CMD files when you intend to just edit them!)

However, maybe the user can be helped, to help himself. In the merge conflict resolution, other solutions instead of just opening the editor should be possible. (some of this may have been mentioned before)

  1. add the following items to a drop-down button next to the 'open with {editor}' button.
    1. 'Open with default program' (like in the changed file list)
    2. 'Show in Explorer/Finder' with the file selected. (also like in the changed file list)

@danielb987
Copy link

I have found a HUGE bug in either GitHub Desktop or Atom.

I merged master into my working branch and got a conflict in a file. I opened the file in Atom (which I have installed against my will since I'm forced to).

I wasn't sure what was the correct change, but I clicked on one of the options in Atom to resolve the conflict. Then I realized that I had done the wrong thing and wanted to go back so I closed Atom and returned to GitHub Desktop. I opened Atom again from GitHub Desktop and realized that Atom kept the change!!!! What a stupid program to do that!!!

So I closed Atom, aborted the merge. I did the merge again, got the merge conflict in GitHub Desktop and opened the file in Atom. And Atom still has saved the change!!!!

Atom seems to be the most stupid software I have ever seen!!!! What in hell should I do to revert the change????

If you really, really wants the users to hate GitHub desktop, this is the way to do it.

I'm sorry, but I'm very angry at the moment. If I would have had the option to edit the file in my own editor this would never had happen!!!

@msftrncs
Copy link
Contributor

msftrncs commented Dec 7, 2018

@danielb987, this sounds like you experienced what is called 'HOT EXIT' where you can exit with a file in an unsaved state, and the next time you open the editor it will restore your changes to the file right where you left off. This is a common feature of VS Code, but I am not sure about Atom since I do not use it, but they are very similar. If Atom is like VS Code, there is a context item somewhere to 'REVERT' back to the copy on disk, possibly in the file menu. If this is HOT EXIT, the changes are not saved to the file, but to a temp file somewhere else, and so you can easily REVERT, or you can just use a different editor, which will not see the change.

@billygriffin
Copy link
Contributor

@danielb987 Thanks for the feedback. The merge conflict modal that directs you to your editor is in no way forcing you to use that path or to use Atom. You can open it on the command line from the modal, or open it in your preferred editor by just switching to that editor and resolving your conflicts there and then coming back to GitHub Desktop to commit the merge. We created the new flow to be helpful for those who have editors set up in Desktop and can easily be directed to the file in their editor, but if you're already in your editor and don't want to add it as a supported editor in Desktop, I'd recommend just switching to it when you encounter a conflict, resolving it there, and when you're finished then come back to Desktop to complete it (or obviously you're welcome to do it elsewhere too).

I also just tried to reproduce the issue you described and was unable to do so (aborting the merge successfully got everything back into the previous state in Atom or any other editor). If you're able to reproduce it reliably, you're welcome to file an issue for it in https://github.com/atom/atom.

@lee-dohm
Copy link
Contributor

lee-dohm commented Dec 7, 2018

@danielb987 your comment above is a violation of the Desktop Code of Conduct as it is insulting or derogatory. You may consider this an official warning.

@billygriffin
Copy link
Contributor

billygriffin commented Dec 7, 2018

add the following items to a drop-down button next to the 'open with {editor}' button.

'Open with default program' (like in the changed file list)
'Show in Explorer/Finder' with the file selected. (also like in the changed file list)

@msftrncs Thanks for these suggestions. We're discussing a few options to help clarify that this isn't intended to be your only way to resolve conflicts, but that we're just trying to make it more convenient by routing you to your editor if it's set up in Desktop. 😄 More to come here.

@ampinsk
Copy link
Contributor

ampinsk commented Dec 10, 2018

@msftrncs thanks for the suggestion!

dropdown-open-options

We'll update the button with a dropdown option to expose more open options. I added this to the designs above!

@lee-dohm
Copy link
Contributor

@Swarzkopf314 your comment was deleted as a violation of the Desktop Code of Conduct as it was insulting or derogatory. You may consider this an official warning.

@Swarzkopf314
Copy link

Swarzkopf314 commented Dec 18, 2018

Why wasn't the close button in the initial release of this feature? It breaks my workflow as I have the same problem as mentioned here: #6311.

It seems a really bad and annoying practice to force users to change their workflow and I'm increasingly worried about the course you're taking with the app (it started with the decision to remove "magic stashing" entirely instead of making it an opt-in option, but let's forget that for now).

@billygriffin
Copy link
Contributor

Hi @Swarzkopf314, thanks for the updated response. Prior to the last release, we did usability testing with several people and everyone indicated that closing the modal or getting back to the diff added an extra level of complexity that didn't feel necessary. However, we understand after the feedback we've received here that there are some percentage of people who prefer to more manual workflow during conflict resolution for a variety of reasons. We know when we release things that each individual thing isn't going to be perfect, but that's why we listen to feedback, try to unpack the underlying problems, and iterate from there.

With respect to magic stashing, I totally understand the frustration having used the previous Mac app and liking that feature. As you saw, we're working on that in #6107 and it's on the roadmap for the next few big things we're working on. Because the current app was entirely new from the previous apps and only the Mac app had magic stashing (and it was relatively unreliable and sometimes got people into bad spots), we decided to prioritize other things. But we do think it's important and we are starting to work on it. Appreciate the feedback and hope this helps explain our process a bit more.

@Swarzkopf314
Copy link

Swarzkopf314 commented Dec 18, 2018 via email

@billygriffin
Copy link
Contributor

Since you are not doing that it just feels that somebody out there just thinks he knows better and wants to bring salvation upon the app's misguided and uneducated users by force.

That's a totally fair way to feel. I think in retrospect we calibrated too far on the side of being too opinionated here and didn't leave enough room for other workflows. Candidly, we hadn't heard feedback from people who were like, "this merge conflicts error and just leaving me on my own in Desktop is really working for me." Feedback we've heard has always been that it was confusing and unclear as to what the state was and what to do next. So we're always trying to improve our own way of doing things, but we definitely don't think we know best and we're learning as we go. And yep, that's exactly why your feedback is valuable. So thank you. ❤️

@shiftkey
Copy link
Member

This issue has covered a lot of things, which we spun off into #6379 #6380 #6381. We're aiming to ship these improvements out to everyone with 1.5.1 tomorrow, so I'm in favour of closing this out in favour of more specific feedback based on those changes.

@billygriffin let me know if there's anything else we should be tracking in separate issues based on this discussion.

@shiftkey shiftkey assigned billygriffin and unassigned outofambit Dec 18, 2018
@billygriffin
Copy link
Contributor

That sounds right @shiftkey, thanks! 1.5.1 is scheduled to ship (🤞 tomorrow), and feel free to follow up here with product feedback. There are still a few small iterative things we're working on in 1.6 but overall the bulk of this is already on beta and will be released to production in 1.5.1. Thank you to everyone for the thoughtful feedback!

@billygriffin billygriffin removed their assignment Dec 18, 2018
@billygriffin
Copy link
Contributor

Thanks everyone for all the great feedback and @decabeza for opening this issue in the first place. 1.5.1 is out and includes most of the planned iteration here. Thanks again for helping us improve it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests