Merge pull request #1013 from MLH-Fellowship/feat/diff-merge-conflicts

Diff and resolve files on merge conflict

Author: fcollonval

jupyterlab_git/git.py

@@ -14,7 +14,7 @@
 import pexpect
 import tornado
 import tornado.locks
-from nbdime import diff_notebooks
+from nbdime import diff_notebooks, merge_notebooks
 from jupyter_server.utils import ensure_async
 
 from .log import get_logger
@@ -320,14 +320,20 @@ async def fetch(self, path):
 
         return result
 
-    async def get_nbdiff(self, prev_content: str, curr_content: str) -> dict:
+    async def get_nbdiff(
+        self, prev_content: str, curr_content: str, base_content=None
+    ) -> dict:
         """Compute the diff between two notebooks.
 
         Args:
             prev_content: Notebook previous content
             curr_content: Notebook current content
+            base_content: Notebook base content - only passed during a merge conflict
         Returns:
-            {"base": Dict, "diff": Dict}
+            if not base_content:
+                {"base": Dict, "diff": Dict}
+            else:
+                {"base": Dict, "merge_decisions": Dict}
         """
 
         def read_notebook(content):
@@ -345,14 +351,35 @@ def read_notebook(content):
             else:
                 return nbformat.reads(content, as_version=4)
 
+        # TODO Fix this in nbdime
+        def remove_cell_ids(nb):
+            for cell in nb.cells:
+                del cell["id"]
+            return nb
+
         current_loop = tornado.ioloop.IOLoop.current()
         prev_nb = await current_loop.run_in_executor(None, read_notebook, prev_content)
         curr_nb = await current_loop.run_in_executor(None, read_notebook, curr_content)
-        thediff = await current_loop.run_in_executor(
-            None, diff_notebooks, prev_nb, curr_nb
-        )
+        if base_content:
+            base_nb = await current_loop.run_in_executor(
+                None, read_notebook, base_content
+            )
+            # Only remove ids from merge_notebooks as a workaround
+            _, merge_decisions = await current_loop.run_in_executor(
+                None,
+                merge_notebooks,
+                remove_cell_ids(base_nb),
+                remove_cell_ids(prev_nb),
+                remove_cell_ids(curr_nb),
+            )
+
+            return {"base": base_nb, "merge_decisions": merge_decisions}
+        else:
+            thediff = await current_loop.run_in_executor(
+                None, diff_notebooks, prev_nb, curr_nb
+            )
 
-        return {"base": prev_nb, "diff": thediff}
+            return {"base": prev_nb, "diff": thediff}
 
     async def status(self, path):
         """

jupyterlab_git/handlers.py

@@ -705,7 +705,10 @@ async def post(self):
                 status_code=400, reason=f"Missing POST key: {e}"
             )
         try:
-            content = await self.git.get_nbdiff(prev_content, curr_content)
+            base_content = data.get("baseContent")
+            content = await self.git.get_nbdiff(
+                prev_content, curr_content, base_content
+            )
         except Exception as e:
             get_logger().error(f"Error computing notebook diff.", exc_info=e)
             raise tornado.web.HTTPError(

src/commandsAndMenu.tsx

@@ -104,7 +104,7 @@ export function addCommands(
   settings: ISettingRegistry.ISettings,
   trans: TranslationBundle
 ): void {
-  const { commands, shell } = app;
+  const { commands, shell, serviceManager } = app;
 
   /**
    * Commit using a keystroke combination when in CommitBox.
@@ -419,16 +419,17 @@ export function addCommands(
   /**
    * Git display diff command - internal command
    *
-   * @params model {Git.Diff.IModel<string>}: The diff model to display
+   * @params model {Git.Diff.IModel: The diff model to display
    * @params isText {boolean}: Optional, whether the content is a plain text
+   * @params isMerge {boolean}: Optional, whether the diff is a merge conflict
    * @returns the main area widget or null
    */
   commands.addCommand(CommandIDs.gitShowDiff, {
     label: trans.__('Show Diff'),
     caption: trans.__('Display a file diff.'),
     execute: async args => {
       const { model, isText } = args as any as {
-        model: Git.Diff.IModel<string>;
+        model: Git.Diff.IModel;
         isText?: boolean;
       };
 
@@ -470,26 +471,67 @@ export function addCommands(
 
             diffWidget.toolbar.addItem('spacer', Toolbar.createSpacerItem());
 
-            const refreshButton = new ToolbarButton({
-              label: trans.__('Refresh'),
-              onClick: async () => {
-                await widget.refresh();
-                refreshButton.hide();
-              },
-              tooltip: trans.__('Refresh diff widget'),
-              className: 'jp-git-diff-refresh'
-            });
-            refreshButton.hide();
-            diffWidget.toolbar.addItem('refresh', refreshButton);
-
-            const refresh = () => {
-              refreshButton.show();
-            };
+            // Do not allow the user to refresh during merge conflicts
+            if (model.hasConflict) {
+              const resolveButton = new ToolbarButton({
+                label: trans.__('Mark as resolved'),
+                onClick: async () => {
+                  if (!widget.isFileResolved) {
+                    const result = await showDialog({
+                      title: trans.__('Resolve with conflicts'),
+                      body: trans.__(
+                        'Are you sure you want to mark this file as resolved with merge conflicts?'
+                      )
+                    });
+
+                    // Bail early if the user wants to finish resolving conflicts
+                    if (!result.button.accept) {
+                      return;
+                    }
+                  }
+
+                  try {
+                    await serviceManager.contents.save(
+                      model.filename,
+                      await widget.getResolvedFile()
+                    );
+                    await gitModel.add(model.filename);
+                    await gitModel.refresh();
+                  } catch (reason) {
+                    logger.log({
+                      message: reason.message ?? reason,
+                      level: Level.ERROR
+                    });
+                  } finally {
+                    diffWidget.dispose();
+                  }
+                },
+                tooltip: trans.__('Mark file as resolved'),
+                className: 'jp-git-diff-resolve'
+              });
+
+              diffWidget.toolbar.addItem('resolve', resolveButton);
+            } else {
+              const refreshButton = new ToolbarButton({
+                label: trans.__('Refresh'),
+                onClick: async () => {
+                  await widget.refresh();
+                  refreshButton.hide();
+                },
+                tooltip: trans.__('Refresh diff widget'),
+                className: 'jp-git-diff-refresh'
+              });
+
+              refreshButton.hide();
+              diffWidget.toolbar.addItem('refresh', refreshButton);
+
+              const refresh = () => {
+                refreshButton.show();
+              };
 
-            model.changed.connect(refresh);
-            widget.disposed.connect(() => {
-              model.changed.disconnect(refresh);
-            });
+              model.changed.connect(refresh);
+              widget.disposed.connect(() => model.changed.disconnect(refresh));
+            }
 
             // Load the diff widget
             modelIsLoading.resolve();
@@ -569,25 +611,29 @@ export function addCommands(
 
         const repositoryPath = gitModel.getRelativeFilePath();
         const filename = PathExt.join(repositoryPath, filePath);
-
-        let diffContext = context;
-        if (!diffContext) {
-          const specialRef =
-            status === 'staged'
-              ? Git.Diff.SpecialRef.INDEX
-              : Git.Diff.SpecialRef.WORKING;
-          diffContext = {
-            currentRef: specialRef,
-            previousRef: 'HEAD'
-          };
-        }
+        const specialRef =
+          status === 'staged'
+            ? Git.Diff.SpecialRef.INDEX
+            : Git.Diff.SpecialRef.WORKING;
+
+        const diffContext: Git.Diff.IContext =
+          status === 'unmerged'
+            ? {
+                currentRef: 'HEAD',
+                previousRef: 'MERGE_HEAD',
+                baseRef: 'ORIG_HEAD'
+              }
+            : context ?? {
+                currentRef: specialRef,
+                previousRef: 'HEAD'
+              };
 
         const challengerRef = Git.Diff.SpecialRef[diffContext.currentRef as any]
           ? { special: Git.Diff.SpecialRef[diffContext.currentRef as any] }
           : { git: diffContext.currentRef };
 
-        // Create the diff widget
-        const model = new DiffModel<string>({
+        // Base props used for Diff Model
+        const props: Omit<Git.Diff.IModel, 'changed' | 'hasConflict'> = {
           challenger: {
             content: async () => {
               return requestAPI<Git.IDiffContent>(
@@ -623,7 +669,32 @@ export function addCommands(
             source: diffContext.previousRef,
             updateAt: Date.now()
           }
-        });
+        };
+
+        if (diffContext.baseRef) {
+          props.reference.label = trans.__('CURRENT');
+          props.challenger.label = trans.__('INCOMING');
+
+          // Only add base when diff-ing merge conflicts
+          props.base = {
+            content: async () => {
+              return requestAPI<Git.IDiffContent>(
+                URLExt.join(repositoryPath, 'content'),
+                'POST',
+                {
+                  filename: filePath,
+                  reference: { git: diffContext.baseRef }
+                }
+              ).then(data => data.content);
+            },
+            label: trans.__('RESULT'),
+            source: diffContext.baseRef,
+            updateAt: Date.now()
+          };
+        }
+
+        // Create the diff widget
+        const model = new DiffModel(props);
 
         const widget = await commands.execute(CommandIDs.gitShowDiff, {
           model,

src/components/FileItem.tsx

@@ -173,7 +173,10 @@ export class FileItem extends React.PureComponent<IFileItemProps> {
   render(): JSX.Element {
     const { file } = this.props;
     const status_code = file.status === 'staged' ? file.x : file.y;
-    const status = this._getFileChangedLabel(status_code as any);
+    const status =
+      file.status === 'unmerged'
+        ? 'Conflicted'
+        : this._getFileChangedLabel(status_code as any);
 
     return (
       <div
@@ -205,7 +208,11 @@ export class FileItem extends React.PureComponent<IFileItemProps> {
         />
         {this.props.actions}
         <span className={this._getFileChangedLabelClass(this.props.file.y)}>
-          {this.props.file.y === '?' ? 'U' : status_code}
+          {this.props.file.status === 'unmerged'
+            ? '!'
+            : this.props.file.y === '?'
+            ? 'U'
+            : status_code}
         </span>
       </div>
     );