From 63bb0bcd4af3e70afe13173b4e3b9173fcb8eaea Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Mon, 28 Dec 2015 15:47:36 +0100 Subject: [PATCH] branch command: provide convenient and meaningful options help Added tests for all options, fixed multi-valued options parsing. Bug: 484951 Change-Id: I5558589049544ea6c84932bc01f1f9df09e1f682 Signed-off-by: Andrey Loskutov --- .../tst/org/eclipse/jgit/pgm/BranchTest.java | 184 +++++++++++++++++- .../jgit/pgm/internal/CLIText.properties | 5 + .../src/org/eclipse/jgit/pgm/Branch.java | 130 +++++++++---- .../eclipse/jgit/pgm/internal/CLIText.java | 2 + .../eclipse/jgit/pgm/opt/CmdLineParser.java | 1 + .../pgm/opt/OptionWithValuesListHandler.java | 52 +++++ 6 files changed, 330 insertions(+), 44 deletions(-) create mode 100644 org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/OptionWithValuesListHandler.java diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/BranchTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/BranchTest.java index f369577ba..74506bc94 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/BranchTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/BranchTest.java @@ -43,6 +43,11 @@ package org.eclipse.jgit.pgm; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.File; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.CLIRepositoryTestCase; @@ -62,8 +67,9 @@ public void setUp() throws Exception { @Test public void testList() throws Exception { + assertEquals("* master", toString(execute("git branch"))); assertEquals("* master 6fd41be initial commit", - execute("git branch -v")[0]); + toString(execute("git branch -v"))); } @Test @@ -71,8 +77,10 @@ public void testListDetached() throws Exception { RefUpdate updateRef = db.updateRef(Constants.HEAD, true); updateRef.setNewObjectId(db.resolve("6fd41be")); updateRef.update(); - assertEquals("* (no branch) 6fd41be initial commit", - execute("git branch -v")[0]); + assertEquals( + toString("* (no branch) 6fd41be initial commit", + "master 6fd41be initial commit"), + toString(execute("git branch -v"))); } @Test @@ -80,15 +88,175 @@ public void testListContains() throws Exception { new Git(db).branchCreate().setName("initial").call(); RevCommit second = new Git(db).commit().setMessage("second commit") .call(); - assertArrayOfLinesEquals(new String[] { " initial", "* master", "" }, - execute("git branch --contains 6fd41be")); - assertArrayOfLinesEquals(new String[] { "* master", "" }, - execute("git branch --contains " + second.name())); + assertEquals(toString(" initial", "* master"), + toString(execute("git branch --contains 6fd41be"))); + assertEquals("* master", + toString(execute("git branch --contains " + second.name()))); } @Test public void testExistingBranch() throws Exception { assertEquals("fatal: A branch named 'master' already exists.", - executeUnchecked("git branch master")[0]); + toString(executeUnchecked("git branch master"))); + } + + @Test + public void testRenameSingleArg() throws Exception { + try { + toString(execute("git branch -m")); + fail("Must die"); + } catch (Die e) { + // expected, requires argument + } + String result = toString(execute("git branch -m slave")); + assertEquals("", result); + result = toString(execute("git branch -a")); + assertEquals("* slave", result); + } + + @Test + public void testRenameTwoArgs() throws Exception { + String result = toString(execute("git branch -m master slave")); + assertEquals("", result); + result = toString(execute("git branch -a")); + assertEquals("* slave", result); + } + + @Test + public void testCreate() throws Exception { + try { + toString(execute("git branch a b")); + fail("Must die"); + } catch (Die e) { + // expected, too many arguments + } + String result = toString(execute("git branch second")); + assertEquals("", result); + result = toString(execute("git branch")); + assertEquals(toString("* master", "second"), result); + result = toString(execute("git branch -v")); + assertEquals(toString("* master 6fd41be initial commit", + "second 6fd41be initial commit"), result); + } + + @Test + public void testDelete() throws Exception { + try { + toString(execute("git branch -d")); + fail("Must die"); + } catch (Die e) { + // expected, requires argument + } + String result = toString(execute("git branch second")); + assertEquals("", result); + result = toString(execute("git branch -d second")); + assertEquals("", result); + result = toString(execute("git branch")); + assertEquals("* master", result); + } + + @Test + public void testDeleteMultiple() throws Exception { + String result = toString(execute("git branch second", + "git branch third", "git branch fourth")); + assertEquals("", result); + result = toString(execute("git branch -d second third fourth")); + assertEquals("", result); + result = toString(execute("git branch")); + assertEquals("* master", result); + } + + @Test + public void testDeleteForce() throws Exception { + try { + toString(execute("git branch -D")); + fail("Must die"); + } catch (Die e) { + // expected, requires argument + } + String result = toString(execute("git branch second")); + assertEquals("", result); + result = toString(execute("git checkout second")); + assertEquals("Switched to branch 'second'", result); + + File a = writeTrashFile("a", "a"); + assertTrue(a.exists()); + execute("git add a", "git commit -m 'added a'"); + + result = toString(execute("git checkout master")); + assertEquals("Switched to branch 'master'", result); + + result = toString(execute("git branch")); + assertEquals(toString("* master", "second"), result); + + try { + toString(execute("git branch -d second")); + fail("Must die"); + } catch (Die e) { + // expected, the current HEAD is on second and not merged to master + } + result = toString(execute("git branch -D second")); + assertEquals("", result); + + result = toString(execute("git branch")); + assertEquals("* master", result); + } + + @Test + public void testDeleteForceMultiple() throws Exception { + String result = toString(execute("git branch second", + "git branch third", "git branch fourth")); + + assertEquals("", result); + result = toString(execute("git checkout second")); + assertEquals("Switched to branch 'second'", result); + + File a = writeTrashFile("a", "a"); + assertTrue(a.exists()); + execute("git add a", "git commit -m 'added a'"); + + result = toString(execute("git checkout master")); + assertEquals("Switched to branch 'master'", result); + + result = toString(execute("git branch")); + assertEquals(toString("fourth", "* master", "second", "third"), result); + + try { + toString(execute("git branch -d second third fourth")); + fail("Must die"); + } catch (Die e) { + // expected, the current HEAD is on second and not merged to master + } + result = toString(execute("git branch")); + assertEquals(toString("fourth", "* master", "second", "third"), result); + + result = toString(execute("git branch -D second third fourth")); + assertEquals("", result); + + result = toString(execute("git branch")); + assertEquals("* master", result); + } + + @Test + public void testCreateFromOldCommit() throws Exception { + File a = writeTrashFile("a", "a"); + assertTrue(a.exists()); + execute("git add a", "git commit -m 'added a'"); + File b = writeTrashFile("b", "b"); + assertTrue(b.exists()); + execute("git add b", "git commit -m 'added b'"); + String result = toString(execute("git log -n 1 --reverse")); + String firstCommitId = result.substring("commit ".length(), + result.indexOf('\n')); + + result = toString(execute("git branch -f second " + firstCommitId)); + assertEquals("", result); + + result = toString(execute("git branch")); + assertEquals(toString("* master", "second"), result); + + result = toString(execute("git checkout second")); + assertEquals("Switched to branch 'second'", result); + assertFalse(b.exists()); } } diff --git a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties index 3dcbda3b9..a24b81794 100644 --- a/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties +++ b/org.eclipse.jgit.pgm/resources/org/eclipse/jgit/pgm/internal/CLIText.properties @@ -20,6 +20,7 @@ branchAlreadyExists=A branch named ''{0}'' already exists. branchCreatedFrom=branch: Created from {0} branchDetachedHEAD=detached HEAD branchIsNotAnAncestorOfYourCurrentHEAD=The branch ''{0}'' is not an ancestor of your current HEAD.\nIf you are sure you want to delete it, run ''jgit branch -D {0}''. +branchNameRequired=branch name required branchNotFound=branch ''{0}'' not found. cacheTreePathInfo="{0}": {1} entries, {2} children cannotBeRenamed={0} cannot be renamed @@ -89,7 +90,9 @@ metaVar_author=AUTHOR metaVar_base=base metaVar_blameL=START,END metaVar_blameReverse=START..END +metaVar_branchAndStartPoint=branch [start-name] metaVar_branchName=branch +metaVar_branchNames=branch ... metaVar_bucket=BUCKET metaVar_command=command metaVar_commandDetail=DETAIL @@ -109,6 +112,7 @@ metaVar_message=message metaVar_n=n metaVar_name=name metaVar_object=object +metaVar_oldNewBranchNames=[oldbranch] newbranch metaVar_op=OP metaVar_pass=PASS metaVar_path=path @@ -125,6 +129,7 @@ metaVar_treeish=tree-ish metaVar_uriish=uri-ish metaVar_url=URL metaVar_user=USER +metaVar_values=value ... metaVar_version=VERSION mostCommonlyUsedCommandsAre=The most commonly used commands are: needApprovalToDestroyCurrentRepository=Need approval to destroy current repository diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Branch.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Branch.java index 65aa24f35..045f3571e 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Branch.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Branch.java @@ -45,7 +45,6 @@ import java.io.IOException; import java.text.MessageFormat; -import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; @@ -65,15 +64,18 @@ import org.eclipse.jgit.lib.RefUpdate.Result; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.pgm.internal.CLIText; -import org.eclipse.jgit.pgm.opt.CmdLineParser; +import org.eclipse.jgit.pgm.opt.OptionWithValuesListHandler; import org.eclipse.jgit.revwalk.RevWalk; import org.kohsuke.args4j.Argument; -import org.kohsuke.args4j.ExampleMode; import org.kohsuke.args4j.Option; @Command(common = true, usage = "usage_listCreateOrDeleteBranches") class Branch extends TextBuiltin { + private String otherBranch; + private boolean createForce; + private boolean rename; + @Option(name = "--remote", aliases = { "-r" }, usage = "usage_actOnRemoteTrackingBranches") private boolean remote = false; @@ -83,23 +85,69 @@ class Branch extends TextBuiltin { @Option(name = "--contains", metaVar = "metaVar_commitish", usage = "usage_printOnlyBranchesThatContainTheCommit") private String containsCommitish; - @Option(name = "--delete", aliases = { "-d" }, usage = "usage_deleteFullyMergedBranch") - private boolean delete = false; + private List delete; - @Option(name = "--delete-force", aliases = { "-D" }, usage = "usage_deleteBranchEvenIfNotMerged") - private boolean deleteForce = false; + @Option(name = "--delete", aliases = { + "-d" }, metaVar = "metaVar_branchNames", usage = "usage_deleteFullyMergedBranch", handler = OptionWithValuesListHandler.class) + public void delete(List names) { + if (names.isEmpty()) { + throw die(CLIText.get().branchNameRequired); + } + delete = names; + } - @Option(name = "--create-force", aliases = { "-f" }, usage = "usage_forceCreateBranchEvenExists") - private boolean createForce = false; + private List deleteForce; - @Option(name = "-m", usage = "usage_moveRenameABranch") - private boolean rename = false; + @Option(name = "--delete-force", aliases = { + "-D" }, metaVar = "metaVar_branchNames", usage = "usage_deleteBranchEvenIfNotMerged", handler = OptionWithValuesListHandler.class) + public void deleteForce(List names) { + if (names.isEmpty()) { + throw die(CLIText.get().branchNameRequired); + } + deleteForce = names; + } + + @Option(name = "--create-force", aliases = { + "-f" }, metaVar = "metaVar_branchAndStartPoint", usage = "usage_forceCreateBranchEvenExists", handler = OptionWithValuesListHandler.class) + public void createForce(List branchAndStartPoint) { + createForce = true; + if (branchAndStartPoint.isEmpty()) { + throw die(CLIText.get().branchNameRequired); + } + if (branchAndStartPoint.size() > 2) { + throw die(CLIText.get().tooManyRefsGiven); + } + if (branchAndStartPoint.size() == 1) { + branch = branchAndStartPoint.get(0); + } else { + branch = branchAndStartPoint.get(0); + otherBranch = branchAndStartPoint.get(1); + } + } + + @Option(name = "--move", aliases = { + "-m" }, metaVar = "metaVar_oldNewBranchNames", usage = "usage_moveRenameABranch", handler = OptionWithValuesListHandler.class) + public void moveRename(List currentAndNew) { + rename = true; + if (currentAndNew.isEmpty()) { + throw die(CLIText.get().branchNameRequired); + } + if (currentAndNew.size() > 2) { + throw die(CLIText.get().tooManyRefsGiven); + } + if (currentAndNew.size() == 1) { + branch = currentAndNew.get(0); + } else { + branch = currentAndNew.get(0); + otherBranch = currentAndNew.get(1); + } + } @Option(name = "--verbose", aliases = { "-v" }, usage = "usage_beVerbose") private boolean verbose = false; - @Argument - private List branches = new ArrayList(); + @Argument(metaVar = "metaVar_name") + private String branch; private final Map printRefs = new LinkedHashMap(); @@ -110,30 +158,33 @@ class Branch extends TextBuiltin { @Override protected void run() throws Exception { - if (delete || deleteForce) - delete(deleteForce); - else { - if (branches.size() > 2) - throw die(CLIText.get().tooManyRefsGiven + new CmdLineParser(this).printExample(ExampleMode.ALL)); - + if (delete != null || deleteForce != null) { + if (delete != null) { + delete(delete, false); + } + if (deleteForce != null) { + delete(deleteForce, true); + } + } else { if (rename) { String src, dst; - if (branches.size() == 1) { + if (otherBranch == null) { final Ref head = db.getRef(Constants.HEAD); - if (head != null && head.isSymbolic()) + if (head != null && head.isSymbolic()) { src = head.getLeaf().getName(); - else + } else { throw die(CLIText.get().cannotRenameDetachedHEAD); - dst = branches.get(0); + } + dst = branch; } else { - src = branches.get(0); + src = branch; final Ref old = db.getRef(src); if (old == null) throw die(MessageFormat.format(CLIText.get().doesNotExist, src)); if (!old.getName().startsWith(Constants.R_HEADS)) throw die(MessageFormat.format(CLIText.get().notABranch, src)); src = old.getName(); - dst = branches.get(1); + dst = otherBranch; } if (!dst.startsWith(Constants.R_HEADS)) @@ -145,13 +196,14 @@ protected void run() throws Exception { if (r.rename() != Result.RENAMED) throw die(MessageFormat.format(CLIText.get().cannotBeRenamed, src)); - } else if (branches.size() > 0) { - String newHead = branches.get(0); + } else if (createForce || branch != null) { + String newHead = branch; String startBranch; - if (branches.size() == 2) - startBranch = branches.get(1); - else + if (createForce) { + startBranch = otherBranch; + } else { startBranch = Constants.HEAD; + } Ref startRef = db.getRef(startBranch); ObjectId startAt = db.resolve(startBranch + "^0"); //$NON-NLS-1$ if (startRef != null) { @@ -164,22 +216,27 @@ protected void run() throws Exception { } startBranch = Repository.shortenRefName(startBranch); String newRefName = newHead; - if (!newRefName.startsWith(Constants.R_HEADS)) + if (!newRefName.startsWith(Constants.R_HEADS)) { newRefName = Constants.R_HEADS + newRefName; - if (!Repository.isValidRefName(newRefName)) + } + if (!Repository.isValidRefName(newRefName)) { throw die(MessageFormat.format(CLIText.get().notAValidRefName, newRefName)); - if (!createForce && db.resolve(newRefName) != null) + } + if (!createForce && db.resolve(newRefName) != null) { throw die(MessageFormat.format(CLIText.get().branchAlreadyExists, newHead)); + } RefUpdate updateRef = db.updateRef(newRefName); updateRef.setNewObjectId(startAt); updateRef.setForceUpdate(createForce); updateRef.setRefLogMessage(MessageFormat.format(CLIText.get().branchCreatedFrom, startBranch), false); Result update = updateRef.update(); - if (update == Result.REJECTED) + if (update == Result.REJECTED) { throw die(MessageFormat.format(CLIText.get().couldNotCreateBranch, newHead, update.toString())); + } } else { - if (verbose) + if (verbose) { rw = new RevWalk(db); + } list(); } } @@ -249,7 +306,8 @@ private void printHead(final ObjectReader reader, final String ref, outw.println(); } - private void delete(boolean force) throws IOException { + private void delete(List branches, boolean force) + throws IOException { String current = db.getBranch(); ObjectId head = db.resolve(Constants.HEAD); for (String branch : branches) { diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java index f5d581ad0..fd86ddf2b 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/internal/CLIText.java @@ -85,6 +85,7 @@ public static String formatLine(String line) { /***/ public String branchCreatedFrom; /***/ public String branchDetachedHEAD; /***/ public String branchIsNotAnAncestorOfYourCurrentHEAD; + /***/ public String branchNameRequired; /***/ public String branchNotFound; /***/ public String cacheTreePathInfo; /***/ public String configFileNotFound; @@ -184,6 +185,7 @@ public static String formatLine(String line) { /***/ public String metaVar_uriish; /***/ public String metaVar_url; /***/ public String metaVar_user; + /***/ public String metaVar_values; /***/ public String metaVar_version; /***/ public String mostCommonlyUsedCommandsAre; /***/ public String needApprovalToDestroyCurrentRepository; diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/CmdLineParser.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/CmdLineParser.java index 66d481642..a1e39502c 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/CmdLineParser.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/CmdLineParser.java @@ -86,6 +86,7 @@ public class CmdLineParser extends org.kohsuke.args4j.CmdLineParser { registerHandler(RefSpec.class, RefSpecHandler.class); registerHandler(RevCommit.class, RevCommitHandler.class); registerHandler(RevTree.class, RevTreeHandler.class); + registerHandler(List.class, OptionWithValuesListHandler.class); } private final Repository db; diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/OptionWithValuesListHandler.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/OptionWithValuesListHandler.java new file mode 100644 index 000000000..3de7a8109 --- /dev/null +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/opt/OptionWithValuesListHandler.java @@ -0,0 +1,52 @@ +package org.eclipse.jgit.pgm.opt; + +import java.util.ArrayList; +import java.util.List; + +import org.eclipse.jgit.pgm.internal.CLIText; +import org.kohsuke.args4j.CmdLineException; +import org.kohsuke.args4j.CmdLineParser; +import org.kohsuke.args4j.OptionDef; +import org.kohsuke.args4j.spi.OptionHandler; +import org.kohsuke.args4j.spi.Parameters; +import org.kohsuke.args4j.spi.Setter; + +/** + * Handler which allows to parse option with few values + * + * @since 4.2 + */ +public class OptionWithValuesListHandler extends OptionHandler> { + + /** + * @param parser + * @param option + * @param setter + */ + public OptionWithValuesListHandler(CmdLineParser parser, + OptionDef option, Setter> setter) { + super(parser, option, setter); + } + + @Override + public int parseArguments(Parameters params) throws CmdLineException { + final List list = new ArrayList<>(); + for (int idx = 0; idx < params.size(); idx++) { + final String p; + try { + p = params.getParameter(idx); + } catch (CmdLineException cle) { + break; + } + list.add(p); + } + setter.addValue(list); + return list.size(); + } + + @Override + public String getDefaultMetaVariable() { + return CLIText.get().metaVar_values; + } + +}