cli icon indicating copy to clipboard operation
cli copied to clipboard

v2 feature: Comma separated slice flags

Open zemzale opened this issue 4 years ago • 14 comments

Checklist

  • [x] Are you running the latest v2 release? The list of releases is here.
  • [x] Did you check the manual for your release? The v2 manual is here
  • [x] 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: #1118 #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.

zemzale avatar May 05 '20 20:05 zemzale

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

saschagrunert avatar May 06 '20 06:05 saschagrunert

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?

vschettino avatar Jul 13 '20 14:07 vschettino

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

xordspar0 avatar Oct 01 '20 15:10 xordspar0

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[bot] avatar Dec 31 '20 00:12 stale[bot]

not stale

noerw avatar Jan 14 '21 10:01 noerw

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.

stale[bot] avatar Jan 14 '21 10:01 stale[bot]

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

coilysiren avatar Jan 15 '21 20:01 coilysiren

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

Haransis avatar Apr 14 '21 11:04 Haransis

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

Haransis avatar Apr 14 '21 12:04 Haransis

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[bot] avatar Jul 13 '21 17:07 stale[bot]

Closing this as it has become stale.

stale[bot] avatar Aug 18 '21 01:08 stale[bot]

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

potuz avatar Nov 10 '21 01:11 potuz

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?

gaocegege avatar Jul 21 '22 12:07 gaocegege

v1 to v2 is expected to have breaking changes.

dearchap avatar Aug 01 '22 00:08 dearchap

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?

feedmeapples avatar Oct 07 '22 21:10 feedmeapples

Not a problem at all. Go ahead . Thanks a lot

Get Outlook for iOShttps://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 #1241https://github.com/urfave/cli/pull/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 GitHubhttps://github.com/urfave/cli/issues/1134#issuecomment-1272105669, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAYNLZWH2QRGCNUQUJJWUPTWCCJZBANCNFSM4MZ4KMUA. You are receiving this because you commented.Message ID: @.***>

dearchap avatar Oct 08 '22 08:10 dearchap

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?

ina-stoyanova avatar Oct 27 '22 12:10 ina-stoyanova

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

dearchap avatar Oct 27 '22 12:10 dearchap

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 🙏🙏🙏

ina-stoyanova avatar Oct 27 '22 13:10 ina-stoyanova

@ina-stoyanova @dearchap added one more PR https://github.com/urfave/cli/pull/1582 to actually allow disabling separator completely

feedmeapples avatar Nov 11 '22 02:11 feedmeapples

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

ina-stoyanova avatar Dec 05 '22 13:12 ina-stoyanova