Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.3
    • Component/s: CLI-1.x, Options definition
    • Labels:
      None
    • Environment:

      windows jdk1.6.0_21

      Description

      Having some problems setting options as required. Have I misunderstood the documentation?

      • In test1() the option is required but also supplied, I expect to get no exceptions.
      • In test2() the option is required but not supplied, I expect to get an exception.
      public class Testing {
      
          public static void main(String[] args) {
              System.out.println("Begin test 1");
              test1();
              System.out.println("End test 1");
              System.out.println("Begin test 2");
              test2();
              System.out.println("End test 2");
          }
      
          private static void test1() {
      
              String[] args = new String[] { "--foo" };
      
              Options options = new Options();
              options.addOption("f", "foo", false, "");
              options.getRequiredOptions().add(options.getOption("foo"));
      
              GnuParser parser = new GnuParser();
              try {
                  parser.parse(options, args);
              } catch (ParseException e) {
                  // didn't expect to get MissingOptionException here
                  System.out.println("ERROR: " + e.getMessage());
              }
          }
      
          private static void test2() {
      
              String[] args = new String[] { };
      
              Options options = new Options();
              options.addOption("f", "foo", false, "");
              options.getOption("f").setRequired(true);
      
              GnuParser parser = new GnuParser();
              try {
                  parser.parse(options, args);
              } catch (ParseException e) {
                  // expected to get MissingOptionException here
                  System.out.println("ERROR: " + e.getMessage());
              }
          }
      }
      

        Activity

        Hide
        tn Thomas Neidhart added a comment -

        In the current CLI version, it is not advised to alter Option objects after they have been added to an Options instance, as the changes will not be reflected, thus you should do something like that:

        Options options = new Options();
        
        Option o = OptionBuilder.withLongOpt("foo").isRequired().create("f");
        options.addOption(o);
        

        There are several flaws in the current API design of CLI, which will hopefully corrected with CLI2, but we can not do it right now as it would break binary/backwards compatibility.

        Show
        tn Thomas Neidhart added a comment - In the current CLI version, it is not advised to alter Option objects after they have been added to an Options instance, as the changes will not be reflected, thus you should do something like that: Options options = new Options(); Option o = OptionBuilder.withLongOpt("foo").isRequired().create("f"); options.addOption(o); There are several flaws in the current API design of CLI, which will hopefully corrected with CLI2, but we can not do it right now as it would break binary/backwards compatibility.
        Hide
        tn Thomas Neidhart added a comment -

        Actually, Options.getRequiredOptions() should return an unmodifiable list, adding there something really breaks some internal logic.

        Show
        tn Thomas Neidhart added a comment - Actually, Options.getRequiredOptions() should return an unmodifiable list, adding there something really breaks some internal logic.
        Hide
        mizipzor Alexander Fast added a comment -

        Thank you for a swift response. I had indeed missed the part about not being allowed to alter Options after they've been added.

        The code now works. Although my IDE (IntelliJ Idea 12.0.3) gives me the following warning:

        Static member 'org.apache.commons.cli.OptionBuilder.isRequired()' accessed via instance reference.
        Shows references to static methods and fields via class instance rather than a class itself.

        Closing issue.

        Show
        mizipzor Alexander Fast added a comment - Thank you for a swift response. I had indeed missed the part about not being allowed to alter Options after they've been added. The code now works. Although my IDE (IntelliJ Idea 12.0.3) gives me the following warning: Static member 'org.apache.commons.cli.OptionBuilder.isRequired()' accessed via instance reference. Shows references to static methods and fields via class instance rather than a class itself. Closing issue.
        Hide
        mizipzor Alexander Fast added a comment -

        Actually, Options.getRequiredOptions() should return an unmodifiable list, adding there something really breaks some internal logic.

        Agreed, I thought that was strange as well (got warnings about unchecked when adding typed stuff to the list).

        Show
        mizipzor Alexander Fast added a comment - Actually, Options.getRequiredOptions() should return an unmodifiable list, adding there something really breaks some internal logic. Agreed, I thought that was strange as well (got warnings about unchecked when adding typed stuff to the list).
        Hide
        tn Thomas Neidhart added a comment -

        Ok good, it is advised to ask questions first on the user mailinglist (user@commons.apache.org) with the prefix [cli].

        The warnings you get when using the OptionBuilder are due to the fact that all methods there are static. The 1.3 release will provide a different way to build options using a Builder class in the Option class itself.

        The problem with the required options list will need a fix though to prevent such mistakes.

        Show
        tn Thomas Neidhart added a comment - Ok good, it is advised to ask questions first on the user mailinglist (user@commons.apache.org) with the prefix [cli] . The warnings you get when using the OptionBuilder are due to the fact that all methods there are static. The 1.3 release will provide a different way to build options using a Builder class in the Option class itself. The problem with the required options list will need a fix though to prevent such mistakes.
        Hide
        tn Thomas Neidhart added a comment -

        Applied changes in r1444940.

        Also added a clarifying statement in Option javadoc wrt changing the required flag after the option has been added to Options.

        Show
        tn Thomas Neidhart added a comment - Applied changes in r1444940. Also added a clarifying statement in Option javadoc wrt changing the required flag after the option has been added to Options.

          People

          • Assignee:
            Unassigned
            Reporter:
            mizipzor Alexander Fast
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development