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

[FR] return X instead of exit X #43

Open
rockandska opened this issue Feb 12, 2020 · 8 comments
Open

[FR] return X instead of exit X #43

rockandska opened this issue Feb 12, 2020 · 8 comments
Labels
enhancement waiting for more example More input is required to clarify the problem. Please add more example to reproduce the issue.

Comments

@rockandska
Copy link

Hi

Note that due to the above, docopts can't be used to parse shell function arguments: exit(1) quits the entire interpreter, not just the current function.

Would be really nice to add an option to use "return" instead of "exit" to be able to use docopts inside function too.

Regards,

@Sylvain303
Copy link
Collaborator

Hi @rockandska,

Tanks for you interest in docopts. Yes, it could be a nice feature to have. I don't remember yet the limit it imposes to have return instead of exit. The main obstacle I see, is the way we expect to call docopts.

By design docopts is a text processor, due to the limited way it can interact with the parent shell calling it. Of course, you can call it directly with all parameters without eval the output, and you will have some bash code outputted.

I don't really have spare time right now, but if you are bash coder enough, I guess you can wrap docopts call and replace the output to suite your need. Let's try it.

Could you drop a function call example here, as you would expect to call it?
So we could see how we could alter the output of docopts.

May be starting by some thing like:

#!/usr/bin/env bash

docopts_wrap_function() {
  local usage="$1"
  shift
  eval "$(./docopts -h "$usage" : "$@" | sed -e 's/^exit/return/')"
}

hello() {
  if docopts_wrap_function "Usage: hello WORLD [NAME]"  "$@" ; then
    echo "hello $WORLD"
    if [[ -n $NAME ]] ; then                                                    
      echo "and welcome $NAME"
    fi
  else
    echo "hello(): wrong argument"
  fi
}

hello "$@"

If it's so simple than just changing a single word, I could add a -f, --function switch to docopts and the wrapper will disappear.

What do you think about that?

@rockandska
Copy link
Author

Hi

I don't really have spare time right now, but if you are bash coder enough, I guess you can wrap docopts call and replace the output to suite your need. Let's try it.

Already had the idea but is only a dirty quick fix ^^ and doesn't cover the declaration of variables as "local" without adding a supplementary parser to the wrapper to add "local" in front of the variables declarations.

If it's so simple than just changing a single word, I could add a -f, --function switch to docopts and the wrapper will disappear.

it will be more clean and could add "local" to the variables declaration too when intend to be used in function.

PS: not related but i don't get why using : as a separator for usage args and not the conventional --

./docopts -h "$usage" : "$@" vs ./docopts -h "$usage" -- "$@"

Best regards

@Sylvain303
Copy link
Collaborator

PS: not related but i don't get why using : as a separator for usage args and not the conventional --

./docopts -h "$usage" : "$@" vs ./docopts -h "$usage" -- "$@"

Its legacy compatibility reason, with the python version, from previous maintainers version.

Before, you ask -h to pass Usage: is also legacy and add special code to handle it outside docopt lib. 😉

I will remove it in next major version, I think.

Back on track, calling within function. Add local, OK.You could try it too through a wrapper.

You know the 3 different call:

Global

$ ./docopts -h "Usage: hello WORLD [NAME]" : 'le monde' sylvain 
WORLD='le monde'
NAME='sylvain'

Global using prefix

$ ./docopts -G prefix -h "Usage: hello WORLD [NAME]" : 'Logiciel Libre' sylvain 
prefix_WORLD='Logiciel Libre'
prefix_NAME='sylvain'

And associative array:
because declare -A spawn local variable in this form of usage:

$ ./docopts -A 'args' -h "Usage: hello WORLD [NAME]" : 'le monde' sylvain 
declare -A args
args['WORLD']='le monde'
args['NAME']='sylvain'

I met that on our internal docopts.sh but from the need to exclude declare -A

How your call would look like inside your function?

@Sylvain303
Copy link
Collaborator

Hi @rockandska

Did you manage to perform some test with the actual output?
How would you call a function having a docopts Usage:?
Where would you like to store and pass the $usage value for a function?

@rockandska
Copy link
Author

Hi @Sylvain303

Sorry to not have take the time to answer.

How your call would look like inside your function?

Nothing fancy, your examples already show a call inside a function.
Adding another example will not add informations about the fact that calling docopts inside a function is not possible (clearly stated in README) since docopts use exit instead of return by default, and expose all variables as global by default.

if you are bash coder enough, I guess you can wrap docopts call and replace the output to suite your need
Back on track, calling within function. Add local, OK.You could try it too through a wrapper.

Sure i could write a wrapper for the exit and local in the meantime but will be better to not have to.

My issue is not about the fact I'm not able to achieve my needs with native docopts implementation (by adding some wrappers) but the need to add an option when using doctops inside a function to declare variables as local and using return instead exit.

Regards,

@Sylvain303
Copy link
Collaborator

Hi @rockandska

Thanks for your reply. In fact it would help me to go forward on this feature request, to see it working from your point of view. For now adding support to function wasn't requested. The exit statement comes from original docopts in python. So I can change it easily. And, as you seemed interested, your contribution is welcome, because it could be used to see the goal and for testing purpose too.

My questions was essentially on testing the PoC and prototyping what could be done. I will do some similar job in bash testing, before I could deliver something for that.

As mentioned -A already do the local scope in bash using a declare -A statement.
I did not experiment more, than what I posted.

Here is a more concrete case, I'm working on. Here follows a bash function with argument. I don't care about the function's code only how about it uses its arguments. And by laziness I don't have to write working code. 😉

# create_instance PROJECT_ID IMAGE_ID SSHKEY_ID HOSTNAME INIT_SCRIPT
# you can change flavor by defining FLAVOR_NAME global variable.
# outputs json
create_instance()
{
  local p=$1
  local image_id=$2
  local sshkey=$3
  local hostname=$4
  local init_script=${5:-}

  fail_if_empty sshkey hostname

  local myflavor=$FLAVOR_NAME

  debug "create_instance $p \"$image_id\" \"$sshkey\"  \"$hostname\" \"$init_script\""

  if [[ -z "$myflavor" ]]
  then
    # you can define it in cloud.conf
    myflavor=$DEFAULT_FLAVOR
  fi
  local flavor_id=$(get_flavor $p $myflavor)

  if [[ -z "$flavor_id" ]]
  then
    fail "'$myflavor' not found flavor_id on region $REGION"
  fi

  local create_json ret
  if [[ -n "$init_script" && -e "$init_script" ]]
  then
    # with an init_shostname
    local tmp_init=$(preprocess_init --json "$init_script")

    create_json="$(cat << END
{
  "flavorId": "$flavor_id",
  "imageId": "$image_id",
  "monthlyBilling": false,
  "name": "$hostname",
  "region": "$REGION",
  "sshKeyId": "$sshkey",
  "userData": "$(cat $tmp_init)"
}
END
)"

    #ovh_cli --format json cloud project $p instance create \
    #  --flavorId $flavor_id \
    #  --imageId $image_id \
    #  --monthlyBilling false \
    #  --name "$hostname" \
    #  --region $REGION \
    #  --sshKeyId $sshkey \
    #  --userData "$(cat $tmp_init)" \

    ## we merge the init_script in the outputed json so it becomes parsable
    ovhapi POST "/cloud/project/$p/instance" <<< "$create_json" \
        | jq_or_fail ". + {\"init_script\" : \"$tmp_init\"}"
    ret=$?

    if [[ $ret -eq 0 ]] ; then
      rm $tmp_init
    fi
  else
    # without init_script
    ovh_cli --format json cloud project $p instance create \
      --flavorId $flavor_id \
      --imageId $image_id \
      --monthlyBilling false \
      --name "$hostname" \
      --region $REGION \
      --sshKeyId $sshkey
    ret=$?
  fi

  return $ret
}

If I would like to use it with docopts It should require a simple syntax...

If I try to convert it... here's the full working code with some fake functions

#!/usr/bin/env bash
#
# This is a prototype wrapper to use docopts inside a function
# https://github.com/docopt/docopts/issues/43

# looks for local built docopts
PATH=..:$PATH

docopts_wrap_function() {
  local usage="$1"
  shift
  # first let's try to simply change exit with return
  docopts -A args -h "$usage" : "$@" | sed -e 's/^exit/return/'
}


DEFAULT_FLAVOR=s1-2
REGION=WAW1

function debug()
{
  if [[ $DEBUG -eq 1 ]] ; then
    # write on stderr
    >&2 echo "debug: $*"
  fi
}

get_flavor()
{
  echo "my_flavor_is_rich"
}

ovhapi() {
  if [ -t 0 ] ; then 
    BODY=""
  else
    BODY=$(cat)
  fi
  echo "BODY: $BODY"
  echo "ovhapi $@"
}

ovh_cli() {
  echo "ovh_cli $@"
}

preprocess_init() {
  echo "$2"
}

jq_or_fail() {
  echo '{ "json" : "somevm-id-1234556"}'
}

# you can change flavor by defining FLAVOR_NAME global variable.
# outputs json
create_instance()
{
  local u="$(cat <<END
Usage: create_instance PROJECT_ID IMAGE_ID SSHKEY_ID HOSTNAME [INIT_SCRIPT]

Arguments:
  PROJECT_ID    Openstack project id
  IMAGE_ID      The image base for creating the VM
  SSHKEY_ID     Id of the ssk public key to deploy
  HOSTNAME      Machine name or fqdn of the VM
  INIT_SCRIPT   Path to a local script bash or cloud_init
END
)"

  eval "$(docopts_wrap_function "$u" "$@")"

  local myflavor=$FLAVOR_NAME

  debug "create_instance ${args[PROJECT_ID]} \"${args[IMAGE_ID]}\" \"${args[SSHKEY_ID]}\"  \"${args[HOSTNAME]}\" \"${args[INIT_SCRIPT]}\""

  if [[ -z "$myflavor" ]]
  then
    # you can define it in cloud.conf
    myflavor=$DEFAULT_FLAVOR
  fi
  local flavor_id=$(get_flavor ${args[PROJECT_ID]} $myflavor)

  if [[ -z "$flavor_id" ]]
  then
    fail "'$myflavor' not found flavor_id on region $REGION"
  fi

  local create_json ret
  if [[ -n "${args[INIT_SCRIPT]}" && -e "${args[INIT_SCRIPT]}" ]]
  then
    # with an init_shostname
    local tmp_init=$(preprocess_init --json "${args[INIT_SCRIPT]}")

    create_json="$(cat << END
{
  "flavorId": "${args[FLAVOR_ID]}",
  "imageId": "${args[IMAGE_ID]}",
  "monthlyBilling": false,
  "name": "${args[HOSTNAME]}",
  "region": "$REGION",
  "sshKeyId": "${args[SSHKEY_ID]}",
  "userData": "$(cat $tmp_init)"
}
END
)"

    ## we merge the init_script in the outputed json so it becomes parsable
    ovhapi POST "/cloud/project/${args[PROJECT_ID]}/instance" <<< "$create_json" \
        | jq_or_fail ". + {\"init_script\" : \"$tmp_init\"}"
    ret=$?

    if [[ $ret -eq 0 ]] ; then
      echo rm $tmp_init
    fi
  else
    # without init_script
    ovh_cli --format json cloud project ${args[PROJECT_ID]} instance create \
      --flavorId ${args[FLAVOR_ID]} \
      --imageId ${args[IMAGE_ID]} \
      --monthlyBilling false \
      --name "${args[HOSTNAME]}" \
      --region $REGION \
      --sshKeyId ${args[SSHKEY_ID]}
    ret=$?
  fi

  return $ret
}

create_instance "$@"

I did not extensively tested, but it seems to work with the actual docopts and a single sed rewrite. Which of course suppose that the output doesn't contains any other "exit" text.

Could you try it on some code of yours to raise some limitation or missing behavior?
What to you think about this kind of use?

@Sylvain303
Copy link
Collaborator

Hi @rockandska

I now remember why I didn't go deeper in this feature before. One limitation is the performance!

Starting docopts and parsing is a bit expensive. For the whole bash script it should be ok, but if we start calling doctops again and again during the bash script execution it can become heavy.

That's also why I ask you some contribution idea, because I can't see where it could become useful in function call. As function are protected for some wrong call because, they become part of the code, and the options parsed from the cli give the opportunity to call function wisely.

Do you have a more explicit example?

@Sylvain303 Sylvain303 added the waiting for more example More input is required to clarify the problem. Please add more example to reproduce the issue. label Dec 6, 2020
@Sylvain303
Copy link
Collaborator

So @rqelibari submitted PR #57 , thanks.

If you feel it's acceptable about performance what is needed too (you can contribute or wait that I'll do it):

  • update README (section about using in function, with a performance warning)
  • add examples
  • check with -A and -G define what to do with local/global here
  • could we have a call syntax more "comfortable" than the actual usage sample in Enable the half-implemented -f, --function option #57 (which is ok, but a bit complicated)
  • eval for bash function call, what to return which error message, should we propagate failure?

In order to simplify function call syntax, here's a suggestion:

We can keep indent auto-removal. MacOS user was asking for bash3.51 compatibility, is it always that old? Oh, they moved, are moving, to zsh...

Here's a sample working bash code, not tested against patched docopts yet, only counting argument sent to a fake function.

#!/usr/bin/env bash
docopts_function()
{
  local usage="$1"
  shift

  # trim usage
  # remove leading whitespace characters
  usage="${usage#"${usage%%[!$'\n']*}"}"
  # remove trailing whitespace characters
  usage="${usage%"${usage##*[!$'\n']}"}"

  # and remove indent
  local indent=${usage%%[![:space:]]*}

  echo ">$usage< : ${args[@]}"
  echo "indent: '$indent' size: ${#indent}"

  # remove indent and call docopts
  sed -e "s/^$indent//" <<< "$usage" | docopts -f -V - -h - : "$@"
}

docopts()
{
  echo $#
}


myfunc()
{
echo "$(docopts_function "
    Usage: myfunc <your-name>
    ----
    Version: 1.0
   "  "$@"
   )"
}

myfunc -f pipo

The call is multi-line and still a bit wired:

myfunc()
{
# only echo here but should become eval with the return value if failing or new parsed val
echo "$(docopts_function "
    Usage: myfunc <your-name>
    ----
    Version: 1.0
   "  "$@"
   )"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement waiting for more example More input is required to clarify the problem. Please add more example to reproduce the issue.
Projects
None yet
Development

No branches or pull requests

2 participants