From fb5056c2c5e96b815abe568af588157083c66197 Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Sun, 3 Jan 2016 16:15:34 +0100 Subject: [PATCH] branch command: print help if requested, even if arguments are wrong git branch -d -h reports an error (because of missing -d option value) but does not print the help as expected. To fix this, CmdLineParser must catch, print but do not propagate exceptions if help is requested. Bug: 484951 Change-Id: I51265ebe295f22da540792c6a1980b8bdb295a02 Signed-off-by: Andrey Loskutov --- .../tst/org/eclipse/jgit/pgm/BranchTest.java | 10 +++++++ .../src/org/eclipse/jgit/pgm/TextBuiltin.java | 12 ++++++-- .../eclipse/jgit/pgm/opt/CmdLineParser.java | 29 +++++++++++++++++-- 3 files changed, 47 insertions(+), 4 deletions(-) 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 74506bc94..f1a53d7dc 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 @@ -53,6 +53,7 @@ import org.eclipse.jgit.lib.CLIRepositoryTestCase; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.pgm.internal.CLIText; import org.eclipse.jgit.revwalk.RevCommit; import org.junit.Before; import org.junit.Test; @@ -65,6 +66,15 @@ public void setUp() throws Exception { new Git(db).commit().setMessage("initial commit").call(); } + @Test + public void testHelpAfterDelete() throws Exception { + String err = toString(executeUnchecked("git branch -d")); + String help = toString(executeUnchecked("git branch -h")); + String errAndHelp = toString(executeUnchecked("git branch -d -h")); + assertEquals(CLIText.fatalError(CLIText.get().branchNameRequired), err); + assertEquals(toString(err, help), errAndHelp); + } + @Test public void testList() throws Exception { assertEquals("* master", toString(execute("git branch"))); 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 c4c5d64a0..0dc549c7d 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 @@ -277,8 +277,16 @@ protected void printUsage(final String message, final CmdLineParser clp) } /** - * @return the resource bundle that will be passed to args4j for purpose - * of string localization + * @return error writer, typically this is standard error. + * @since 4.2 + */ + public ThrowingPrintWriter getErrorWriter() { + return errw; + } + + /** + * @return the resource bundle that will be passed to args4j for purpose of + * string localization */ protected ResourceBundle getResourceBundle() { return CLIText.get().resourceBundle(); 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 a1e39502c..b531ba65a 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,6 +43,7 @@ package org.eclipse.jgit.pgm.opt; +import java.io.IOException; import java.io.Writer; import java.lang.reflect.Field; import java.util.ArrayList; @@ -53,6 +54,7 @@ import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.pgm.Die; import org.eclipse.jgit.pgm.TextBuiltin; import org.eclipse.jgit.pgm.internal.CLIText; import org.eclipse.jgit.revwalk.RevCommit; @@ -95,6 +97,8 @@ public class CmdLineParser extends org.kohsuke.args4j.CmdLineParser { private boolean seenHelp; + private TextBuiltin cmd; + /** * Creates a new command line owner that parses arguments/options and set * them into the given object. @@ -126,8 +130,12 @@ public CmdLineParser(final Object bean) { */ public CmdLineParser(final Object bean, Repository repo) { super(bean); - if (repo == null && bean instanceof TextBuiltin) - repo = ((TextBuiltin) bean).getRepository(); + if (bean instanceof TextBuiltin) { + cmd = (TextBuiltin) bean; + } + if (repo == null && cmd != null) { + repo = cmd.getRepository(); + } this.db = repo; } @@ -167,6 +175,11 @@ public void parseArgument(final String... args) throws CmdLineException { try { super.parseArgument(tmp.toArray(new String[tmp.size()])); + } catch (Die e) { + if (!seenHelp) { + throw e; + } + printToErrorWriter(CLIText.fatalError(e.getMessage())); } finally { // reset "required" options to defaults for correct command printout if (backup != null && !backup.isEmpty()) { @@ -176,6 +189,18 @@ public void parseArgument(final String... args) throws CmdLineException { } } + private void printToErrorWriter(String error) { + if (cmd == null) { + System.err.println(error); + } else { + try { + cmd.getErrorWriter().println(error); + } catch (IOException e1) { + System.err.println(error); + } + } + } + private List unsetRequiredOptions() { List options = getOptions(); List backup = new ArrayList<>(options);