diff --git a/org.eclipse.jgit.pgm.test/src/org/eclipse/jgit/pgm/CLIGitCommand.java b/org.eclipse.jgit.pgm.test/src/org/eclipse/jgit/pgm/CLIGitCommand.java index bf15fed81..7d2cbca72 100644 --- a/org.eclipse.jgit.pgm.test/src/org/eclipse/jgit/pgm/CLIGitCommand.java +++ b/org.eclipse.jgit.pgm.test/src/org/eclipse/jgit/pgm/CLIGitCommand.java @@ -50,6 +50,7 @@ import java.util.List; import org.eclipse.jgit.internal.storage.file.FileRepository; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.pgm.TextBuiltin.TerminatedByHelpException; import org.eclipse.jgit.pgm.internal.CLIText; import org.eclipse.jgit.pgm.opt.CmdLineParser; import org.eclipse.jgit.pgm.opt.SubcommandHandler; @@ -120,12 +121,15 @@ public class CLIGitCommand { System.arraycopy(args, 1, argv, 0, args.length - 1); CLIGitCommand bean = new CLIGitCommand(); - final CmdLineParser clp = new CmdLineParser(bean); + final CmdLineParser clp = new TestCmdLineParser(bean); clp.parseArgument(argv); final TextBuiltin cmd = bean.getSubcommand(); ByteArrayOutputStream baos = new ByteArrayOutputStream(); cmd.outs = baos; + ByteArrayOutputStream errs = new ByteArrayOutputStream(); + cmd.errs = errs; + boolean seenHelp = TextBuiltin.containsHelp(argv); if (cmd.requiresRepository()) cmd.init(db, null); else @@ -133,9 +137,22 @@ public class CLIGitCommand { try { cmd.execute(bean.getArguments().toArray( new String[bean.getArguments().size()])); + } catch (TerminatedByHelpException e) { + seenHelp = true; + // this is not a failure, command execution should just not happen } finally { - if (cmd.outw != null) + if (cmd.outw != null) { cmd.outw.flush(); + } + if (cmd.errw != null) { + cmd.errw.flush(); + } + if (seenHelp) { + return errs.toByteArray(); + } else if (errs.size() > 0) { + // forward the errors to the standard err + System.err.print(errs.toString()); + } } return baos.toByteArray(); } @@ -195,4 +212,14 @@ public class CLIGitCommand { return list.toArray(new String[list.size()]); } + static class TestCmdLineParser extends CmdLineParser { + public TestCmdLineParser(Object bean) { + super(bean); + } + + @Override + protected boolean containsHelp(String... args) { + return false; + } + } } diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/AddTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/AddTest.java index 4253080a6..ca3f00f49 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/AddTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/AddTest.java @@ -46,14 +46,10 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import java.lang.Exception; -import java.lang.String; - import org.eclipse.jgit.api.Git; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.lib.CLIRepositoryTestCase; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; public class AddTest extends CLIRepositoryTestCase { @@ -66,14 +62,12 @@ public class AddTest extends CLIRepositoryTestCase { git = new Git(db); } - @Ignore("args4j exit()s on error instead of throwing, JVM goes down") @Test public void testAddNothing() throws Exception { assertEquals("fatal: Argument \"filepattern\" is required", // execute("git add")[0]); } - @Ignore("args4j exit()s for --help, too") @Test public void testAddUsage() throws Exception { execute("git add --help"); diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ArchiveTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ArchiveTest.java index 4222a2dcc..9aca2d823 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ArchiveTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ArchiveTest.java @@ -52,17 +52,15 @@ import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; -import java.io.InputStreamReader; import java.io.IOException; +import java.io.InputStreamReader; import java.io.OutputStream; -import java.lang.Object; -import java.lang.String; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.concurrent.Callable; -import java.util.concurrent.Executors; import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -71,9 +69,7 @@ import org.eclipse.jgit.api.Git; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.lib.CLIRepositoryTestCase; import org.eclipse.jgit.lib.FileMode; -import org.eclipse.jgit.pgm.CLIGitCommand; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; public class ArchiveTest extends CLIRepositoryTestCase { @@ -89,7 +85,6 @@ public class ArchiveTest extends CLIRepositoryTestCase { emptyTree = db.resolve("HEAD^{tree}").abbreviate(12).name(); } - @Ignore("Some versions of java.util.zip refuse to write an empty ZIP") @Test public void testEmptyArchive() throws Exception { byte[] result = CLIGitCommand.rawExecute( diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CommitTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CommitTest.java new file mode 100644 index 000000000..360ee5a6e --- /dev/null +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/CommitTest.java @@ -0,0 +1,113 @@ +/* + * Copyright (C) 2015, Andrey Loskutov + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.eclipse.jgit.pgm; + +import static org.junit.Assert.assertTrue; + +import org.eclipse.jgit.lib.CLIRepositoryTestCase; +import org.junit.Test; + +public class CommitTest extends CLIRepositoryTestCase { + + @Test + public void testCommitPath() throws Exception { + writeTrashFile("a", "a"); + writeTrashFile("b", "a"); + String result = toString(execute("git add a")); + assertTrue("Unexpected output: " + result, result.isEmpty()); + + result = toString(execute("git status -- a")); + assertTrue("Unexpected output: " + result, + result.contains("new file: a")); + + result = toString(execute("git status -- b")); + assertTrue("Unexpected output: " + result, + result.trim().contains("Untracked files:\n b")); + + result = toString(execute("git commit a -m 'added a'")); + assertTrue("Unexpected output: " + result, result.contains("added a")); + + result = toString(execute("git status -- a")); + assertTrue("Unexpected output: " + result, + result.trim().equals("On branch master")); + + result = toString(execute("git status -- b")); + assertTrue("Unexpected output: " + result, + result.trim().contains("Untracked files:\n b")); + } + + @Test + public void testCommitAll() throws Exception { + writeTrashFile("a", "a"); + writeTrashFile("b", "a"); + String result = toString(execute("git add a b")); + assertTrue("Unexpected output: " + result, result.isEmpty()); + + result = toString(execute("git status -- a b")); + assertTrue("Unexpected output: " + result, + result.contains("new file: a")); + assertTrue("Unexpected output: " + result, + result.contains("new file: b")); + + result = toString(execute("git commit -m 'added a b'")); + assertTrue("Unexpected output: " + result, + result.contains("added a b")); + + result = toString(execute("git status -- a b")); + assertTrue("Unexpected output: " + result, + result.trim().equals("On branch master")); + } + + String toString(String[] arr) { + StringBuilder sb = new StringBuilder(); + for (String s : arr) { + if (s != null && !s.isEmpty()) { + sb.append(s); + if (!s.endsWith("\n")) { + sb.append('\n'); + } + } + } + return sb.toString(); + } +} diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DescribeTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DescribeTest.java index 6352a2652..1c8ba0c2f 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DescribeTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DescribeTest.java @@ -43,6 +43,10 @@ package org.eclipse.jgit.pgm; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.CLIRepositoryTestCase; @@ -103,4 +107,22 @@ public class DescribeTest extends CLIRepositoryTestCase { assertArrayEquals(new String[] { "v1.0-0-g6fd41be", "" }, execute("git describe --long HEAD")); } + + @Test + public void testHelpArgumentBeforeUnknown() throws Exception { + String[] output = execute("git describe -h -XYZ"); + String all = Arrays.toString(output); + assertTrue("Unexpected help output: " + all, + all.contains("jgit describe")); + assertFalse("Unexpected help output: " + all, all.contains("fatal")); + } + + @Test + public void testHelpArgumentAfterUnknown() throws Exception { + String[] output = execute("git describe -XYZ -h"); + String all = Arrays.toString(output); + assertTrue("Unexpected help output: " + all, + all.contains("jgit describe")); + assertTrue("Unexpected help output: " + all, all.contains("fatal")); + } } diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/RepoTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/RepoTest.java index 90efae286..85fc1dbb4 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/RepoTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/RepoTest.java @@ -42,10 +42,13 @@ */ package org.eclipse.jgit.pgm; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.io.File; +import java.util.Arrays; + import org.eclipse.jgit.api.Git; import org.eclipse.jgit.junit.JGitTestUtil; import org.eclipse.jgit.lib.CLIRepositoryTestCase; @@ -97,6 +100,27 @@ public class RepoTest extends CLIRepositoryTestCase { resolveRelativeUris(); } + @Test + public void testMissingPath() throws Exception { + assertEquals("fatal: Argument \"path\" is required", + execute("git repo")[0]); + } + + /** + * See bug 484951: "git repo -h" should not print unexpected values + * + * @throws Exception + */ + @Test + public void testZombieHelpArgument() throws Exception { + String[] output = execute("git repo -h"); + String all = Arrays.toString(output); + assertTrue("Unexpected help output: " + all, + all.contains("jgit repo")); + assertFalse("Unexpected help output: " + all, + all.contains("jgit repo VAL")); + } + @Test public void testAddRepoManifest() throws Exception { StringBuilder xmlContent = new StringBuilder(); diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ResetTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ResetTest.java index dae477928..16c5889c4 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ResetTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/ResetTest.java @@ -48,6 +48,7 @@ import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.CLIRepositoryTestCase; import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; public class ResetTest extends CLIRepositoryTestCase { @@ -61,6 +62,20 @@ public class ResetTest extends CLIRepositoryTestCase { git = new Git(db); } + @Test + public void testPathOptionHelp() throws Exception { + String[] result = execute("git reset -h"); + assertTrue("Unexpected argument: " + result[1], + result[1].endsWith("[-- path ... ...]")); + } + + @Test + public void testZombieArgument_Bug484951() throws Exception { + String[] result = execute("git reset -h"); + assertFalse("Unexpected argument: " + result[0], + result[0].contains("[VAL ...]")); + } + @Test public void testResetSelf() throws Exception { RevCommit commit = git.commit().setMessage("initial commit").call(); @@ -91,15 +106,28 @@ public class ResetTest extends CLIRepositoryTestCase { @Test public void testResetPathDoubleDash() throws Exception { - resetPath(true); + resetPath(true, true); } @Test public void testResetPathNoDoubleDash() throws Exception { - resetPath(false); + resetPath(false, true); + } + + @Test + public void testResetPathDoubleDashNoRef() throws Exception { + resetPath(true, false); + } + + @Ignore("Currently we cannote recognize if a name is a commit-ish or a path, " + + "so 'git reset a' will not work if 'a' is not a branch name but a file path") + @Test + public void testResetPathNoDoubleDashNoRef() throws Exception { + resetPath(false, false); } - private void resetPath(boolean useDoubleDash) throws Exception { + private void resetPath(boolean useDoubleDash, boolean supplyCommit) + throws Exception { // create files a and b writeTrashFile("a", "Hello world a"); writeTrashFile("b", "Hello world b"); @@ -115,8 +143,9 @@ public class ResetTest extends CLIRepositoryTestCase { git.add().addFilepattern(".").call(); // reset only file a - String cmd = String.format("git reset %s%s a", commit.getId().name(), - (useDoubleDash) ? " --" : ""); + String cmd = String.format("git reset %s%s a", + supplyCommit ? commit.getId().name() : "", + useDoubleDash ? " --" : ""); assertStringArrayEquals("", execute(cmd)); assertEquals(commit.getId(), git.getRepository().exactRef("HEAD").getObjectId()); diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/StatusTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/StatusTest.java index 854c52d88..368047c60 100644 --- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/StatusTest.java +++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/StatusTest.java @@ -44,6 +44,7 @@ package org.eclipse.jgit.pgm; import static org.eclipse.jgit.lib.Constants.MASTER; import static org.eclipse.jgit.lib.Constants.R_HEADS; +import static org.junit.Assert.assertTrue; import java.io.IOException; @@ -55,6 +56,13 @@ import org.junit.Test; public class StatusTest extends CLIRepositoryTestCase { + @Test + public void testPathOptionHelp() throws Exception { + String[] result = execute("git status -h"); + assertTrue("Unexpected argument: " + result[1], + result[1].endsWith("[-- path ... ...]")); + } + @Test public void testStatusDefault() throws Exception { executeTest("git status", false, true); 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 335336da2..3dcbda3b9 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 @@ -337,6 +337,7 @@ usage_recordChangesToRepository=Record changes to the repository usage_recurseIntoSubtrees=recurse into subtrees usage_renameLimit=limit size of rename matrix usage_reset=Reset current HEAD to the specified state +usage_resetReference=Reset to given reference name usage_resetHard=Resets the index and working tree usage_resetSoft=Resets without touching the index file nor the working tree usage_resetMixed=Resets the index but not the working tree diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java index 9c72b4aad..d04cef0c7 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Main.java @@ -170,7 +170,7 @@ public class Main { } private void execute(final String[] argv) throws Exception { - final CmdLineParser clp = new CmdLineParser(this); + final CmdLineParser clp = new SubcommandLineParser(this); PrintWriter writer = new PrintWriter(System.err); try { clp.parseArgument(argv); @@ -335,4 +335,19 @@ public class Main { } } } + + /** + * Parser for subcommands which doesn't stop parsing on help options and so + * proceeds all specified options + */ + static class SubcommandLineParser extends CmdLineParser { + public SubcommandLineParser(Object bean) { + super(bean); + } + + @Override + protected boolean containsHelp(String... args) { + return false; + } + } } diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Remote.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Remote.java index 70868e920..24916bd1c 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Remote.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Remote.java @@ -144,7 +144,7 @@ class Remote extends TextBuiltin { } @Override - public void printUsageAndExit(final String message, final CmdLineParser clp) + public void printUsage(final String message, final CmdLineParser clp) throws IOException { errw.println(message); errw.println("jgit remote [--verbose (-v)] [--help (-h)]"); //$NON-NLS-1$ @@ -160,7 +160,6 @@ class Remote extends TextBuiltin { errw.println(); errw.flush(); - throw die(true); } private void print(List remotes) throws IOException { diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Repo.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Repo.java index db88008e1..ea59527fe 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Repo.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Repo.java @@ -55,7 +55,7 @@ class Repo extends TextBuiltin { @Option(name = "--groups", aliases = { "-g" }, usage = "usage_groups") private String groups = "default"; //$NON-NLS-1$ - @Argument(required = true, usage = "usage_pathToXml") + @Argument(required = true, metaVar = "metaVar_path", usage = "usage_pathToXml") private String path; @Option(name = "--record-remote-branch", usage = "usage_branches") diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Reset.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Reset.java index 6ba076290..eb222747d 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Reset.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Reset.java @@ -51,7 +51,7 @@ import org.eclipse.jgit.api.ResetCommand; import org.eclipse.jgit.api.ResetCommand.ResetType; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.Option; -import org.kohsuke.args4j.spi.StopOptionHandler; +import org.kohsuke.args4j.spi.RestOfArgumentsHandler; @Command(common = true, usage = "usage_reset") class Reset extends TextBuiltin { @@ -65,12 +65,12 @@ class Reset extends TextBuiltin { @Option(name = "--hard", usage = "usage_resetHard") private boolean hard = false; - @Argument(required = true, index = 0, metaVar = "metaVar_name", usage = "usage_reset") + @Argument(required = false, index = 0, metaVar = "metaVar_commitish", usage = "usage_resetReference") private String commit; - @Argument(index = 1) - @Option(name = "--", metaVar = "metaVar_paths", multiValued = true, handler = StopOptionHandler.class) - private List paths = new ArrayList(); + @Argument(required = false, index = 1, metaVar = "metaVar_paths") + @Option(name = "--", metaVar = "metaVar_paths", multiValued = true, handler = RestOfArgumentsHandler.class) + private List paths = new ArrayList<>(); @Override protected void run() throws Exception { diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Status.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Status.java index be82d070f..6a6322131 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Status.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Status.java @@ -59,8 +59,9 @@ import org.eclipse.jgit.lib.IndexDiff.StageState; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.pgm.internal.CLIText; +import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.Option; - +import org.kohsuke.args4j.spi.RestOfArgumentsHandler; import org.eclipse.jgit.pgm.opt.UntrackedFilesHandler; /** @@ -83,7 +84,8 @@ class Status extends TextBuiltin { @Option(name = "--untracked-files", aliases = { "-u", "-uno", "-uall" }, usage = "usage_untrackedFilesMode", handler = UntrackedFilesHandler.class) protected String untrackedFilesMode = "all"; // default value //$NON-NLS-1$ - @Option(name = "--", metaVar = "metaVar_path", multiValued = true) + @Argument(required = false, index = 0, metaVar = "metaVar_paths") + @Option(name = "--", metaVar = "metaVar_paths", multiValued = true, handler = RestOfArgumentsHandler.class) protected List filterPaths; @Override diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/TextBuiltin.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/TextBuiltin.java index 40a42e520..95938326f 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/TextBuiltin.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/TextBuiltin.java @@ -212,17 +212,20 @@ public abstract class TextBuiltin { */ protected void parseArguments(final String[] args) throws IOException { final CmdLineParser clp = new CmdLineParser(this); + help = containsHelp(args); try { clp.parseArgument(args); } catch (CmdLineException err) { - if (!help) { - this.errw.println(MessageFormat.format(CLIText.get().fatalError, err.getMessage())); - throw die(true, err); + this.errw.println(MessageFormat.format(CLIText.get().fatalError, err.getMessage())); + if (help) { + printUsage("", clp); //$NON-NLS-1$ } + throw die(true, err); } if (help) { - printUsageAndExit(clp); + printUsage("", clp); //$NON-NLS-1$ + throw new TerminatedByHelpException(); } argWalk = clp.getRevWalkGently(); @@ -246,6 +249,20 @@ public abstract class TextBuiltin { * @throws IOException */ public void printUsageAndExit(final String message, final CmdLineParser clp) throws IOException { + printUsage(message, clp); + throw die(true); + } + + /** + * @param message + * non null + * @param clp + * parser used to print options + * @throws IOException + * @since 4.2 + */ + protected void printUsage(final String message, final CmdLineParser clp) + throws IOException { errw.println(message); errw.print("jgit "); //$NON-NLS-1$ errw.print(commandName); @@ -257,7 +274,6 @@ public abstract class TextBuiltin { errw.println(); errw.flush(); - throw die(true); } /** @@ -346,4 +362,36 @@ public abstract class TextBuiltin { dst = dst.substring(R_REMOTES.length()); return dst; } + + /** + * @param args + * non null + * @return true if the given array contains help option + * @since 4.2 + */ + public static boolean containsHelp(String[] args) { + for (String str : args) { + if (str.equals("-h") || str.equals("--help")) { //$NON-NLS-1$ //$NON-NLS-2$ + return true; + } + } + return false; + } + + /** + * Exception thrown by {@link TextBuiltin} if it proceeds 'help' option + * + * @since 4.2 + */ + public static class TerminatedByHelpException extends Die { + private static final long serialVersionUID = 1L; + + /** + * Default constructor + */ + public TerminatedByHelpException() { + super(true); + } + + } } 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 3f77aa668..66d481642 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 @@ -43,17 +43,14 @@ package org.eclipse.jgit.pgm.opt; +import java.io.Writer; import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.ResourceBundle; -import org.kohsuke.args4j.Argument; -import org.kohsuke.args4j.CmdLineException; -import org.kohsuke.args4j.IllegalAnnotationError; -import org.kohsuke.args4j.NamedOptionDef; -import org.kohsuke.args4j.Option; -import org.kohsuke.args4j.OptionDef; -import org.kohsuke.args4j.spi.OptionHandler; -import org.kohsuke.args4j.spi.Setter; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.pgm.TextBuiltin; @@ -63,6 +60,15 @@ import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.RefSpec; import org.eclipse.jgit.treewalk.AbstractTreeIterator; +import org.kohsuke.args4j.Argument; +import org.kohsuke.args4j.CmdLineException; +import org.kohsuke.args4j.IllegalAnnotationError; +import org.kohsuke.args4j.NamedOptionDef; +import org.kohsuke.args4j.Option; +import org.kohsuke.args4j.OptionDef; +import org.kohsuke.args4j.spi.OptionHandler; +import org.kohsuke.args4j.spi.RestOfArgumentsHandler; +import org.kohsuke.args4j.spi.Setter; /** * Extended command line parser which handles --foo=value arguments. @@ -86,6 +92,8 @@ public class CmdLineParser extends org.kohsuke.args4j.CmdLineParser { private RevWalk walk; + private boolean seenHelp; + /** * Creates a new command line owner that parses arguments/options and set * them into the given object. @@ -143,9 +151,58 @@ public class CmdLineParser extends org.kohsuke.args4j.CmdLineParser { } tmp.add(str); + + if (containsHelp(args)) { + // suppress exceptions on required parameters if help is present + seenHelp = true; + // stop argument parsing here + break; + } + } + List backup = null; + if (seenHelp) { + backup = unsetRequiredOptions(); + } + + try { + super.parseArgument(tmp.toArray(new String[tmp.size()])); + } finally { + // reset "required" options to defaults for correct command printout + if (backup != null && !backup.isEmpty()) { + restoreRequiredOptions(backup); + } + seenHelp = false; + } + } + + private List unsetRequiredOptions() { + List options = getOptions(); + List backup = new ArrayList<>(options); + for (Iterator iterator = options.iterator(); iterator + .hasNext();) { + OptionHandler handler = iterator.next(); + if (handler.option instanceof NamedOptionDef + && handler.option.required()) { + iterator.remove(); + } } + return backup; + } - super.parseArgument(tmp.toArray(new String[tmp.size()])); + private void restoreRequiredOptions(List backup) { + List options = getOptions(); + options.clear(); + options.addAll(backup); + } + + /** + * @param args + * non null + * @return true if the given array contains help option + * @since 4.2 + */ + protected boolean containsHelp(final String... args) { + return TextBuiltin.containsHelp(args); } /** @@ -181,7 +238,7 @@ public class CmdLineParser extends org.kohsuke.args4j.CmdLineParser { return walk; } - static class MyOptionDef extends OptionDef { + class MyOptionDef extends OptionDef { public MyOptionDef(OptionDef o) { super(o.usage(), o.metaVar(), o.required(), o.handler(), o @@ -201,6 +258,11 @@ public class CmdLineParser extends org.kohsuke.args4j.CmdLineParser { return metaVar(); } } + + @Override + public boolean required() { + return seenHelp ? false : super.required(); + } } @Override @@ -211,4 +273,55 @@ public class CmdLineParser extends org.kohsuke.args4j.CmdLineParser { return super.createOptionHandler(new MyOptionDef(o), setter); } + + @SuppressWarnings("unchecked") + private List getOptions() { + List options = null; + try { + Field field = org.kohsuke.args4j.CmdLineParser.class + .getDeclaredField("options"); //$NON-NLS-1$ + field.setAccessible(true); + options = (List) field.get(this); + } catch (NoSuchFieldException | SecurityException + | IllegalArgumentException | IllegalAccessException e) { + // ignore + } + if (options == null) { + return Collections.emptyList(); + } + return options; + } + + @Override + public void printSingleLineUsage(Writer w, ResourceBundle rb) { + List options = getOptions(); + if (options.isEmpty()) { + super.printSingleLineUsage(w, rb); + return; + } + List backup = new ArrayList<>(options); + boolean changed = sortRestOfArgumentsHandlerToTheEnd(options); + try { + super.printSingleLineUsage(w, rb); + } finally { + if (changed) { + options.clear(); + options.addAll(backup); + } + } + } + + private boolean sortRestOfArgumentsHandlerToTheEnd( + List options) { + for (int i = 0; i < options.size(); i++) { + OptionHandler handler = options.get(i); + if (handler instanceof RestOfArgumentsHandler + || handler instanceof PathTreeFilterHandler) { + options.remove(i); + options.add(handler); + return true; + } + } + return false; + } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java index 9466dab74..5b7d84f73 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/CommitCommand.java @@ -681,7 +681,7 @@ public class CommitCommand extends GitCommand { */ public CommitCommand setAll(boolean all) { checkCallable(); - if (!only.isEmpty()) + if (all && !only.isEmpty()) throw new JGitInternalException(MessageFormat.format( JGitText.get().illegalCombinationOfArguments, "--all", //$NON-NLS-1$ "--only")); //$NON-NLS-1$