Skip to content

Commit a955324

Browse files
committed
HelpFormatter now accepts a null comparator to preserve the declaration order of the options (CLI-212)
git-svn-id: https://svn.apache.org/repos/asf/commons/proper/cli/trunk@1091575 13f79535-47bb-0310-9956-ffa450edef68
1 parent 8bc8db0 commit a955324

3 files changed

Lines changed: 26 additions & 29 deletions

File tree

src/main/java/org/apache/commons/cli/HelpFormatter.java

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -341,20 +341,13 @@ public Comparator<Option> getOptionComparator()
341341

342342
/**
343343
* Set the comparator used to sort the options when they output in help text.
344-
* Passing in a null parameter will set the ordering to the default mode.
344+
* Passing in a null comparator will keep the options in the order they were declared.
345345
*
346346
* @since 1.2
347347
*/
348348
public void setOptionComparator(Comparator<Option> comparator)
349349
{
350-
if (comparator == null)
351-
{
352-
this.optionComparator = new OptionComparator();
353-
}
354-
else
355-
{
356-
this.optionComparator = comparator;
357-
}
350+
this.optionComparator = comparator;
358351
}
359352

360353
/**
@@ -546,17 +539,17 @@ public void printUsage(PrintWriter pw, int width, String app, Options options)
546539
// create a list for processed option groups
547540
final Collection<OptionGroup> processedGroups = new ArrayList<OptionGroup>();
548541

549-
// temp variable
550-
Option option;
551-
552542
List<Option> optList = new ArrayList<Option>(options.getOptions());
553-
Collections.sort(optList, getOptionComparator());
543+
if (getOptionComparator() != null)
544+
{
545+
Collections.sort(optList, getOptionComparator());
546+
}
554547
// iterate over the options
555548
for (Iterator i = optList.iterator(); i.hasNext();)
556549
{
557550
// get the next Option
558-
option = (Option) i.next();
559-
551+
Option option = (Option) i.next();
552+
560553
// check if the option is part of an OptionGroup
561554
OptionGroup group = options.getOptionGroup(option);
562555

@@ -611,7 +604,10 @@ private void appendOptionGroup(final StringBuffer buff, final OptionGroup group)
611604
}
612605

613606
List<Option> optList = new ArrayList<Option>(group.getOptions());
614-
Collections.sort(optList, getOptionComparator());
607+
if (getOptionComparator() != null)
608+
{
609+
Collections.sort(optList, getOptionComparator());
610+
}
615611
// for each option in the OptionGroup
616612
for (Iterator i = optList.iterator(); i.hasNext();)
617613
{
@@ -757,16 +753,18 @@ protected StringBuffer renderOptions(StringBuffer sb, int width, Options options
757753
// the longest opt string this list will be then used to
758754
// sort options ascending
759755
int max = 0;
760-
StringBuffer optBuf;
761756
List<StringBuffer> prefixList = new ArrayList<StringBuffer>();
762757

763758
List<Option> optList = options.helpOptions();
764759

765-
Collections.sort(optList, getOptionComparator());
760+
if (getOptionComparator() != null)
761+
{
762+
Collections.sort(optList, getOptionComparator());
763+
}
766764

767765
for (Option option : optList)
768766
{
769-
optBuf = new StringBuffer();
767+
StringBuffer optBuf = new StringBuffer();
770768

771769
if (option.getOpt() == null)
772770
{
@@ -806,7 +804,7 @@ protected StringBuffer renderOptions(StringBuffer sb, int width, Options options
806804
for (Iterator i = optList.iterator(); i.hasNext();)
807805
{
808806
Option option = (Option) i.next();
809-
optBuf = new StringBuffer(prefixList.get(x++).toString());
807+
StringBuffer optBuf = new StringBuffer(prefixList.get(x++).toString());
810808

811809
if (optBuf.length() < max)
812810
{

src/main/java/org/apache/commons/cli/Options.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.HashMap;
2525
import java.util.HashSet;
2626
import java.util.Iterator;
27+
import java.util.LinkedHashMap;
2728
import java.util.List;
2829
import java.util.Map;
2930

@@ -49,10 +50,10 @@ public class Options implements Serializable
4950
private static final long serialVersionUID = 1L;
5051

5152
/** a map of the options with the character key */
52-
private Map<String, Option> shortOpts = new HashMap<String, Option>();
53+
private Map<String, Option> shortOpts = new LinkedHashMap<String, Option>();
5354

5455
/** a map of the options with the long key */
55-
private Map<String, Option> longOpts = new HashMap<String, Option>();
56+
private Map<String, Option> longOpts = new LinkedHashMap<String, Option>();
5657

5758
/** a map of the required options */
5859
private List<Object> requiredOpts = new ArrayList<Object>();

src/test/java/org/apache/commons/cli/HelpFormatterTest.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -269,13 +269,11 @@ public void testPrintSortedUsage()
269269
opts.addOption(new Option("c", "third"));
270270

271271
HelpFormatter helpFormatter = new HelpFormatter();
272-
helpFormatter.setOptionComparator(new Comparator()
272+
helpFormatter.setOptionComparator(new Comparator<Option>()
273273
{
274-
public int compare(Object o1, Object o2)
274+
public int compare(Option opt1, Option opt2)
275275
{
276276
// reverses the fuctionality of the default comparator
277-
Option opt1 = (Option) o1;
278-
Option opt2 = (Option) o2;
279277
return opt2.getKey().compareToIgnoreCase(opt1.getKey());
280278
}
281279
});
@@ -289,17 +287,17 @@ public int compare(Object o1, Object o2)
289287
public void testPrintSortedUsageWithNullComparator()
290288
{
291289
Options opts = new Options();
292-
opts.addOption(new Option("a", "first"));
290+
opts.addOption(new Option("c", "first"));
293291
opts.addOption(new Option("b", "second"));
294-
opts.addOption(new Option("c", "third"));
292+
opts.addOption(new Option("a", "third"));
295293

296294
HelpFormatter helpFormatter = new HelpFormatter();
297295
helpFormatter.setOptionComparator(null);
298296

299297
StringWriter out = new StringWriter();
300298
helpFormatter.printUsage(new PrintWriter(out), 80, "app", opts);
301299

302-
assertEquals("usage: app [-a] [-b] [-c]" + EOL, out.toString());
300+
assertEquals("usage: app [-c] [-b] [-a]" + EOL, out.toString());
303301
}
304302

305303
public void testPrintOptionGroupUsage()

0 commit comments

Comments
 (0)