microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.68k stars 28.69k forks source link

merge-conflict: Add compare {current,incoming} to original #133088

Open goldfirere opened 3 years ago

goldfirere commented 3 years ago

I use 3-way merge conflict checking, where I can see all of the current changes, the original code, and the incoming changes. My workflow for resolving conflicts is to compare the original code to the current and then, separately, to the incoming changes. I can then figure out which changes are easier to port (say, reflect the original-to-incoming changes in the current code), and then actually port the changes. A critical step here is to be able to compare the original version against the updated versions.

VSCode's merge-conflict support allows me to easily compare the current code against the incoming code, but that's not nearly as actionable as comparing against the original code. Is it possible to add a feature where, in a 3-way conflict, I can compare current and incoming against original?

Useful prior art: emacs's smerge-mode allows the smerge-refine command, which cycles through the 3 possible comparisons (original/current, original/incoming, current/incoming).

IllusionMH commented 3 years ago

Looks like duplicate of #37350

goldfirere commented 3 years ago

Thanks, @IllusionMH, but I don't think so. I feel like I must be missing something, but it looks like #37350 is essentially a side-by-side representation of the diff3 conflict style that git provides. This side-by-side representation is really useful, but I'm looking for something more: word-by-word (or char-by-char) highlights of what actually changed. Let me demonstrate.

This is the rendering of a 3-way merge in VSCode today.

Screen Shot 2021-09-14 at 11 17 08 AM

With merge-conflict.compare, I can get this:

Screen Shot 2021-09-14 at 11 19 16 AM

Note that orig is highlighted, along with the fact that EQ1 and EQ2 are different. These highlights are so, so useful. But this comparison is between the current and incoming versions. I want the same display, but comparing the current and original or the incoming and original. That's it! No three-way fanciness required (beyond what git already does).

For comparison, here is emacs:

Screen Shot 2021-09-14 at 11 22 12 AM

I have cycled through the three possible head-to-head comparisons to see that original-vs-incoming involves only a 1-character change. I can then confidently change EQ2 to EQ1 in the current code, accept the current version, and move on. Without manually doing the word-by-word or char-by-char comparison, I don't see how I can as easily resolve this conflict in VSCode.

As an added bonus, I'm hoping that implementing what I want is far easier than #37350. It's exactly the same as the existing merge-conflict.compare, but just changing the files that are compared.

goldfirere commented 3 years ago

I made a vain attempt to fix this myself in https://github.com/goldfirere/vscode/commit/9f8c9ebdfad14c6fea58f71e20c8a018f9e5aa42, but it doesn't work (and fails the pre-commit lint, due to my constructed string). I've never worked in TypeScript before, so there could be all manner of other silliness here, but the commit should get the idea across. Someone more knowledgeable with VSCode internals could hopefully take what I have and make it actually work. (It doesn't: somehow, my new commands aren't recognized. This baffles me.)

chrmarti commented 3 years ago

Nothing stands out on first look. Try making some trivial change to verify it gets compiled and picked up after reloading the window.

goldfirere commented 3 years ago

It's definitely recompiling and loading my changes. If I just changed this line in the original code to use conflict.commonAncestors[0].content, I was able to observe the result I wanted.

In the version I've built, my new command e.g. merge-conflict.compare-current-original appears in the command palette (when I Cmd+Shift+P), but then executing it produces an error that the command doesn't exist. It smells to me like the registerTextEditorCommand isn't working somehow -- or that I've mistyped the name of the command in different places. But I don't spot my mistake.

chrmarti commented 3 years ago

If nothing triggers the activation of the extension, this would be expected. Try adding onCommand activation events for each command in the package.json's activationEvents. I guess you would not run them from the command palette normally which is why onStartupFinished as the only activation event works for the existing commands.

goldfirere commented 3 years ago

Thanks for this advice. I couldn't get anything with activationEvents working, but your message inspired me to clone my local repo into a new folder (yarn clean isn't a thing... and I only thought of git clean when writing this comment), where my new stuff works. Hooray!

What are good next steps here? I already noticed a bug in my code, in that if I click on a code lens to do the comparison, the cursor still needs to be within the conflict region for the comparison to work. This is because I didn't, at first, understand how/where the conflict ever gets passed to the action. I'm pretty sure I can fix this. And I suppose I could avoid the constructed string by just enumerating the three possibilities separately. (Luckily, three is a small number.)

But the tasks beyond this are beyond me: creating tests, any documentation, etc. I could put in this time, but is it a reasonable expectation that this patch would get merged? I don't want to spend time cleaning the code and learning more about how to contribute without a reward at the end. Or, is it a better strategy to wait for the VSCode team to address this, essentially using my patch as a tighter specification of what I'd like?

What do you suggest?

Thanks for your help!

chrmarti commented 3 years ago

Could you open a pull request with your latest code? I wonder if just adding more code lenses will overload the UI a bit.

goldfirere commented 3 years ago

I agree about the code lenses -- it was actually just a small experiment to see if the strange behavior I was witnessing was related to the command palette somehow and I wanted a new way to trigger my new code path. I will remove it -- happy for this feature to be available only for someone who looks for it.

OK. I will polish a bit and post a PR. Thanks.