[task]: refactoring, api changed in VcsTaskHandler

* use validator instead of custom prohibited symbols;
* unified error messages;
* test added;

platform/vcs-api/src/com/intellij/openapi/vcs/VcsTaskHandler.java

@@ -20,18 +20,14 @@ import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.Condition;
 import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 
 import java.util.Collection;
 import java.util.List;
+import java.util.regex.Pattern;
 
-/**
- * @author Dmitry Avdeev
- *         Date: 16.07.13
- */
 public abstract class VcsTaskHandler {
 
-  public static final String DEFAULT_PROHIBITED_SYMBOLS = " ";
+  private static final String DEFAULT_PROHIBITED_SYMBOLS = " ";
 
   public static VcsTaskHandler[] getAllHandlers(final Project project) {
     VcsTaskHandler[] extensions = EXTENSION_POINT_NAME.getExtensions(project);
@@ -91,10 +87,23 @@ public abstract class VcsTaskHandler {
   public abstract TaskInfo[] getAllExistingTasks();
 
   /**
-   * @return REGEXP with prohibited symbols for replaceAll(REGEXP, other) future call, null if every symbol is possible
+   * Should check prohibited symbols and constructions; name ref conflicts depended on Repository will be checked separately if needed
+   *
+   * @param branchName to check
+   * @return true if valid
    */
-  @Nullable
-  public String getProhibitedSymbolsInBranchNames() {
-    return DEFAULT_PROHIBITED_SYMBOLS;
+  public boolean isBranchNameValid(@NotNull String branchName) {
+    return !Pattern.compile(DEFAULT_PROHIBITED_SYMBOLS).matcher(branchName).find();
+  }
+
+  /**
+   * Update branchName to valid
+   *
+   * @param suggestedName suggested name
+   * @return new valid branchName
+   */
+  @NotNull
+  public String cleanUpBranchName(@NotNull String suggestedName) {
+    return suggestedName.replaceAll(DEFAULT_PROHIBITED_SYMBOLS, "-");
   }
 }

plugins/git4idea/src/git4idea/GitTaskHandler.java

@@ -22,6 +22,7 @@ import com.intellij.util.containers.ContainerUtil;
 import git4idea.branch.GitBrancher;
 import git4idea.repo.GitRepository;
 import git4idea.repo.GitRepositoryManager;
+import git4idea.validators.GitRefNameValidator;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -35,10 +36,12 @@ import java.util.List;
 public class GitTaskHandler extends DvcsTaskHandler<GitRepository> {
 
   @NotNull private final GitBrancher myBrancher;
+  @NotNull private final GitRefNameValidator myNameValidator;
 
   public GitTaskHandler(@NotNull GitBrancher brancher, @NotNull GitRepositoryManager repositoryManager, @NotNull Project project) {
     super(repositoryManager, project, "branch");
     myBrancher = brancher;
+    myNameValidator = GitRefNameValidator.getInstance();
   }
 
   @Override
@@ -76,4 +79,15 @@ public class GitTaskHandler extends DvcsTaskHandler<GitRepository> {
       }
     });
   }
+
+  @Override
+  public boolean isBranchNameValid(@NotNull String branchName) {
+    return myNameValidator.checkInput(branchName);
+  }
+
+  @NotNull
+  @Override
+  public String cleanUpBranchName(@NotNull String suggestedName) {
+    return myNameValidator.cleanUpBranchName(suggestedName);
+  }
 }

plugins/git4idea/src/git4idea/validators/GitRefNameValidator.java

@@ -17,6 +17,7 @@ package git4idea.validators;
 
 import com.intellij.openapi.ui.InputValidator;
 import com.intellij.openapi.util.text.StringUtil;
+import org.jetbrains.annotations.NotNull;
 
 import java.util.regex.Pattern;
 
@@ -24,7 +25,6 @@ import java.util.regex.Pattern;
  * Checks that the specified String is a valid Git reference name.
  * See <a href="http://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html">
  * http://www.kernel.org/pub/software/scm/git/docs/git-check-ref-format.html</a>
- *
  */
 public final class GitRefNameValidator implements InputValidator {
 
@@ -66,4 +66,9 @@ public final class GitRefNameValidator implements InputValidator {
   public boolean canClose(String inputString) {
     return checkInput(inputString);
   }
+
+  @NotNull
+  public String cleanUpBranchName(@NotNull String branchName) {
+    return branchName.replaceAll(ILLEGAL.pattern(), "_");
+  }
 }

plugins/hg4idea/src/org/zmlx/hg4idea/HgTaskHandler.java

@@ -34,15 +34,19 @@ import org.zmlx.hg4idea.execution.HgCommandException;
 import org.zmlx.hg4idea.repo.HgRepository;
 import org.zmlx.hg4idea.repo.HgRepositoryManager;
 import org.zmlx.hg4idea.util.HgErrorUtil;
+import org.zmlx.hg4idea.util.HgReferenceValidator;
 import org.zmlx.hg4idea.util.HgUtil;
 
 import java.util.List;
 
 public class HgTaskHandler extends DvcsTaskHandler<HgRepository> {
 
+  private HgReferenceValidator myNameValidator;
+
   public HgTaskHandler(@NotNull HgRepositoryManager repositoryManager,
                        @NotNull Project project) {
     super(repositoryManager, project, "bookmark");
+    myNameValidator = HgReferenceValidator.getInstance();
   }
 
   @Override
@@ -104,8 +108,13 @@ public class HgTaskHandler extends DvcsTaskHandler<HgRepository> {
   }
 
   @Override
-  @Nullable
-  public String getProhibitedSymbolsInBranchNames() {
-    return null;
+  public boolean isBranchNameValid(@NotNull String branchName) {
+    return myNameValidator.checkInput(branchName);
+  }
+
+  @NotNull
+  @Override
+  public String cleanUpBranchName(@NotNull String suggestedName) {
+    return myNameValidator.cleanUpBranchName(suggestedName);
   }
 }

plugins/hg4idea/src/org/zmlx/hg4idea/util/HgReferenceValidator.java

@@ -17,6 +17,7 @@ package org.zmlx.hg4idea.util;
 
 import com.intellij.openapi.ui.InputValidatorEx;
 import com.intellij.openapi.util.text.StringUtil;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.regex.Pattern;
@@ -83,4 +84,10 @@ public class HgReferenceValidator implements InputValidatorEx {
   public String getErrorText(@Nullable String inputString) {
     return myErrorText;
   }
+
+  @NotNull
+  public String cleanUpBranchName(@NotNull String branchName) {
+    if (onlyDigits(branchName)) return branchName + "_";
+    return branchName.replaceAll(ILLEGAL.pattern(), "_");
+  }
 }

plugins/tasks/tasks-core/src/com/intellij/tasks/actions/OpenTaskDialog.java

@@ -44,7 +44,6 @@ import javax.swing.*;
 import java.awt.event.ActionEvent;
 import java.awt.event.ActionListener;
 import java.util.Collection;
-import java.util.regex.Pattern;
 
 /**
  * @author Dmitry Avdeev
@@ -174,11 +173,9 @@ public class OpenTaskDialog extends DialogWrapper {
           }
         });
       }
-
-      myBranchName.setText(
-        taskManager.suggestBranchName(task, myVcsTaskHandler != null
-                                            ? myVcsTaskHandler.getProhibitedSymbolsInBranchNames()
-                                            : VcsTaskHandler.DEFAULT_PROHIBITED_SYMBOLS));
+      myBranchName.setText(myVcsTaskHandler != null
+                           ? myVcsTaskHandler.cleanUpBranchName(taskManager.constructDefaultBranchName(task))
+                           : taskManager.suggestBranchName(task));
       myChangelistName.setText(taskManager.getChangelistName(task));
     }
     updateFields(true);
@@ -262,11 +259,8 @@ public class OpenTaskDialog extends DialogWrapper {
       if (branchName.isEmpty()) {
         return new ValidationInfo("Branch name should not be empty", myBranchName);
       }
-      else if (myVcsTaskHandler != null) {
-        String regExp = myVcsTaskHandler.getProhibitedSymbolsInBranchNames();
-        if (regExp != null && (Pattern.compile(regExp).matcher(branchName)).find()) {
-          return new ValidationInfo("Branch name contains prohibited symbols or regexp: \'" + regExp + "\'");
-        }
+      else if (myVcsTaskHandler != null && !myVcsTaskHandler.isBranchNameValid(branchName)){
+        return new ValidationInfo("Branch name is not valid; check your vcs branch name restrictions.");
       }
       else if (branchName.contains(" ")) {
         return new ValidationInfo("Branch name should not contain spaces");

plugins/tasks/tasks-core/src/com/intellij/tasks/impl/TaskManagerImpl.java

@@ -997,22 +997,18 @@ public class TaskManagerImpl extends TaskManager implements ProjectComponent, Pe
     return StringUtil.shortenTextWithEllipsis(name, 100, 0);
   }
 
-  @TestOnly
+  @NotNull
   public String suggestBranchName(@NotNull Task task) {
-    return suggestBranchName(task, VcsTaskHandler.DEFAULT_PROHIBITED_SYMBOLS);
+    String name = constructDefaultBranchName(task);
+    if (task.isIssue()) return name.replace(' ', '-');
+    List<String> words = StringUtil.getWordsIn(name);
+    String[] strings = ArrayUtil.toStringArray(words);
+    return StringUtil.join(strings, 0, Math.min(2, strings.length), "-");
   }
 
-  public String suggestBranchName(@NotNull Task task, @Nullable String prohibitedRegExp) {
-    if (task.isIssue()) {
-      String formatTask = TaskUtil.formatTask(task, myConfig.branchNameFormat);
-      return prohibitedRegExp != null ? formatTask.replaceAll(prohibitedRegExp, "-") : formatTask;
-    }
-    else {
-      String summary = task.getSummary();
-      List<String> words = StringUtil.getWordsIn(summary);
-      String[] strings = ArrayUtil.toStringArray(words);
-      return StringUtil.join(strings, 0, Math.min(2, strings.length), "-");
-    }
+  @NotNull
+  public String constructDefaultBranchName(@NotNull Task task) {
+    return task.isIssue() ? TaskUtil.formatTask(task, myConfig.branchNameFormat) : task.getSummary();
   }
 
   @TestOnly

plugins/tasks/tasks-tests/test/com/intellij/tasks/vcs/TaskBranchesTest.java

@@ -206,6 +206,17 @@ public abstract class TaskBranchesTest extends PlatformTestCase {
     assertEquals(2, getNumberOfBranches(repository));
   }
 
+  public void testSuggestBranchName() {
+    initRepositories("community", "idea");
+    VcsTaskHandler[] handlers = VcsTaskHandler.getAllHandlers(getProject());
+    assertEquals(1, handlers.length);
+    VcsTaskHandler handler = handlers[0];
+    String startName = "-Hello, this is long name with : and $";
+    assertFalse(handler.isBranchNameValid(startName));
+    String cleanUpBranchName = handler.cleanUpBranchName(startName);
+    assertTrue(handler.isBranchNameValid(cleanUpBranchName));
+  }
+
   public void _testCurrentTasks() throws Exception {
     initRepositories("foo", "bar");
     VcsTaskHandler handler = VcsTaskHandler.getAllHandlers(getProject())[0];

plugins/tasks/tasks-tests/test/com/intellij/tasks/vcs/TaskVcsTest.java

@@ -387,7 +387,6 @@ public class TaskVcsTest extends CodeInsightFixtureTestCase {
 
     LocalTaskImpl withIllegalSymbolsInIssue = new LocalTaskImpl("1", "contains Illegal$Symbols");
     withIllegalSymbolsInIssue.setIssue(true);
-    assertEquals("1-contains-Illegal-Symbols", myTaskManager.suggestBranchName(withIllegalSymbolsInIssue, " |\\$"));
     assertEquals("1-contains-Illegal$Symbols", myTaskManager.suggestBranchName(withIllegalSymbolsInIssue));
   }