Add merging functionality and UI

[ianhi]

Fix #310

This adds a button in all branch list item and in the Git menu to merge local branch into the current one.

Using the Git menu entry, the user will be prompted to select the branch to merge.

---

Original post

This PR is an attempt to implement a merging UI similar to what github desktop has. I don't think it's complete but wanted to put it here for any feedback so that if I'm going in the wrong direction I can stop that sooner rather than later. In particular, before I go about writing tests and such.

I drew pretty heavily from the github desktop UI, but didn't replicate their pre-emptive checking of if a merge is conflict free. Though this looks like it could be figured out with a series of git commands: https://stackoverflow.com/questions/501407/is-there-a-git-merge-dry-run-option

Here's the current UI, I think my one big worry is that was I was too invasive in modifying the existing newBranchDialog and should have just made a mergeDialog object. (To be clear the actual UI of the newBranchDialog is unchanged)

Button in BranchMenu to trigger a dialog:

Dialog that pops up:
Before a branch is selected:

After a branch is selected:

merge conflict:
The error reporting here could definitely use some work. I think using something other than the default error box will allow for better listing of the files that were the problem.

merge settings

I just wrote the merge command as git merge branch and didn't play arround with the other arguments like -ff. I also didn't try to extract the diff stat that merge ouputs in order to display it somewhere because this would have necessitated popping up another dialog.

[kgryte]

I would suggest creating a separate dialog.

IMO one of the current design problems within this extension is that components tend to be shoehorned into serving multiple purposes, thus increasing code complexity and affecting maintainability.

My preference would be components which are dedicated to single purposes. Here, that purpose would be to provide merge functionality.

[ianhi]

Thanks for the feedback, that makes sense to me. My reasoning for doing it this this way was to avoid replicating the filtering and branch selection and some of the styling. As this branch selection seems like the core shared feature of both of these dialogs.

Would you suggest having an entirely separate widget that does its own listing and selection of branches? Or is it worth extracting parts that can be shared between the newBranch and merging dialogs? i.e. having a single component that displays lists of branches and incorporating that into both dialogs.

[mlucool]

Notebooks aren't great with regular git merge. You may want to consider integrating nbdime for the notebook merge: https://nbdime.readthedocs.io/en/latest/#why-is-nbdime-needed

[fcollonval]

Would you suggest having an entirely separate widget that does its own listing and selection of branches? Or is it worth extracting parts that can be shared between the newBranch and merging dialogs? i.e. having a single component that displays lists of branches and incorporating that into both dialogs.

Smart laziness should drive developper work 😃 ; meaning make the code as modular as possible to avoid copy-paste (because you are copy-pasting bugs 😉). For ReactJS, this means composing component rather than inheriting (see the documentation). So creating small reusable component is the way to go.

Note: prefer functional component rather than ES6 classes for simple small component (e.g. ActionButton)

[fcollonval]

Notebooks aren't great with regular git merge. You may want to consider integrating nbdime for the notebook merge: https://nbdime.readthedocs.io/en/latest/#why-is-nbdime-needed

@mlucool I think @ianhi just want to provide a way to carry a simple merge. Resolving conflict is a though business (although it will be a great addition). It should be carry out in subsequent PRs.

Note: technologies to have a resolve conflicts UI are already there for plain text (the code include the full merge add-on of code mirror) and notebooks (nbdime is included). So this should ease subsequent PRs for conflict resolution.

[ianhi]

provide a way to carry a simple merge

This is indeed my goal.

You can configure git to use nbmerge as the merge driver for ipynb files. Unfortunately there does not seem to be a way to do this as an argument to git merge. Instead you need to modify a .gitattributes file or .git/config (ref)

To associate the merge driver with a file type, add the following entry to the appropriate .gitattributes file:

*.ipynb merge=jupyternotebook

Introducing changes to settings of a git repo seems a bit invasive. Maybe this extension could toggle this setting before doing a merge git-nbmergedriver config --enable and then disable it afterwards.

But I think it makes sense to limit this PR to just implementing standard git merge.

[ianhi]

Oops. clicked the wrong button. didn't intend to close