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

Change default git conflict experience to be the inline editor #160806

Closed
isidorn opened this issue Sep 13, 2022 · 27 comments
Closed

Change default git conflict experience to be the inline editor #160806

isidorn opened this issue Sep 13, 2022 · 27 comments
Assignees
Labels
candidate Issue identified as probable candidate for fixing in the next release merge-editor verified Verification succeeded

Comments

@isidorn
Copy link
Contributor

isidorn commented Sep 13, 2022

After contemplating on all the merge editor feedback we have received, thinking about the experience as a whole and discussing with the team - we decided to change back the default of git.mergeEditor setting to false. Reasoning:

  • We have underestimated how good the previous inline conflict experience is. We have never given it enough thought, it was just a VS Code extension we decided to build-in, but luckily the experience just worked to the delight of our users.
  • VS Code is a text editor, and it delights our users when they find out that VS Code Can Do That. By default we should be simple, and users should be delighted when they discover advanced experiences. The old inline conflict one is a much simpler experience that better suits a text editor.
  • Inline conflict experience vs the merge editor is a personal preference. Each one has it is pros and cons based on monitor size, complexity of conflict, user experience. The default should be skewed towards the majority. Especially since the experts are more easily to grasp the transition to the advanced merge editor.
  • Git conflicts are complicated, to reduce total complication for users the default experience should be the simpler one.
  • If we make the merge editor an opt-in experience we can re-visit our approach to make the original file dirty - since it would not by accident change the underlying file just by opening.
  • Transition from the previous inline conflict experience to the new merge editor is straight forward, we have added a big blue button in the bottom right corner. The other direction is not so obvious (editor title area button in a complex UI).
  • The large majority of the GitHub, Twitter feedback we got strongly prefers the previous inline conflict experience.

Of course, we will continue improving the merge editor. It was the top most up-voted feature in our repo for a reason, a lot of users want and love this ❤️

In this journey we have learned a ton and without it I am sure we would have never been able to create an optimal merge experience. With the learnings we have I am sure we will have a flexible merge experience to the delight of all our users 🚀

@isidorn isidorn added candidate Issue identified as probable candidate for fixing in the next release merge-editor labels Sep 13, 2022
@isidorn isidorn added this to the August 2022 Recovery 2 milestone Sep 13, 2022
@fvsch
Copy link

fvsch commented Sep 14, 2022

Great decision!

Small feature request: would it possible to trigger the merge editor on-demand for a specific file?

I'm thinking about a UX like the one for Markdown files. By default you get an text editor, but there is an "Open Preview to the Side" button in the top right that you can click to get a rendered preview. Similarly, any file with conflicts could have an "Open in Merge Editor" button in the top right.

That would also help to onboard users onto this new tool.

@isidorn
Copy link
Contributor Author

isidorn commented Sep 14, 2022

Yes, that is exactly how we are thinking about this and it is covered in my second to last bullet point.
In the lower right corner there is a large "Open in Merge Editor" button for each file under conflict so users can easily transition to the merge editor (picture below).
For users that want the Merge Editor all the time they can manually set git.mergeEditor to true in their settings.

Screenshot 2022-09-14 at 14 25 40

@NickCraver
Copy link
Member

@isidorn Follow-up question to the above, can we please have a way to disable the blue button appearing via an option? If the user never wants the new view, it'd be akin to getting a popup you can't dismiss which for me would be many dozens of times a day minimum.

Also: mass kudos to the team for revisiting this.

@blurymind
Copy link

blurymind commented Sep 14, 2022

The main reason I stopped using vscode over any jetbrains editor is their 3 column merge conflict editor. Left side is your local branch, right side is the changes coming in and the middle is the result.

This is far superior to what you have. It let's me resolve it right in the middle, I can quickly jump through conflicting lines, I can text edit it, its light years ahead

@mjbvz
Copy link
Collaborator

mjbvz commented Sep 14, 2022

Closing as merged into 1.71 with #160925

@mjbvz mjbvz closed this as completed Sep 14, 2022
@NickCraver
Copy link
Member

@mjbvz I understand the issue is closed but...could we please get an answer on the above ask? I want to make sure this doesn't get missed because it would be a very common annoyance if we can't disable it.

@mjbvz mjbvz added the verified Verification succeeded label Sep 14, 2022
@isidorn
Copy link
Contributor Author

isidorn commented Sep 15, 2022

@NickCraver please open a new issue for that and please ping me @isidorn on the issue. Thanks a lot!

@davidcam-nwse
Copy link

davidcam-nwse commented Sep 16, 2022

Closing as merged into 1.71 with #160925

best decision ever and thanks for listening your users and reverting this decision with proper defaults, I personally greatly appreciate your changes. Thanks so much! 🙂 ❤️

@sanket-bhalerao
Copy link

I also liked the checkboxes.

@enjibby
Copy link

enjibby commented Oct 7, 2022

Massive Kudos for the team for this! The new view is honestly growing on me, but I doubt anything beats the inline experience for complex conflicts, especially in 3-way inline mode. My current workflow includes switching between these views.

I really appreciate you revisiting the experience and tuning it the way you have.

@andreiwow2
Copy link

andreiwow2 commented Oct 7, 2022

I had once an issue with the git conflict experience where somehow the conflict highlights went away and I was left with my original file being modified with both versions of the conflict and I have to spend some time cleaning it manually.

@chaffeqa
Copy link

chaffeqa commented Oct 7, 2022

I love how perceptive you are to the UX! It's not easy to "walk back" a large feature based on responses... kudos!

@levrik
Copy link

levrik commented Oct 7, 2022

I'm actually very surprised by this change. I started using the new merge editor since its first introduction and never looked back. The previous editor was awful for resolving complex conflicts and the new merge editor is a delight to use.

I'm now a bit afraid that the new merge editor will go away in the future again because most people seem to prefer the previous one. Will the new merge editor stay a feature in VS Code for the foreseeable future?

Also I would like to add that the experience was very much degraded for me by the switch from checkboxes to inline labels. It would be great to get this as an option back.

@isidorn
Copy link
Contributor Author

isidorn commented Oct 7, 2022

@levrik thanks for your feedback. No, the merge editor will not go away in the future. Please read my initial comment from above - our plan is to allow users to easily transition from the default experience to the merge editor whenever they want.

As for the checkboxes being superior to inline labels - this is not what we were seeing in multiple user studies we conducted and in user feedback on GitHub. I suggest you open a new issue for this and we can continue discussing there, I would be interested in what exactly degraded compared to the checkboxes from your point of view. Thanks!

@davidcam-nwse

This comment was marked as off-topic.

@AnrDaemon
Copy link

The only reason I'm not using the new merge editor is checkmarks. They are NOT visual. So, Meld is all the way down. (Speaking of which, they have since fixed Windows builds, so go check it out!)

@mateka
Copy link

mateka commented Oct 10, 2022

I love the new merge editor! I would not trust the argument of "strong preference of community": On the Internet, most of the time, you can only hear the voice of a small, dissatisfied minority. On the other hand, happy users will stay quiet and will be ignored 😉.

I am delighted, that I can toggle the flag in settings and use the new merge editor! I have written this comment not to start a discussion, but to say to the devs, that the new merge editor is great and I am grateful for it! Keep up the good work! 🙂

@pofl
Copy link

pofl commented Oct 10, 2022

I love the 3-way-view, but honestly it would be more useful to me if in the middle I saw the base version of the file to see what exactly the two conflicting changes were compared to the original. That allows me to judge and decide how to even do the merge in the first place.
Resolving conflicts was no problem for me and the 3-way-merge view didn't improve that experience (because no imporvement was necessary there). But a 3-way-comparison with the base version, now that would be a real assistance to decide how to resolve the conflicts.
Edit: reading just one paragraph further in the release notes shows me you have already delivered the base comparison view :D thank you ❤️

@domnantas
Copy link

While I personally prefer the 3-way merge editor, I can totally understand that default option should be as simple as possible. I would always confuse the Incoming and Current changes, maybe moving the Accept Incoming changes right above Incoming changes would make it more clear?
image

@hediet
Copy link
Member

hediet commented Oct 10, 2022

I love the 3-way-view, but honestly it would be more useful to me if in the middle I saw the base version of the file to see what exactly the two conflicting changes were compared to the original. That allows me to judge and decide how to even do the merge in the first place.

You can do this in the latest release!

@domnantas please create a new issue for that!

@niktor76
Copy link

I love the 3-way-view, but honestly it would be more useful to me if in the middle I saw the base version of the file to see what exactly the two conflicting changes were compared to the original. That allows me to judge and decide how to even do the merge in the first place.

You can do this in the latest release!

Which settings do I have to activate for it?

@levrik
Copy link

levrik commented Oct 10, 2022

@isidorn

As for the checkboxes being superior to inline labels - this is not what we were seeing in multiple user studies we conducted and in user feedback on GitHub.

For me it was by far easier to see if both, just one or none side had been chosen. Also way faster to toggle between a few versions before finding the correct merged variant. The inline labels might be easier to understand at first (it also took me a moment to understand what kind of state the checkboxes are showing me) but as soon you understand it, the checkboxes are way faster and easier to work with.

I'm having the feeling that the on-boarding to this concept was too steep, so people didn't understand it and suggested/voted for something they were familiar with, the inline labels.

I can open a separate issue, so this can be properly tracked.

@georgefst
Copy link

georgefst commented Oct 11, 2022

I'm having the feeling that the on-boarding to this concept was too steep, so people didn't understand it and suggested/voted for something they were familiar with, the inline labels.

Strongly agreed. I think it's a classic case of:

  • Implement a feature, X, with some rough edges.
  • Get negative feedback.
  • Assume that the negative feedback means X was fundamentally the wrong idea. When instead, the kinks just needed working out.
  • Revert X.

Combined with people having an aversion to change. And, as noted above, the fact that satisfied users tend to be quiet.

To be honest, I think this applies both to the merge editor as a whole, and the checkboxes.

I can open a separate issue, so this can be properly tracked.

Did you do this? I'd like to continue the discussion somewhere.

@georgefst
Copy link

Also I would like to add that the experience was very much degraded for me by the switch from checkboxes to inline labels. It would be great to get this as an option back.

Okay, I've actually just finally had to use this for the first time. It's so obviously a regression in UX that I'm pretty sceptical of how MS are running their user studies.

@levrik
Copy link

levrik commented Oct 12, 2022

@georgefst I didn't create a ticket but just found #163109

@respectTheCode
Copy link

This is so disappointing.

@Galzk
Copy link

Galzk commented Oct 13, 2022

I really think the JetBrains solution is the way to go, I personally had trouble dealing with the new vscode merge editor
My gripes with the merge editor:

  • Couldn't figure out how to view the conflict in "raw" format
  • "Accept all incoming changes" option would not work when the editor is open, plus I need to save the file for the option to take effect - that's just not clear (I think accepting should auto save)
  • In addition to the above, accepting incoming changes, then trying to exit the merge editor will present a warning - it should just close the merge editor IMO
  • Taking only a part of the change on the merge editor is not easily possible, you have to accept everything and then copy-paste parts of the code
  • Sometimes it would present a conflict that should not really be a conflict, such as a new line at the end of the file (probably not related to this merge editor issue)
  • The consolidation view is in an awkward position at the bottom, making it hard to understand what's going on with the result
  • sometimes the arrows at the top that takes you to the next conflict would just do nothing, and trying to stage the file will result in a warning that there are more conflicts to resolve
  • Using GitLens I didn't get inline blame in the merge editor, trying to understand what's the most recent change was challenging - might be GitLense issue though

I propose the following design:

Screen Shot 2022-10-13 at 12 31 11

Screen Shot 2022-10-13 at 12 45 00

That being said, I commend you guys on developing this feature! I'll toggle it on by default and will continue to use it
I was waiting for years for that feature to be added in vscode, and I follow the logic, and in favor of this default revert. very exciting times!

@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
candidate Issue identified as probable candidate for fixing in the next release merge-editor verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.