Skip to content

Disable "null..." comment in primitive types #118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Disable "null..." comment in primitive types #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5e4ff5f
- Removed note "This option can be set to 'null' to clear the default…
yfarjoun Dec 10, 2017
584a34b
- oops
yfarjoun Dec 10, 2017
f3d3d34
- newlines and <>
yfarjoun Dec 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/org/broadinstitute/barclay/argparser/LegacyCommandLineArgumentParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ private void printOptionParamUsage(final StringBuilder sb, final String name, fi
}

int numSpaces = OPTION_COLUMN_WIDTH - optionLabel.length();
if (optionLabel.length() > OPTION_COLUMN_WIDTH) {
if (optionLabel.length() > OPTION_COLUMN_WIDTH - 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this only happen in the legacy parser? I guess that for concordance, it should be modified in the CommandLineArgumentParser

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this change in the other parser as well ?

sb.append("\n");
numSpaces = OPTION_COLUMN_WIDTH;
}
Expand Down Expand Up @@ -672,7 +672,7 @@ private String makeOptionDescription(final OptionDefinition optionDefinition) {
sb.append("Default value: ");
sb.append(optionDefinition.defaultValue);
sb.append(". ");
if (!optionDefinition.defaultValue.equals("null")) {
if (!optionDefinition.defaultValue.equals("null") && !optionDefinition.field.getType().isPrimitive() ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line does not appear in the CommandLineArgumentParser implementation. Is it better to remove from here too?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other parser does allow "null" for optional args. There are some tests there for it, but please make the same change to check for isPrimitive and add the same usage test there as well.

sb.append("This option can be set to 'null' to clear the default value. ");
}
} else if (!optionDefinition.isCollection) {
Expand Down
74 changes: 68 additions & 6 deletions src/test/java/org/broadinstitute/barclay/argparser/LegacyCommandLineArgumentParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

import java.io.*;
import java.util.*;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class LegacyCommandLineArgumentParserTest {

Expand All @@ -44,7 +46,7 @@ enum FrobnicationFlavor {
class FrobnicateOptions {

@PositionalArguments(minElements = 2, maxElements = 2)
public List<File> positionalArguments = new ArrayList<File>();
public List<File> positionalArguments = new ArrayList<>();

@Argument(shortName = "T", doc = "Frobnication threshold setting.")
public Integer FROBNICATION_THRESHOLD = 20;
Expand All @@ -53,12 +55,45 @@ class FrobnicateOptions {
public FrobnicationFlavor FROBNICATION_FLAVOR;

@Argument(doc = "Allowed shmiggle types.", minElements = 1, maxElements = 3)
public List<String> SHMIGGLE_TYPE = new ArrayList<String>();
public List<String> SHMIGGLE_TYPE = new ArrayList<>();

@Argument
public Boolean TRUTHINESS;
}

@CommandLineProgramProperties(
summary = "Usage: frobnicate [options] input-file output-file\n\nRead input-file, frobnicate it, " +
"and write frobnicated results to output-file\n",
oneLineSummary = "Read input-file, frobnicate it, and write frobnicated results to output-file",
programGroup = TestProgramGroup.class
)
class FrobnicateOptionsWithPrimitives {

@Argument(doc = "Primitive long")
public long Prim_longArgument = 1L;

@Argument(doc = "Primitive int")
public int Prim_FROBNICATION_THRESHOLD = 20;

@Argument(doc = "Primitive short")
public short Prim_FROBNICATION_FLAVOR = 2;

@Argument(doc = "Primitive boolean")
public boolean Prim_SHMIGGLE_TYPE = false;

@Argument(doc = "Boxed Long")
public Long longArgument = 1L;

@Argument(doc = "Boxed Integer")
public Integer FROBNICATION_THRESHOLD = 20;

@Argument(doc = "Boxed Short")
public Short FROBNICATION_FLAVOR = 2;

@Argument(doc = "Boxed Boolean")
public Boolean SHMIGGLE_TYPE = false;
}

@CommandLineProgramProperties(
summary = "Usage: frobnicate [options] input-file output-file\n\nRead input-file, frobnicate it, and write frobnicated results to output-file\n",
oneLineSummary = "Read input-file, frobnicate it, and write frobnicated results to output-file",
Expand All @@ -67,7 +102,7 @@ class FrobnicateOptions {
class FrobnicateOptionsWithNullList {

@PositionalArguments(minElements = 2, maxElements = 2)
public List<File> positionalArguments = new ArrayList<File>();
public List<File> positionalArguments = new ArrayList<>();

@Argument(shortName = "T", doc = "Frobnication threshold setting.")
public Integer FROBNICATION_THRESHOLD = 20;
Expand All @@ -76,7 +111,7 @@ class FrobnicateOptionsWithNullList {
public FrobnicationFlavor FROBNICATION_FLAVOR;

@Argument(doc = "Allowed shmiggle types.", minElements = 0, maxElements = 3)
public List<String> SHMIGGLE_TYPE = new ArrayList<String>();
public List<String> SHMIGGLE_TYPE = new ArrayList<>();

@Argument
public Boolean TRUTHINESS;
Expand All @@ -96,7 +131,7 @@ class OptionsWithoutPositional {
public FrobnicationFlavor FROBNICATION_FLAVOR;

@Argument(doc = "Allowed shmiggle types.", minElements = 1, maxElements = 3)
public List<String> SHMIGGLE_TYPE = new ArrayList<String>();
public List<String> SHMIGGLE_TYPE = new ArrayList<>();

@Argument
public Boolean TRUTHINESS;
Expand Down Expand Up @@ -129,7 +164,6 @@ class MutexOptions {
public String Y;
@Argument(mutex = {"A", "B", "M", "N"})
public String Z;

}

@Test
Expand All @@ -139,6 +173,34 @@ public void testUsage() {
clp.usage(false, true);
}

@Test
public void testUsageWithPrimitives() {
final FrobnicateOptionsWithPrimitives fo = new FrobnicateOptionsWithPrimitives();
final LegacyCommandLineArgumentParser clp = new LegacyCommandLineArgumentParser(fo);
final String usage = clp.usage(false, true);

final Pattern primitiveVars = Pattern.compile(" +Primitive .*");
final Pattern boxedVars = Pattern.compile(" +Boxed .*");
final Pattern nulls = Pattern.compile("null");

final Matcher primMatcher = primitiveVars.matcher(usage);
int countPrimitives = 0;
while (primMatcher.find()) {
Assert.assertFalse(nulls.matcher(primMatcher.toMatchResult().group()).find());
countPrimitives++;
}
Assert.assertEquals(countPrimitives, 4);

final Matcher boxedMatcher = boxedVars
.matcher(usage);
int countBoxed = 0;
while (boxedMatcher.find()) {
Assert.assertTrue(nulls.matcher(boxedMatcher.toMatchResult().group()).find());
countBoxed++;
}
Assert.assertEquals(countBoxed, 4);
}

@Test
public void testUsageWithDefault() {
final FrobnicateOptions fo = new FrobnicateOptions();
Expand Down