Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command execute doesn't allow repeated types #681

Closed
antho1404 opened this issue Jan 7, 2019 · 7 comments · Fixed by #684
Closed

Command execute doesn't allow repeated types #681

antho1404 opened this issue Jan 7, 2019 · 7 comments · Fixed by #684
Labels
bug Something isn't working

Comments

@antho1404
Copy link
Member

This PR #679 introduced a new data type: repeated data

This file https://github.com/mesg-foundation/core/blob/master/utils/servicecasting/cast.go doesn't do anything about repeated parameters and so the command service execute cannot handle repeated data with the flag --data

@antho1404 antho1404 added the bug Something isn't working label Jan 7, 2019
@antho1404
Copy link
Member Author

Future types like Any and Object need to be tested as well

@krhubert
Copy link
Contributor

krhubert commented Jan 7, 2019

How do we want to represent the repeated types in cli?

@antho1404
Copy link
Member Author

Some ideas:

  • --data foo=[a, b, c]
  • --data foo.0=a --data foo.1=b --data foo.2=c
  • --data foo=a,b,c
  • Maybe we just support repeated with the --json and not the --data

@NicolasMahe
Copy link
Member

Let's see what is the simplest to implement in xpflag.NewStringToStringValue.

I like --data foo=[a, b, c].

@krhubert
Copy link
Contributor

krhubert commented Jan 8, 2019

the most common way i saw in cli is:
--data foo=a,b,c where , is default separator and could be changed by --data-separator flag

--data foo=[a, b, c] - remember that spaces here won't be allowed (because of parsing cli args) so it would become --data foo=[a,b,c]

--data foo.0=a --data foo.1=b --data foo.2=c this one introduce complexity because someone may pass --data foo.100000=a and you have to fill previous values with zeros or return an error

@ilgooz
Copy link
Contributor

ilgooz commented Jan 8, 2019

  • I think repeated types can be supported with both -d a=1,2 & -d a=1 -d a=2 syntaxes.

  • And objects should be only supported with json option. Because it can be really hard to deal with nested data specially if the data has too many fields. So, in the real world, devs aren't going to use it because of the complexity anyway.

@NicolasMahe
Copy link
Member

let's go with the simplest implementation!

This was referenced Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants