CommandLineUtils icon indicating copy to clipboard operation
CommandLineUtils copied to clipboard

multiple values (#Question)

Open lobster2012-user opened this issue 6 years ago • 9 comments

Hi! Is your feature request related to a problem? Please describe. Found that multiple values ​​need to be set like this --key value1 --key value2 Expected --key value1 value2 --key value1 , value2 --key value1 + value2 --key "value1,value2"

Why is this the only way?

lobster2012-user avatar May 01 '19 18:05 lobster2012-user

It was a deliberate choice because it is a simple, sensible default. Bearing in mind that the parser gets an array of strings, consider some of these ambiguities:

{ "--key", "value1", "value2" }  
 // is 'value2' supposed to be a subcommand, a positional parameter, or 
 // another value for --key? If your app only accepts one option, no positional
 // parameters, and no subcommands, then we could infer that "value2" is meant
 // to be a new value of `--key`. This isn't implemented, however, because this
 // is an edge case that's rare. Most command line tools accept multiple options or parameters.

{ "--key", "value1", ",", "value2" } 
{ "--key", "value1", "+", "value2" }
 // You could argue ',' or '+' is a special character which should be parsed to mean
 // "join the array item before and after"...but what if you wanted to pass 
 // a comma as input? For instance, what if I want to write a tool with valid usage like this?
 //    localize.exe --decimal-separator , --date dd/mm/yyyy


{ "--key", "value1,value2" }
 // You could argue that the library should always call 
 // `string.Split(",")`, but then how would you write a tool with valid usage like this? 
 //   write.exe --author "McMaster, Nate"

That said, if there are clear ways we can improve the parser to support some of these patterns, I'm open to suggestions.

natemcmaster avatar May 02 '19 06:05 natemcmaster

Honestly, only the first option is close to me. ( '+' - I took this from windows / cmd / copy for example) Yes, there is an ambiguous situation in the theory, (--key value1 value2) but in practice I could set a global parameter or a parameter of this option to process the several values.

but what if you wanted to pass // a comma as input

"aaa", "\," "bbb"

lobster2012-user avatar May 02 '19 06:05 lobster2012-user

Honestly, only the first option is close to me.

If you think there is enough value here, I'd take a PR as long as the syntax remains unambiguous.

"aaa", "," "bbb"

This isn't something I plan to add. I don't think requiring escaping of commas is an improvement to the parser.

natemcmaster avatar May 03 '19 05:05 natemcmaster

This isn't something I plan to add. I don't think requiring escaping of commas is an improvement to the parser.

This was just an example. Personally, I do not like points 2.3, etc.

If you think there is enough value here, I'd take a PR as long as the syntax remains unambiguous.

One of the easy solutions I see is to use PeekableEnumerator Then the minimum number of changes is required.

  ProcessOption()
  {
...
/* Global or local for this option. Did not come up with the name of the setting*/
  if(СanHaveMultipleConsecutiveValues(option)) 
  {
              do {
          if (!_enumerator.MoveNext())
                {
                    _currentCommand.ShowHint();
                    throw new CommandParsingException(_currentCommand, $"Missing value for option                 '{name}'");
                }

                var nextArg = _enumerator.Current;
                if (!option.TryParse(nextArg.Raw))
                {
                    _currentCommand.ShowHint();
                    throw new CommandParsingException(_currentCommand,
                        $"Unexpected value '{nextArg.Raw}' for option '{name}'");
                }
       } while ( enumerator.TryPeek(out var arg) && arg.IsArgumentOrValue)
  }
... 

  }

Or what is your solution?

lobster2012-user avatar May 03 '19 07:05 lobster2012-user

Something like PeekableEnumerator seems like a promising idea. The pseudo-implementation you suggested would need to be fleshed out and tested quite a bit more, but the general premise (i.e. using multiple args to categorize each arg) is a compelling idea. If you think of the args as a sort of programming language, there is opportunity here to use a more powerful parsing algorithm. The current parser is a simple left-to-right parser, and misses cases which a look-ahead parser might be able to handle better.

By the way, here is one scenario where I think there is a clear, simple solution. For inputs like this,

{ "--key", "value1,value2" }

I would be happy to add an API that allowed you to specify a 'split' string. Something like

[Option(SplitOn = ",")]
public string[] Key { get; }

natemcmaster avatar May 16 '19 13:05 natemcmaster

I already implemented peekableEnumerabler. Later I will pull it and tests.

What about SplitOn. I would suggest to have a global default configuration setting and local.

In fact, I like only the version with spaces. There is ambiguous behavior in the presence of 2 commands in a row, global and local settings can be circumvented.

lobster2012-user avatar May 16 '19 14:05 lobster2012-user

PeekableEnumerable to make a better individual repository or add it here?

lobster2012-user avatar May 16 '19 17:05 lobster2012-user

Putting this in the 3.0 milestone. I think there is value in providing functionality to help with this problem.

natemcmaster avatar Jul 16 '19 18:07 natemcmaster

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please comment if you believe this should remain open, otherwise it will be closed in 14 days. Thank you for your contributions to this project.

stale[bot] avatar Jul 21 '21 02:07 stale[bot]

This issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please comment if you believe this should remain open, otherwise it will be closed in 14 days. Thank you for your contributions to this project.

github-actions[bot] avatar Nov 15 '22 02:11 github-actions[bot]

Closing due to inactivity. If you are looking at this issue in the future and think it should be reopened, please make a commented here and mention natemcmaster so he sees the notification.

github-actions[bot] avatar Nov 30 '22 02:11 github-actions[bot]