YunoHost-Apps / Experimental_helpers

6 stars 12 forks source link

Create ynh_handle_getopts_args #21

Closed maniackcrudelis closed 6 years ago

maniackcrudelis commented 6 years ago

Instead of merging this new helper directly, I prefer to create a pull request because I want to get your reviews about this helper.

I think that maybe this helper can be complicated, so I would like to know if you understand how it works and especially how to use it.

Just to explain the idea, this helper is design to be used by other helpers to manage their arguments and get rid of positional arguments by using '"getopts" arguments (-l or --long_option)

alexAubin commented 6 years ago

Not sure what's happening, but if I do :

function my_helper()
{
    declare -Ar args_array=( [p]=pikachu= )
    local pikachu
    ynh_handle_getopts_args "$@"
    echo "pikachu=$pikachu"
}

my_helper --pikachu "pika"

then I get

line 95: =1 : commande introuvable
pikachu=

i.e. it doesn't get "pika" as being the value for pikachu. Then it tries to parse "pika" as an option, but it aint one. It works though if I use my_helper --pikachu="pika" but ideally we shoudln't have to use this = ? (I don't know)

maniackcrudelis commented 6 years ago

Huh, the correct usage would be --pikachu="pika", because it's how we're suppose to use long options, but I thought it wouln't work. So I'm a bit confused...

After my diner, I'm going to try that.

alexAubin commented 6 years ago

Fun fact: if I run my_helper --pikachu "pika" "chu", then I still get the error, but then pikachu=chu :scream_cat:

maniackcrudelis commented 6 years ago

Would be better now @alexAubin. There was many errors...

But still, OPTARG doesn't work as expected, and I don't know why. It's not the most important thing though.

maniackcrudelis commented 6 years ago

By the way, --pikachu="pika" should still not works but --pikachu "pika" should. But it's more a question of design, how do we want to handle that ?

JimboJoe commented 6 years ago

That looks like a nice piece of work! I'm not familiar at all with getopts, what do you think is preventing from using a "standard" syntax (--option="value")? Is it getopts?

maniackcrudelis commented 6 years ago

The thing is that getopts doesn't know how to handle long options like --option. It knows only how to manage -o value. As I said, it's only a matter of design, currently, we have to use --option value. But we can also use --option=value instead. Maybe even both. Anyway, long options aren't handle by getopts but by this line arguments[arg]="${arguments[arg]//--${args_array[$key]%=}/-${key}}" which changes long options into short ones.

...and right know, reading this line, I remember that the current correct syntax is supposed to be --option=value, but I think it would rather be --option= value because otherwise the result would be -ovalue. Well, I think the better thing would be to be able to handle both --option=value and --option value.

maniackcrudelis commented 6 years ago

So, it's supposed to be finished now :)

I implemented the modifications we have discussed during the meeting Tuesday evening.

maniackcrudelis commented 6 years ago

Let's merge and try it now