merge: move applyResult() to the MergeRequest
authorAleksey Pivovarov <AMPivovarov@gmail.com>
Mon, 10 Aug 2015 17:15:10 +0000 (20:15 +0300)
committerAleksey Pivovarov <AMPivovarov@gmail.com>
Wed, 12 Aug 2015 16:55:44 +0000 (19:55 +0300)
it should be called always and only once, so it's easier to do it in MergeRequestProcessor, instead of specific tool.

platform/diff-api/src/com/intellij/diff/merge/MergeContext.java
platform/diff-api/src/com/intellij/diff/merge/MergeRequest.java
platform/diff-api/src/com/intellij/diff/merge/MergeTool.java
platform/diff-api/src/com/intellij/diff/merge/ThreesideMergeRequest.java
platform/diff-impl/src/com/intellij/diff/merge/BinaryMergeTool.java
platform/diff-impl/src/com/intellij/diff/merge/ErrorMergeTool.java
platform/diff-impl/src/com/intellij/diff/merge/MergeRequestProcessor.java
platform/diff-impl/src/com/intellij/diff/merge/TextMergeTool.java
platform/diff-impl/src/com/intellij/diff/tools/external/ExternalDiffToolUtil.java

index 9bdcd100c45a86e361d254613d75ae1b6e631882..6727d7e6eafdaa53d2debaa29f2bd095156c3a93 100644 (file)
@@ -19,6 +19,7 @@ import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.Key;
 import com.intellij.openapi.util.UserDataHolder;
 import com.intellij.openapi.util.UserDataHolderBase;
+import org.jetbrains.annotations.CalledInAwt;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -32,7 +33,8 @@ public abstract class MergeContext implements UserDataHolder {
 
   public abstract void requestFocus();
 
-  public abstract void closeDialog();
+  @CalledInAwt
+  public abstract void finishMerge(@NotNull MergeResult result);
 
   @Nullable
   @Override
index 7581cde8b94cc3f66548cf724b52e4fd01c3689f..3eb03a8a449550a90bf3737f5836da48cd583d38 100644 (file)
@@ -18,6 +18,7 @@ package com.intellij.diff.merge;
 import com.intellij.openapi.util.Key;
 import com.intellij.openapi.util.UserDataHolder;
 import com.intellij.openapi.util.UserDataHolderBase;
+import org.jetbrains.annotations.CalledInAwt;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -27,6 +28,12 @@ public abstract class MergeRequest implements UserDataHolder {
   @Nullable
   public abstract String getTitle();
 
+  /*
+   * Called on conflict resolve end.
+   */
+  @CalledInAwt
+  public abstract void applyResult(@NotNull MergeResult result);
+
   @Nullable
   @Override
   public <T> T getUserData(@NotNull Key<T> key) {
index 31978e0c5562b0969f019a29b905809705f8a7d9..e105ee70627234b1c9ecd828970c90e26d80586a 100644 (file)
@@ -35,6 +35,11 @@ public interface MergeTool {
 
   boolean canShow(@NotNull MergeContext context, @NotNull MergeRequest request);
 
+  /*
+   * Merge viewer should call MergeContext.finishMerge(MergeResult) when processing is over.
+   *
+   * MergeRequest.applyResult() will be performed by the caller, so it shouldn't be called by MergeViewer directly.
+   */
   interface MergeViewer extends Disposable {
     @NotNull
     JComponent getComponent();
index 863be10c467737e1b0ff18f915fab669009c738c..982402b26ba99b0d871ed59dcb34228a896155a8 100644 (file)
@@ -37,7 +37,4 @@ public abstract class ThreesideMergeRequest extends MergeRequest {
    */
   @NotNull
   public abstract List<String> getContentTitles();
-
-  @CalledInAwt
-  public abstract void applyResult(@NotNull MergeResult result);
 }
index d2ee066e701fb7f989ad58b5b892c7bb4194dcf8..e254ed9c56282501b3f15957ccdc6e0f99b9f362 100644 (file)
@@ -62,8 +62,6 @@ public class BinaryMergeTool implements MergeTool {
 
     @NotNull private final MyThreesideViewer myViewer;
 
-    private boolean myConflictResolved;
-
     public BinaryMergeViewer(@NotNull MergeContext context, @NotNull ThreesideMergeRequest request) {
       myMergeContext = context;
       myMergeRequest = request;
@@ -125,9 +123,7 @@ public class BinaryMergeTool implements MergeTool {
 
         @Override
         public void perform(@NotNull MergeResult result) {
-          markConflictResolved();
-          myMergeRequest.applyResult(result);
-          myMergeContext.closeDialog();
+          myMergeContext.finishMerge(result);
         }
       });
     }
@@ -143,23 +139,14 @@ public class BinaryMergeTool implements MergeTool {
 
     @NotNull
     public MyThreesideViewer getViewer() {
-      if (!isConflictResolved()) myMergeRequest.applyResult(MergeResult.CANCEL);
       return myViewer;
     }
 
-    public boolean isConflictResolved() {
-      return myConflictResolved;
-    }
-
-    public void markConflictResolved() {
-      myConflictResolved = true;
-    }
-
     //
     // Viewer
     //
 
-    private class MyThreesideViewer extends ThreesideBinaryDiffViewer {
+    private static class MyThreesideViewer extends ThreesideBinaryDiffViewer {
       public MyThreesideViewer(@NotNull DiffContext context, @NotNull DiffRequest request) {
         super(context, request);
       }
index 470dbbf26393197eba6f3e4ec22982d8d0c024b1..7ce439dfb27bb0db26b97fcb553df63fe0344077 100644 (file)
@@ -79,18 +79,12 @@ public class ErrorMergeTool implements MergeTool {
       return MergeUtil.createBottomAction(new MergeUtil.AcceptActionProcessor() {
         @Override
         public boolean isVisible(@NotNull MergeResult result) {
-          if (myMergeRequest instanceof ThreesideMergeRequest) {
-            return result != MergeResult.RESOLVED;
-          }
-          else {
-            return result == MergeResult.CANCEL;
-          }
+          return result == MergeResult.CANCEL;
         }
 
         @Override
         public void perform(@NotNull MergeResult result) {
-          if (myMergeRequest instanceof ThreesideMergeRequest) ((ThreesideMergeRequest)myMergeRequest).applyResult(result);
-          myMergeContext.closeDialog();
+          myMergeContext.finishMerge(result);
         }
       });
     }
index 0b408f025b465db9104fded3f31b4e9ac620dd05..528fc3b5c023d59fc5ae5eda3c3c5d058890c7b4 100644 (file)
@@ -71,6 +71,7 @@ public abstract class MergeRequestProcessor implements Disposable {
   @NotNull private final MergeTool.MergeViewer myViewer;
 
   @Nullable private BooleanGetter myCloseHandler;
+  private boolean myConflictResolved = false;
 
   public MergeRequestProcessor(@Nullable Project project, @NotNull MergeRequest request) {
     myProject = project;
@@ -204,12 +205,25 @@ public abstract class MergeRequestProcessor implements Disposable {
     });
   }
 
+  @CalledInAwt
+  private void applyRequestResult(@NotNull MergeResult result) {
+    if (myConflictResolved) return;
+    myConflictResolved = true;
+    try {
+      myRequest.applyResult(result);
+    }
+    catch (Exception e) {
+      LOG.error(e);
+    }
+  }
+
   //
   // Abstract
   //
 
   @CalledInAwt
   protected void onDispose() {
+    applyRequestResult(MergeResult.CANCEL);
   }
 
   protected void setWindowTitle(@NotNull String title) {
@@ -252,7 +266,7 @@ public abstract class MergeRequestProcessor implements Disposable {
   }
 
   public boolean checkCloseAction() {
-    if (myCloseHandler != null && !myCloseHandler.get()) {
+    if (!myConflictResolved && myCloseHandler != null && !myCloseHandler.get()) {
       return Messages.showYesNoDialog(myPanel.getRootPane(),
                                       DiffBundle.message("merge.dialog.exit.without.applying.changes.confirmation.message"),
                                       DiffBundle.message("cancel.visual.merge.dialog.title"),
@@ -413,7 +427,8 @@ public abstract class MergeRequestProcessor implements Disposable {
     }
 
     @Override
-    public void closeDialog() {
+    public void finishMerge(@NotNull MergeResult result) {
+      applyRequestResult(result);
       MergeRequestProcessor.this.closeDialog();
     }
   }
index e68523de0ac260ea4c0da35afbdbc791cd182685..c165c1cb9fa32e8f2c29095f2ae5360f60ceb07e 100644 (file)
@@ -105,8 +105,6 @@ public class TextMergeTool implements MergeTool {
 
     @NotNull private final MyThreesideViewer myViewer;
 
-    private boolean myConflictResolved;
-
     public TextMergeViewer(@NotNull MergeContext context, @NotNull TextMergeRequest request) {
       myMergeContext = context;
       myMergeRequest = request;
@@ -165,11 +163,10 @@ public class TextMergeTool implements MergeTool {
       components.closeHandler = new BooleanGetter() {
         @Override
         public boolean get() {
-          return isConflictResolved();
+          return myViewer.isContentModified();
         }
       };
 
-
       return components;
     }
 
@@ -182,7 +179,6 @@ public class TextMergeTool implements MergeTool {
     @Override
     public void dispose() {
       Disposer.dispose(myViewer);
-      if (!isConflictResolved()) myMergeRequest.applyResult(MergeResult.CANCEL);
     }
 
     //
@@ -194,14 +190,6 @@ public class TextMergeTool implements MergeTool {
       return myViewer;
     }
 
-    public boolean isConflictResolved() {
-      return myConflictResolved;
-    }
-
-    public void markConflictResolved() {
-      myConflictResolved = true;
-    }
-
     //
     // Viewer
     //
@@ -211,9 +199,12 @@ public class TextMergeTool implements MergeTool {
 
       // all changes - both applied and unapplied ones
       @NotNull private final List<TextMergeChange> myAllMergeChanges = new ArrayList<TextMergeChange>();
-      private boolean myInitialRediffDone;
-      @Nullable private MergeCommandAction myCurrentMergeCommand;
 
+      private boolean myInitialRediffStarted;
+      private boolean myInitialRediffFinished;
+      private boolean myContentModified;
+
+      @Nullable private MergeCommandAction myCurrentMergeCommand;
       private int myBulkChangeUpdateDepth;
 
       private final Set<TextMergeChange> myChangesToUpdate = new HashSet<TextMergeChange>();
@@ -317,10 +308,8 @@ public class TextMergeTool implements MergeTool {
                 return;
               }
             }
-            markConflictResolved();
             destroyChangedBlocks();
-            myMergeRequest.applyResult(result);
-            myMergeContext.closeDialog();
+            myMergeContext.finishMerge(result);
           }
         };
       }
@@ -349,12 +338,18 @@ public class TextMergeTool implements MergeTool {
       @Override
       @CalledInAwt
       public void rediff(boolean trySync) {
-        if (myInitialRediffDone) return;
-        myInitialRediffDone = true;
+        if (myInitialRediffStarted) return;
+        myInitialRediffStarted = true;
         assert myAllMergeChanges.isEmpty();
         doRediff();
       }
 
+      @NotNull
+      @Override
+      protected Runnable performRediff(@NotNull ProgressIndicator indicator) {
+        throw new UnsupportedOperationException();
+      }
+
       @CalledInAwt
       private void doRediff() {
         myStatusPanel.setBusy(true);
@@ -385,14 +380,12 @@ public class TextMergeTool implements MergeTool {
 
               @Override
               public void run(@NotNull ProgressIndicator indicator) {
-                myCallback = performRediff(sequences, outputModificationStamp, indicator);
+                myCallback = doPerformRediff(sequences, outputModificationStamp, indicator);
               }
 
               @Override
               public void onCancel() {
-                markConflictResolved();
-                myMergeRequest.applyResult(MergeResult.CANCEL);
-                myMergeContext.closeDialog();
+                myMergeContext.finishMerge(MergeResult.CANCEL);
               }
 
               @Override
@@ -405,15 +398,9 @@ public class TextMergeTool implements MergeTool {
       }
 
       @NotNull
-      @Override
-      protected Runnable performRediff(@NotNull ProgressIndicator indicator) {
-        throw new UnsupportedOperationException();
-      }
-
-      @NotNull
-      protected Runnable performRediff(@NotNull List<CharSequence> sequences,
-                                       long outputModificationStamp,
-                                       @NotNull ProgressIndicator indicator) {
+      protected Runnable doPerformRediff(@NotNull List<CharSequence> sequences,
+                                         long outputModificationStamp,
+                                         @NotNull ProgressIndicator indicator) {
         try {
           indicator.checkCanceled();
 
@@ -464,6 +451,8 @@ public class TextMergeTool implements MergeTool {
             myStatusPanel.update();
 
             getEditor(ThreeSide.BASE).setViewer(false);
+
+            myInitialRediffFinished = true;
           }
         };
       }
@@ -500,6 +489,8 @@ public class TextMergeTool implements MergeTool {
           return;
         }
 
+        if (myInitialRediffFinished) myContentModified = true;
+
         int line1 = e.getDocument().getLineNumber(e.getOffset());
         int line2 = e.getDocument().getLineNumber(e.getOffset() + e.getOldLength()) + 1;
         int shift = DiffUtil.countLinesShift(e);
@@ -589,10 +580,8 @@ public class TextMergeTool implements MergeTool {
               HyperlinkListener listener = new HyperlinkAdapter() {
                 @Override
                 protected void hyperlinkActivated(HyperlinkEvent e) {
-                  markConflictResolved();
                   destroyChangedBlocks();
-                  myMergeRequest.applyResult(MergeResult.RESOLVED);
-                  myMergeContext.closeDialog();
+                  myMergeContext.finishMerge(MergeResult.RESOLVED);
                 }
               };
 
@@ -614,6 +603,10 @@ public class TextMergeTool implements MergeTool {
       // Getters
       //
 
+      public boolean isContentModified() {
+        return myContentModified;
+      }
+
       @NotNull
       public List<TextMergeChange> getAllChanges() {
         return myAllMergeChanges;
@@ -685,6 +678,7 @@ public class TextMergeTool implements MergeTool {
         @CalledWithWriteLock
         protected final void execute() {
           LOG.assertTrue(myCurrentMergeCommand == null);
+          myContentModified = true;
 
           // We should restore states after changes in document (by DocumentUndoProvider) to avoid corruption by our onBeforeDocumentChange()
           // Undo actions are performed in backward order, while redo actions are performed in forward order.
index 22d5c51e2125c877ba72561986dbc32cf1347ee0..0d075f734268278519cae0a7a686aac8bd395476 100644 (file)
@@ -210,109 +210,108 @@ public class ExternalDiffToolUtil {
                                   @NotNull ExternalDiffSettings settings,
                                   @NotNull ThreesideMergeRequest request)
     throws IOException, ExecutionException {
-    DiffContent outputContent = request.getOutputContent();
-    List<? extends DiffContent> contents = request.getContents();
-    List<String> titles = request.getContentTitles();
-    String windowTitle = request.getTitle();
-
-    assert contents.size() == 3;
-    assert titles.size() == contents.size();
-
-    List<String> files = new ArrayList<String>();
-    for (int i = 0; i < contents.size(); i++) {
-      files.add(createFile(contents.get(i), titles.get(i), windowTitle));
-    }
-
-    OutputFile outputFile = createOutputFile(outputContent, windowTitle);
-
-    CommandLineTokenizer parameterTokenizer = new CommandLineTokenizer(settings.getMergeParameters(), true);
-
-    List<String> args = new ArrayList<String>();
-    while (parameterTokenizer.hasMoreTokens()) {
-      String arg = parameterTokenizer.nextToken();
-      if ("%1".equals(arg)) {
-        args.add(files.get(0));
-      }
-      else if ("%2".equals(arg)) {
-        args.add(files.get(2));
+    boolean success = false;
+    try {
+      DiffContent outputContent = request.getOutputContent();
+      List<? extends DiffContent> contents = request.getContents();
+      List<String> titles = request.getContentTitles();
+      String windowTitle = request.getTitle();
+
+      assert contents.size() == 3;
+      assert titles.size() == contents.size();
+
+      List<String> files = new ArrayList<String>();
+      for (int i = 0; i < contents.size(); i++) {
+        files.add(createFile(contents.get(i), titles.get(i), windowTitle));
       }
-      else if ("%3".equals(arg)) {
-        args.add(files.get(1));
-      }
-      else if ("%4".equals(arg)) {
-        args.add(outputFile.getPath());
-      }
-      else {
-        args.add(arg);
-      }
-    }
 
-    GeneralCommandLine commandLine = new GeneralCommandLine();
-    commandLine.setExePath(settings.getMergeExePath());
+      OutputFile outputFile = createOutputFile(outputContent, windowTitle);
 
-    commandLine.addParameters(args);
-    final Process process = commandLine.createProcess();
+      CommandLineTokenizer parameterTokenizer = new CommandLineTokenizer(settings.getMergeParameters(), true);
 
-    boolean success;
-    if (settings.isMergeTrustExitCode()) {
-      final Ref<Boolean> resultRef = new Ref<Boolean>();
+      List<String> args = new ArrayList<String>();
+      while (parameterTokenizer.hasMoreTokens()) {
+        String arg = parameterTokenizer.nextToken();
+        if ("%1".equals(arg)) {
+          args.add(files.get(0));
+        }
+        else if ("%2".equals(arg)) {
+          args.add(files.get(2));
+        }
+        else if ("%3".equals(arg)) {
+          args.add(files.get(1));
+        }
+        else if ("%4".equals(arg)) {
+          args.add(outputFile.getPath());
+        }
+        else {
+          args.add(arg);
+        }
+      }
 
-      ProgressManager.getInstance().run(new Task.Modal(project, "Waiting for external tool", true) {
-        @Override
-        public void run(@NotNull ProgressIndicator indicator) {
-          final Semaphore semaphore = new Semaphore(0);
-
-          final Thread waiter = new Thread("external process waiter") {
-            @Override
-            public void run() {
-              try {
-                resultRef.set(process.waitFor() == 0);
-              }
-              catch (InterruptedException ignore) {
+      GeneralCommandLine commandLine = new GeneralCommandLine();
+      commandLine.setExePath(settings.getMergeExePath());
+
+      commandLine.addParameters(args);
+      final Process process = commandLine.createProcess();
+
+      if (settings.isMergeTrustExitCode()) {
+        final Ref<Boolean> resultRef = new Ref<Boolean>();
+
+        ProgressManager.getInstance().run(new Task.Modal(project, "Waiting for external tool", true) {
+          @Override
+          public void run(@NotNull ProgressIndicator indicator) {
+            final Semaphore semaphore = new Semaphore(0);
+
+            final Thread waiter = new Thread("external process waiter") {
+              @Override
+              public void run() {
+                try {
+                  resultRef.set(process.waitFor() == 0);
+                }
+                catch (InterruptedException ignore) {
+                }
+                finally {
+                  semaphore.release();
+                }
               }
-              finally {
-                semaphore.release();
+            };
+            waiter.start();
+
+            try {
+              while (true) {
+                indicator.checkCanceled();
+                if (semaphore.tryAcquire(200, TimeUnit.MILLISECONDS)) break;
               }
             }
-          };
-          waiter.start();
-
-          try {
-            while (true) {
-              indicator.checkCanceled();
-              if (semaphore.tryAcquire(200, TimeUnit.MILLISECONDS)) break;
+            catch (InterruptedException ignore) {
+            }
+            finally {
+              waiter.interrupt();
             }
           }
-          catch (InterruptedException ignore) {
-          }
-          finally {
-            waiter.interrupt();
-          }
-        }
-      });
+        });
 
-      success = resultRef.get() == Boolean.TRUE;
-    }
-    else {
-      ProgressManager.getInstance().run(new Task.Modal(project, "Launching external tool", false) {
-        @Override
-        public void run(@NotNull ProgressIndicator indicator) {
-          indicator.setIndeterminate(true);
-          TimeoutUtil.sleep(1000);
-        }
-      });
+        success = resultRef.get() == Boolean.TRUE;
+      }
+      else {
+        ProgressManager.getInstance().run(new Task.Modal(project, "Launching external tool", false) {
+          @Override
+          public void run(@NotNull ProgressIndicator indicator) {
+            indicator.setIndeterminate(true);
+            TimeoutUtil.sleep(1000);
+          }
+        });
 
-      success = Messages.showYesNoDialog(project,
-                                         "Press \"Mark as Resolved\" when you finish resolving conflicts in the external tool",
-                                         "Merge In External Tool", "Mark as Resolved", "Revert", null) == Messages.YES;
-    }
+        success = Messages.showYesNoDialog(project,
+                                           "Press \"Mark as Resolved\" when you finish resolving conflicts in the external tool",
+                                           "Merge In External Tool", "Mark as Resolved", "Revert", null) == Messages.YES;
+      }
 
-    if (success) {
-      outputFile.finish();
-      request.applyResult(MergeResult.RESOLVED);
+      if (success) outputFile.finish();
     }
-    else {
-      request.applyResult(MergeResult.CANCEL);
+    finally {
+      request.applyResult(success ? MergeResult.RESOLVED : MergeResult.CANCEL);
     }
   }