-
-
Notifications
You must be signed in to change notification settings - Fork 90
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
Using flag.conflicts causes issues with set -u #559
Comments
I can't reproduce the problem. Using the provided bashly.yml and $ ./cli -qd
conflicting options: -d cannot be used with --quiet and shellcheck does not have any complaint: Are you on bashly 1.2.2? The only place the generated script has the variable # :command.parse_requirements_while
while [[ $# -gt 0 ]]; do
key="$1"
case "$key" in
# :flag.case and I don't see how it can be unbound. As a side note: All of the tests and example tests run in strict mode, including this conflicts example. |
I use the latest bashly image as an alias.
This is how the parse_requirements function is generated:
The docker image uses bashly 1.2.2, I checked and the header of my generated script also says so |
Your conflicts example only works as long as there are no commands set in the bashly.yml As soon as I add commands the script is generated with the unbound variable: name: download
help: Sample application to demonstrate the use of conflicting flags
version: 0.1.0
flags:
- long: --cache
help: Enable cache
# Running --cache with --no-cache is not permitted
conflicts: [--no-cache]
- long: --no-cache
help: Disable cache
# Running --no-cache with --cache or with --fast is not permitted
conflicts: [--cache, --fast]
- long: --fast
help: Run faster
# Make sure to add the conflicting flags in both flags
conflicts: [--no-cache]
commands:
- name: test
alias: t
help: Example to show parse_requirements is bugged
This is where the fixed_flags_filter is generating the case where the key var is not bound:
Without commands, the flags are generated using And as soon as commands are added, all flags are generated with the |
Confirmed. Will be fixed. |
Thanks! |
In fact, the problem is deeper and more specific. For example, this works: commands:
- name: test
alias: t
help: Example to show parse_requirements is bugged
flags:
- long: --fast
short: -f
conflicts: [--slow]
- long: --slow
short: -s
conflicts: [--fast] The fixed args |
Sure! I'm currently re-writing a script using bashly and so far it has been a good experience and I actually need this to work, so I'll test as soon as you can provide a fix for it. A little offtopic here, but I wondered if you know where I can find more libs for bashly? Using the github search doesn't provide very good results since "bashly" is fuzzy matched against "bash" so I get way too many results, google doesn't provide any good results. I thought since your project had 2k stars there would be more libs around, since I'm really missing some good logging library. |
Happy to hear it, and if something does not work as you expect, I am happy to help in your migration (issues / discussions are both fine). As for libs for bashly - I don't know if there are any, but as you probably know by now - these are just functions that you place in |
This should be fixed now in #560, merged to master, and deployed to the docker edge tag. You can alias like this: $ docker pull dannyben/bashly:edge
$ alias bashly-edge='docker run --rm -it --user $(id -u):$(id -g) --volume "$PWD:/app" dannyben/bashly:edge' |
Works!
|
Excellent. I want to fix #561 and then I will release (probably later today). Thanks for the report. |
Great! Thanks a lot for the quick fix! |
Version 1.2.3 is live with this fix. |
Bashly Version
Latest Docker Image
Description
Using flag.conflicts causes issues with set -u.
If I use strict mode or rather set -u and then try to use conflicting variables I get an unbound variable error since $key is unbound.
Contents of bashly.yml
Reproduction Steps
Use the provided bashly config + settings.yml below, generate a script and then execute the script with -qd or -qv or -qdv.
Actual Behavior
parse_requirements will be generated like this:
Expected Behavior
But it should be generated like this:
The text was updated successfully, but these errors were encountered: