v2 feature: Comma separated slice flags

[zemzale]

Checklist

• Are you running the latest v2 release? The list of releases is here.
• Did you check the manual for your release? The v2 manual is here
• Did you perform a search about this feature? Here's the Github guide about searching.

What problem does this solve?

• The only way how to pass values to slice flags is by calling them multiple times e.g. --foo bar --foo bar2
• By adding comma separation option we could allow users to simplify slice flag population e.g. --foo=bar,bar2
• Related: Add support for comma separated IntSliceFlag and Int64SliceFlag #1118 IntSliceFlag failes at parsing -flag=1,2,3 #1097

Solution description

• Add option CommaSeparated to StringSliceFlag, IntSliceFlag, Int64SliceFlag and Float64SliceFlag that allows comma separation for these flags.
• If this option is enabled we would split these values when Set is called on them.
• It would default to false so it wouldn't break backwards compatibility.

Describe alternatives you've considered

Although this would be the simplest solution IMO as mentioned in #1118

it is absolutely possible to get this behavior for very customized CLIs by implementing the cli.Flag interface manually.

It could be implemented by adding new Flags that add this behavior.

[saschagrunert]

Sounds like a good feature, we could also make the separator configurable.

[vschettino]

Do you guys need help on this one? I think I could open a PR to add this feature. Seems like it would solve #1097 as well, right?

[xordspar0]

@vschettino If you're interested in working on this, you should go ahead and submit a PR! Any help is welcome.

[stale]

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

[noerw]

not stale

[stale]

This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

[coilysiren]

This sounds like a great idea 👍🏼 would love to see a PR for this

[Haransis]

Well if there is no PR opened yet on this, I will submit one.

[Haransis]

Actually, this PR #1241 does exactly that.
It does not use a CommaSeparated flag but it should not break backwards compatibility.

[stale]

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

[stale]

Closing this as it has become stale.

[potuz]

bumping this issue as is a very useful feature, with a PR (albeit unmantained)

[gaocegege]

Actually, this PR #1241 does exactly that. It does not use a CommaSeparated flag but it should not break backwards compatibility.

I am wondering how to disable it without the flag CommaSeparated. I upgraded the package from v1 to v2 and found it broken. Could we provide such a flag for customization?

[dearchap]

v1 to v2 is expected to have breaking changes.

[feedmeapples]

Actually, this PR #1241 does exactly that. It does not use a CommaSeparated flag but it should not break backwards compatibility.

I am wondering how to disable it without the flag CommaSeparated. I upgraded the package from v1 to v2 and found it broken. Could we provide such a flag for customization?

commas are normal especially when passing JSON as a value, ex

tctl workflow start --input '{ \"Concurrency\": 1, \"Count\": 1}'

I would like to send a PR adding CommaSeparated to

• Slice flags. Default false to keep the current behavior working
• App struct as an option to disable comma separation altogether so that the behavior is consistent throughout the app. Default true to keep the current behavior working

Any objections?

[dearchap]

Not a problem at all. Go ahead . Thanks a lot Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Ruslan ***@***.***> Sent: Friday, October 7, 2022 4:37:20 PM To: urfave/cli ***@***.***> Cc: dearchap ***@***.***>; Comment ***@***.***> Subject: Re: [urfave/cli] v2 feature: Comma separated slice flags (#1134) Actually, this PR #1241<#1241> does exactly that. It does not use a CommaSeparated flag but it should not break backwards compatibility. I am wondering how to disable it without the flag CommaSeparated. I upgraded the package from v1 to v2 and found it broken. Could we provide such a flag for customization? commas are normal especially when passing JSON as a value, ex tctl workflow start --input '{ \"Concurrency\": 1, \"Count\": 1}' I would like to send a PR adding CommaSeparated to * Slice flags. Default false to keep the current behavior working * App struct as an option to disable comma separation altogether so that the behavior is consistent throughout the app. Default true to keep the current behavior working Any objections? — Reply to this email directly, view it on GitHub<#1134 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAYNLZWH2QRGCNUQUJJWUPTWCCJZBANCNFSM4MZ4KMUA>. You are receiving this because you commented.Message ID: ***@***.***>

[ina-stoyanova]

I am hitting an issue, where I want to pass in json to a StringSliceFlag, and seeing that urfave actually interprets that as separate flag values by default.

Example:

• I want to be able to pass in

mycli --input 'MAP={ "key1" : "value1", "key2" : { "key3" : "value3" } }"'

• and I want to be able to get back (I'm using printing to stdout here)

MAP={ "key1" : "value1" "key2" : { "key3" : "value3" } }"

• Right now, I get:

MAP={ "key1" : "value1"      <---- this is treated as one value for the flag 
"key2" : { "key3" : "value3" } }"   <---- this is treated as another value for the flag 

Seems like @feedmeapples was hitting the exact same issue here. @feedmeapples did you ever get a chance to open a PR for that?

[dearchap]

@ina-stoyanova PR #1546 has a fix for this. You can set the string slice separator to something else

[ina-stoyanova]

Oh, that's great to see @dearchap! I'll keep an eye out for the latest release in the next few days (I'm pretty sure I can use the branch name until then!)

That's great timing :) Thanks for everyone's effort that has gone into it before I jumped in here 🙏🙏🙏

[feedmeapples]

@ina-stoyanova @dearchap added one more PR #1582 to actually allow disabling separator completely

[ina-stoyanova]

thank you loads @feedmeapples and @dearchap! I'll update to the latest again as soon as I can :)