Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.2
    • Fix Version/s: 1.3
    • Component/s: Help formatter
    • Labels:
      None

      Description

      The HelpFormatter always renders a space between the option name and value even if the option has a value separator that is not a space. For example, this option:

      Option option = new Option("B", "bsize", true, "block size in bytes");
      option.setArgName("SIZE");
      option.setValueSeparator('=');
      

      is rendered like this:

      -B,--bsize <SIZE>  block size in bytes
      

      But I would expect it to be rendered like this:

      -B,--bsize=<SIZE>  block size in bytes
      

      For the automatically generated usage message, a similar problem exists. I changed it to try to use the value separator when rendering the option with the short name only if there's no long name. If there's no short name, it always tries to use the value separator with the long name rendering.

      For example, consider this option containing a short name and a specified value separator:

      Option option = new Option("B", true, "block size in bytes");
      option.setArgName("SIZE");
      option.setValueSeparator('=');
      

      The automatically generated usage message with my changes would look like this:

      usage: app [-B=<SIZE>]
      

      If the same option included a long name too:

      Option option = new Option("B", "bsize", true, "block size in bytes");
      option.setArgName("SIZE");
      option.setValueSeparator('=');
      

      with my changes the value separator is assumed to be meant for use with the long name. The short option rendering in the usage message will not try to use the value separator and will render the usage message like this:

      usage: app [-B <SIZE>]
      

      Attached is a patch for these fixes with test cases. The patch is against http://svn.apache.org/repos/asf/commons/proper/cli/branches/cli-1.x.

      1. use-equal-sign-with-long-opts.patch
        10 kB
        J. Lewis Muir
      2. long-opt-name-arg-separator.patch
        10 kB
        J. Lewis Muir
      3. use-value-separator-in-help-formatter.patch
        10 kB
        J. Lewis Muir

        Activity

        J. Lewis Muir created issue -
        J. Lewis Muir made changes -
        Field Original Value New Value
        Attachment use-value-separator-in-help-formatter.patch [ 12391081 ]
        Hide
        Emmanuel Bourg added a comment -

        Actually the value separator is not the character between the option and its value, it's a character that can be used to split a token into several values.

        For example:

        --classpath foo.jar:bar.jar:cli.jar

        If the 'classpath' option is defined with ':' as value separator, it'll receive 3 values: foo.jar, bar.jar and cli.jar.

        The javadoc isn't clear on the purpose of the value separator, I'll try to improve it.

        Show
        Emmanuel Bourg added a comment - Actually the value separator is not the character between the option and its value, it's a character that can be used to split a token into several values. For example: --classpath foo.jar:bar.jar:cli.jar If the 'classpath' option is defined with ':' as value separator, it'll receive 3 values: foo.jar, bar.jar and cli.jar. The javadoc isn't clear on the purpose of the value separator, I'll try to improve it.
        Emmanuel Bourg made changes -
        Component/s CLI-1.x [ 12311672 ]
        Hide
        J. Lewis Muir added a comment -

        Hmm...that's a bummer.

        The ls example at the bottom of http://commons.apache.org/cli/usage.html suggested to me the meaning I had come to. Look at the --block-size=SIZE help output and then the creation of the corresponding option below with the call to withValueSeparator('='). If the meaning of the value separator is what you say, then this example seems wrong. Now that I'm looking more closely at this example, it doesn't set the argument name to "SIZE" either.

        Sorry for the bogus patch.

        Show
        J. Lewis Muir added a comment - Hmm...that's a bummer. The ls example at the bottom of http://commons.apache.org/cli/usage.html suggested to me the meaning I had come to. Look at the --block-size=SIZE help output and then the creation of the corresponding option below with the call to withValueSeparator('=') . If the meaning of the value separator is what you say, then this example seems wrong. Now that I'm looking more closely at this example, it doesn't set the argument name to "SIZE" either. Sorry for the bogus patch.
        Hide
        Emmanuel Bourg added a comment -

        You are right, this example will be changed with the next release.

        Show
        Emmanuel Bourg added a comment - You are right, this example will be changed with the next release.
        Hide
        J. Lewis Muir added a comment -

        OK, I'm trying again.

        I've added a longOptNameArgSeparator property to HelpFormatter to allow the long option name-argument separator to be specified. The default is still the space character to maintain backward compatibility.

        Now, if you have an option like:

        Option option = new Option("B", "bsize", true, "block size in bytes");
        option.setArgName("SIZE");
        

        and you call setLongOptNameArgSeparator('=') on a HelpFormatter instance, the option would be rendered by that instance in the help message as:

        -B,--bsize=<SIZE>
        

        Attached is a patch with test cases.

        Since this is now really a feature addition, I would understand if it can't go into the next release. But I think it would be great if it could.

        Show
        J. Lewis Muir added a comment - OK, I'm trying again. I've added a longOptNameArgSeparator property to HelpFormatter to allow the long option name-argument separator to be specified. The default is still the space character to maintain backward compatibility. Now, if you have an option like: Option option = new Option( "B" , "bsize" , true , "block size in bytes" ); option.setArgName( "SIZE" ); and you call setLongOptNameArgSeparator('=') on a HelpFormatter instance, the option would be rendered by that instance in the help message as: -B,--bsize=<SIZE> Attached is a patch with test cases. Since this is now really a feature addition, I would understand if it can't go into the next release. But I think it would be great if it could.
        J. Lewis Muir made changes -
        Attachment long-opt-name-arg-separator.patch [ 12391130 ]
        Hide
        Emmanuel Bourg added a comment -

        This implies to change the parsers too. One might expect any character to be used as the option/value separator.

        I would rather add a parameter to the HelpFormatter to instruct him to put a '=' between the option name and the argument name for long options. Something like 'setUseEqualSignWithLongOptions(true)'. Would that do what you want? Or do you need a finer control on the formatting of each option?

        Show
        Emmanuel Bourg added a comment - This implies to change the parsers too. One might expect any character to be used as the option/value separator. I would rather add a parameter to the HelpFormatter to instruct him to put a '=' between the option name and the argument name for long options. Something like 'setUseEqualSignWithLongOptions(true)'. Would that do what you want? Or do you need a finer control on the formatting of each option?
        Hide
        J. Lewis Muir added a comment -

        I'd be fine with something like the setUseEqualSignWithLongOptions(true) you suggest.

        I chose to parameterize the separator to make it a little more flexible. My idea was to keep the parsers and the formatter decoupled. If some new parser comes along that accepts options like --bsize+<SIZE>, then HelpFormatter would be able to handle that since the option long name-argument separator could be set to '+'.

        If there were to be more coupling between the parsers and the formatter, I could see a longOptNameArgSeparator property getting added to Option and making the parsers use that. Then HelpFormatter could just always use the longOptNameArgSeparator of each option it rendered (like what I initially did with the value separator).

        Anyway, I'd be happy with setUseEqualSignWithLongOptions(true) you suggest.

        Show
        J. Lewis Muir added a comment - I'd be fine with something like the setUseEqualSignWithLongOptions(true) you suggest. I chose to parameterize the separator to make it a little more flexible. My idea was to keep the parsers and the formatter decoupled. If some new parser comes along that accepts options like --bsize+<SIZE> , then HelpFormatter would be able to handle that since the option long name-argument separator could be set to '+' . If there were to be more coupling between the parsers and the formatter, I could see a longOptNameArgSeparator property getting added to Option and making the parsers use that. Then HelpFormatter could just always use the longOptNameArgSeparator of each option it rendered (like what I initially did with the value separator). Anyway, I'd be happy with setUseEqualSignWithLongOptions(true) you suggest.
        Hide
        J. Lewis Muir added a comment -

        Should I provide a modified patch for the setUseEqualSignWithLongOptions() approach, or are you planning to do that?

        Show
        J. Lewis Muir added a comment - Should I provide a modified patch for the setUseEqualSignWithLongOptions() approach, or are you planning to do that?
        Hide
        J. Lewis Muir added a comment - - edited

        The patch named use-equal-sign-with-long-opts.patch provides the proposed setUseEqualSignWithLongOptions(boolean) behavior.

        Show
        J. Lewis Muir added a comment - - edited The patch named use-equal-sign-with-long-opts.patch provides the proposed setUseEqualSignWithLongOptions(boolean) behavior.
        J. Lewis Muir made changes -
        Attachment use-equal-sign-with-long-opts.patch [ 12391882 ]
        Emmanuel Bourg made changes -
        Fix Version/s 1.3 [ 12313321 ]
        Fix Version/s 1.2 [ 12312674 ]
        Summary HelpFormatter ignores Option's value separator Formatting option for long opts
        Issue Type Bug [ 1 ] Improvement [ 4 ]
        Priority Major [ 3 ] Minor [ 4 ]
        Hide
        Emmanuel Bourg added a comment -

        My suggestion to use a boolean flag is too restrictive. An extra parameter on the formatter specifying the separator is more flexible. No validation will be performed, it'll be up to the user to pick a separator supported by the parser.

        Show
        Emmanuel Bourg added a comment - My suggestion to use a boolean flag is too restrictive. An extra parameter on the formatter specifying the separator is more flexible. No validation will be performed, it'll be up to the user to pick a separator supported by the parser.
        Hide
        Emmanuel Bourg added a comment -

        Fix committed. The separator can now be changed by calling setLongOptSeparator() on the formatter.

        Show
        Emmanuel Bourg added a comment - Fix committed. The separator can now be changed by calling setLongOptSeparator() on the formatter.
        Emmanuel Bourg made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Mark Thomas made changes -
        Workflow jira [ 12442997 ] Default workflow, editable Closed status [ 12601371 ]

          People

          • Assignee:
            Unassigned
            Reporter:
            J. Lewis Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 0.5h
              0.5h
              Remaining:
              Remaining Estimate - 0.5h
              0.5h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development