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

Bug in expansion of variable length parameters (when count of parameters == 0) #53

Closed
DanaLacoste opened this issue Sep 7, 2021 · 2 comments

Comments

@DanaLacoste
Copy link

DanaLacoste commented Sep 7, 2021

Description

docopts.go is expanding empty arrays to be a 1-element array with a single null/empty string element in

s = fmt.Sprintf("('%s')", strings.Join(arr_out[:],"' '"))

Details

OK, this one is a bit edge-case-y, but it causes an issue with the way bash expansion works. Specifically, docopts is not creating an empty array when a ... element is defined, but no parameters are given. Instead, it creates an array containing a single null element.

This gets confusing, because bash considers it an empty array (if you query the length of the array) as the element is null, but if you pass it using standard bash expansion, it will be expanded to an empty string which will confuse any downstream commands.

Test Case 1

Here is the test script (I tried to use the use case defined in testcases.docopt, but I could not trace down exactly why that specific test case is not failing here)

NOTE: You need two scripts to see the problem clearly :)

test1.sh: this just runs the basic test case which has the bug:

#!/bin/bash

#docopts --debug -h - : "$@" <<EOF
#Test script for ... in docopts
#Usage:
  #$0 [NAME [NAME ...]]
  #$0 -h | --help
#EOF

eval "$(docopts -h - : "$@" <<EOF
Test script for ... in docopts
Usage:
  $0 [NAME [NAME ...]]
  $0 -h | --help
EOF
)"

echo "NAME: Contains ${#NAME} elements"

./test2.sh "${NAME[@]}"

and test2.sh:

#!/bin/bash

echo "$0: Called with $# parameters"

And the results:

% ./test1.sh 1
NAME: Contains 1 elements
./test2.sh: Called with 1 parameters
% ./test1.sh 1 2
NAME: Contains 1 elements
./test2.sh: Called with 2 parameters
% ./test1.sh
NAME: Contains 0 elements
./test2.sh: Called with 1 parameters

As you can see:

  • The first call has 1 param, it gets passed as 1 param, no problem
  • The second case has 2 params, passed as 2, no problem
  • The third case has 0 params, but it is passed as 1.

Test Case 2: Seeing this in pure bash, without docopts

So, let's find the root cause: what does docopts output that bash doesn't handle well?

test3.sh:

#!/bin/bash

declare -a empty_array
empty_array=()

declare -a one_null_array
one_null_array=('')

echo "Empty array:"
./test2.sh "${empty_array[@]}"

echo "One null array:"
./test2.sh "${one_null_array[@]}"

And the result:

% ./test3.sh
Empty array:
./test2.sh: Called with 0 parameters
One null array:
./test2.sh: Called with 1 parameters

Conclusion:

Somehow, this is expanding the empty array once: can we change it so that it just emits a name=() instead of name=('') for this use case? (I need to learn more go :) )

s = fmt.Sprintf("('%s')", strings.Join(arr_out[:],"' '"))

Perhaps we could (maybe) move the ' into the Join rather than outside?

@Sylvain303
Copy link
Collaborator

Hi @DanaLacoste

You're right:

# len of array in bash is ${#var_name[@]}
$ NAME=()
$ echo ${#NAME[@]}
0

$ NAME2=('')
$ echo ${#NAME2[@]}
1

lets try python's version 0.6.1

docker run -ti -v $PWD:/data debian-docpots  docopts -h 'Usage: prog [NAME...]' :
NAME=()

Which was in the code too:

list: lambda x: '(' + ' '.join(map(shellquote, x)) + ')',

So I fixed it. Tanks for reporting.

docopts/docopts.go

Lines 185 to 187 in e7d60ea

if len(arr) == 0 {
// bash emtpy array
s = "()"

It wont fail with test in: testcases.docopt as test are run through testee.sh which in turn uses -A mode which uses a different code path.

So, I added a functional test case for that + unit test_case in go:

@test "multiple length argument returns a 0 sized array" {
# Global mode
run $DOCOPTS_BIN -h 'Usage: prog [NAME...]' :
[[ $status -eq 0 ]]
expected_regexp='NAME=\(\)'
[[ "$output" =~ $expected_regexp ]]
[[ ${#lines[@]} -eq 1 ]]

"EMPTY_ARRAY=()",

Also test are now more documented:

https://github.com/docopt/docopts/blob/e7d60ea17b98db2b1314d7eee7bdf3a3b80722d2/docs/developer.md#add-more-test-to-docopts-use-case

Let me know it it works as expected for you.

@DanaLacoste
Copy link
Author

Yep, that looks perfect, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants