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

Parsing of command-line arguments is broken #43

Open
kurik opened this issue Jul 7, 2021 · 0 comments · May be fixed by #44
Open

Parsing of command-line arguments is broken #43

kurik opened this issue Jul 7, 2021 · 0 comments · May be fixed by #44

Comments

@kurik
Copy link

kurik commented Jul 7, 2021

The jsonpointer ver ~ 2 has the following issues parsing command line arguments:

  1. It is not compatible with the jsonpointer ver ~ 1
  2. It does not parse correctly the -f argument, so it is not possible to provide the pointer in a file (as it was possible in jsonpointer ver ~ 1)
  3. The help text from jsonpointer --help allows ambiguous explanation of the command line parameters
  4. The documentation The jsonpointer commandline utility follows the jsonpointer ver ~ 1 syntax.

Here are more details:
ad 1 and 4) The example from The jsonpointer commandline utility works fine with the jsonpointer ver ~ 1
Unfortunately with the jsonpointer ver ~ 2 the example generates the following error:

$ jsonpointer ./ptr.json ./a.json ./b.json
Could not resolve pointer: location must starts with /
Could not resolve pointer: location must starts with /

This is because the jsonpointer ver ~ 2 expects the pointer as the first argument, instead of a file with the pointer.

ad 2) When I try to use the -f argument to provide the pointer in a file, then the argument of the first json file (a.json) is parsed in a wrong way and jsonpointer does not understand it, as it considers the file name as a pointer expression:

$ jsonpointer -f ./ptr.json ./a.json ./b.json
usage: jsonpointer [-h] [-f [POINTER_FILE]] [--indent INDENT] [-v] [POINTER] FILE [FILE ...]
jsonpointer: error: argument POINTER: not allowed with argument -f/--pointer-file

ad 3) Let's assume the -f works, then the help text

$ jsonpointer --help
usage: jsonpointer [-h] [-f [POINTER_FILE]] [--indent INDENT] [-v] [POINTER] FILE [FILE ...]

has ambiguous explanation because POINTER_FILE is in square brackets. It can been understood as the POINTER_FILE is not mandatory when using the -f argument and then the -f can be used even with the POINTER.

I can write a patch to fix this issues, however before I do so, I need to understand what was the intention behind the change of the command-line syntax in ver ~ 2 and how it is expected those command-line parameters will work.

IMO the best way will be to keep the command-line syntax compatible with ver ~ 1. If there is a need to have a pointer expressed directly on command-line, then a new parameter (i.e. -e|--expression or -p|--ptr|--pointer) should be introduced. However as the ver ~ 2 is already in place for some time, this will introduce a regression to the current syntax in ver ~ 2.

andreasgerstmayr added a commit to andreasgerstmayr/python-json-pointer that referenced this issue Aug 5, 2021
This commit updates the CLI arguments to accept either a file containing
a json pointer expression, or an expression as a commandline argument:

    usage: jsonpointer [-h] (-f [POINTER_FILE] | -p [POINTER]) [--indent INDENT] [-v] FILE [FILE ...]

Without this commit

    jsonpointer "/a" a.json b.json

works fine, however using a file doesn't work:

    $ jsonpointer -f ptr.json a.json b.json
    usage: jsonpointer [-h] [-f [POINTER_FILE]] [--indent INDENT] [-v] [POINTER] FILE [FILE ...]
    jsonpointer: error: argument POINTER: not allowed with argument -f/--pointer-file

This commit also improves the usage message by explaining the mutually exclusive arguments,
adds tests for the changes, updates the documentation and adds a new CHANGELOG.md file.

Resolves stefankoegl#43
andreasgerstmayr added a commit to andreasgerstmayr/python-json-pointer that referenced this issue Aug 5, 2021
This commit updates the CLI arguments to accept either a file containing
a json pointer expression, or an expression as a commandline argument:

    usage: jsonpointer [-h] (-f [POINTER_FILE] | -p [POINTER]) [--indent INDENT] [-v] FILE [FILE ...]

Without this commit

    jsonpointer "/a" a.json b.json

works fine, however using a file doesn't work:

    $ jsonpointer -f ptr.json a.json b.json
    usage: jsonpointer [-h] [-f [POINTER_FILE]] [--indent INDENT] [-v] [POINTER] FILE [FILE ...]
    jsonpointer: error: argument POINTER: not allowed with argument -f/--pointer-file

This commit also improves the usage message by explaining the mutually exclusive arguments,
adds tests for the changes, updates the documentation and adds a new CHANGELOG.md file.

Resolves stefankoegl#43
andreasgerstmayr added a commit to andreasgerstmayr/python-json-pointer that referenced this issue Aug 9, 2021
This commit updates the CLI arguments to accept either a file containing
a json pointer expression, or an expression as a commandline argument:

    usage: jsonpointer [-h] (-f [POINTER_FILE] | -p [POINTER]) [--indent INDENT] [-v] FILE [FILE ...]

Without this commit

    jsonpointer "/a" a.json b.json

works fine, however using a file doesn't work:

    $ jsonpointer -f ptr.json a.json b.json
    usage: jsonpointer [-h] [-f [POINTER_FILE]] [--indent INDENT] [-v] [POINTER] FILE [FILE ...]
    jsonpointer: error: argument POINTER: not allowed with argument -f/--pointer-file

This commit also improves the usage message by explaining the mutually exclusive arguments,
adds tests for the changes, updates the documentation and adds a new CHANGELOG.md file.

Resolves stefankoegl#43
andreasgerstmayr added a commit to andreasgerstmayr/python-json-pointer that referenced this issue Aug 9, 2021
This commit updates the CLI arguments to accept either a file containing
a json pointer expression, or an expression as a commandline argument:

    usage: jsonpointer [-h] (-f [POINTER_FILE] | -p [POINTER]) [--indent INDENT] [-v] FILE [FILE ...]

Without this commit

    jsonpointer "/a" a.json b.json

works fine, however using a file doesn't work:

    $ jsonpointer -f ptr.json a.json b.json
    usage: jsonpointer [-h] [-f [POINTER_FILE]] [--indent INDENT] [-v] [POINTER] FILE [FILE ...]
    jsonpointer: error: argument POINTER: not allowed with argument -f/--pointer-file

This commit also improves the usage message by explaining the mutually exclusive arguments,
adds tests for the changes, updates the documentation and adds a new CHANGELOG.md file.

Resolves stefankoegl#43
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

Successfully merging a pull request may close this issue.

1 participant