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 <loskutov@gmx.de>

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 74506bc9444368998c8f3a33044bb0ca13b72e06..f1a53d7dcc452e49c30449600406364f83c750b2 100644 (file)
--- 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.api.Git;
 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 class BranchTest extends CLIRepositoryTestCase {
                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 c4c5d64a0ea5a0902a70825a68df7436215aa058..0dc549c7d791227de0b1d3b34bbb534c4c65eee6 100644 (file)
--- 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 @@ public abstract class TextBuiltin {
        }
 
        /**
-        * @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 a1e39502c773630dbe6f26d3f6de625d8c42bc28..b531ba65a4dc451a68525664d80fa50ea377233d 100644 (file)
--- 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 java.util.ResourceBundle;
 
 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 class CmdLineParser extends org.kohsuke.args4j.CmdLineParser {
         */
        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 class CmdLineParser extends org.kohsuke.args4j.CmdLineParser {
 
                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 class CmdLineParser extends org.kohsuke.args4j.CmdLineParser {
                }
        }
 
+       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<OptionHandler> unsetRequiredOptions() {
                List<OptionHandler> options = getOptions();
                List<OptionHandler> backup = new ArrayList<>(options);