Skip to contentSkip to conversation

Support automatic command line parsing for TextBuiltin subclasses

PATCH Florian Köberle 2 participants 2 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

Hi Shawn

to use args4j was really a great idea. I had a patch, implementing something similar to args4j in my repository, but using a library is always better. *thumb up*

I had a short look at the files and noticed that the inner "if (help) {" is unnecessary:

+ if (help) {
+ System.err.print("jgit ");
+ System.err.print(commandName);
+ clp.printSingleLineUsage(System.err);
+ System.err.println();
+
+ if (help) {
+ System.err.println();
+ clp.printUsage(System.err);
+ System.err.println();
+ }
+ System.exit(1);
+ }

Also a cool function which you may want to use is:

public <U> Class<? extends U> asSubclass(Class<U> clazz)

With that method of Class<?> you can do the cast before you actually create the object.
You could for example make use of them in SubcommandHandler:
l. 124 cmd = (TextBuiltin) cons.newInstance();

Best regards,
Florian Köberle

#1 lore raw

Review

Quoted 10 lines · Florian KKKberle

I had a short look at the files and noticed that the inner "if (help) {" is unnecessary:

(quoted diff) +7 -0
+		if (help) {
+			System.err.print("jgit ");
+			System.err.print(commandName);
+			clp.printSingleLineUsage(System.err);
+			System.err.println();
+
+			if (help) {

Gaaah. Good catch, thanks. I copied and pasted that block from the global option parser to the per-command parser, and then refactored it a little and missed removing this unnecessary inner if test. I'll fix with a rebase.

Signature · 1 line
Shawn.