search

git-up / GitUp

The Git interface you've been missing all your life has finally arrived.
http://gitup.co
GNU General Public License v3.0
11.44k stars 1.23k forks source link

Handle submodule conflicts #996

Closed lapfelix closed 1 week ago

lapfelix commented 2 months ago

Resolves https://github.com/git-up/GitUp/issues/282

I use submodules every day at work and I think GitUp is the best Git interface so this has been a regular issue for me for way too long 🤪

The basic UI I came up with looks like this:

image

It closely matches the current submodule diff UI, but with buttons to choose which commit should be chosen (as a side note, in a future update it would be interesting to also pull the title of each commits and display it in the diff/conflicts views as I don't know my commit SHA1s by heart).

The "sketchiest" part of this PR is probably in GCDiff.m lines 358-377. I've written an explanation in a comment right above the code to justify it, but it would be nice if the superfluous untracked diff entry was not returned by libgit2 at all, but I couldn't find any combination of diff options that could do that.

Before

Submodule conflicts weren't handled at all and any rebase that involves a submodule conflict errors out in GitUp (and the repository needs to be "Reset to Checkout..." to get a clean working directory again | Submodule conflicts can be handled with a "Choose ours" or "Choose theirs" button

https://github.com/git-up/GitUp/assets/4634735/1c8caddd-e121-49ff-8231-3e0cf76944fc

After

Submodule conflicts can be handled with a "Choose ours" or "Choose theirs" button

https://github.com/git-up/GitUp/assets/4634735/a5494762-4b77-4285-9a34-ba6f0ac9d552

Here's the repository I used for my tests: test-repo-submodules.zip

Once https://github.com/git-up/GitUp/pull/989 is merged, I could use it as a base to write a unit test that checks that submodule conflicts are well handled.

I AGREE TO THE GITUP CONTRIBUTOR LICENSE AGREEMENT

lucasderraugh commented 1 week ago

@lapfelix I'll come back on this one. You mention at the end that you'd use #989 as a base for unit tests on this. Would you still have interest in this? I understand if not, just want to know whether I should review this as is.

lapfelix commented 1 week ago

I might have more time to do this in a month (maybe before). I'd be motivated to do it though!