Fix dashless option usage info#800
Merged
rafaelfranca merged 2 commits intoMay 11, 2023
Merged
Conversation
Aliases are prefixed with a dash if they do not contain one. However, this only makes sense on single letter aliases. Multi-character aliases with a single dash cannot actually be used (although they can be specified). Therefore, these tests are updated to use aliases that would actually work.
sambostock
commented
Nov 14, 2022
| describe "with key as an array" do | ||
| it "sets the first items in the array to the name" do | ||
| expect(parse([:foo, :bar, :baz], true).name).to eq("foo") | ||
| expect(parse([:foo, :b, "--bar"], true).name).to eq("foo") |
Contributor
Author
There was a problem hiding this comment.
As far as I can tell, these two tests were invalid.
Specifying a multicharacter alias without including leading dashes leads to a single dash being prepended (rather than two), which is invalid, as there is no way to invoke it.
By example:
| Specified Alias | Final Alias |
|---|---|
"-b", :"-b", "b", :b, |
-b |
"--bar", :"--bar" |
--bar |
"bar", :bar |
-bar (invalid) |
Therefore, I updated these tests to use legitimate aliases, which then make sense with normalization.
Specifying invalid aliases is still allowed, it just now will generate a single dash in the usage documentation (which reflects what is actually going on).
Another PR could address forbidding invalid aliases (as they do not work, and would be misleading) in isolation.
Although specifying single letter aliases without a dash works, it generated incorrect USAGE documentation (the dash is missing). This moves the alias normalization to happen earlier, which ensures the aliases are described correctly in the USAGE documentation.
0e61aa3 to
ef1a323
Compare
Vasu1105
added a commit
to inspec/inspec
that referenced
this pull request
Oct 20, 2023
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
4 tasks
Vasu1105
added a commit
to inspec/inspec
that referenced
this pull request
Oct 23, 2023
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
Vasu1105
added a commit
to inspec/inspec
that referenced
this pull request
Oct 23, 2023
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
Vasu1105
added a commit
to inspec/inspec
that referenced
this pull request
Oct 23, 2023
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
Vasu1105
added a commit
to inspec/inspec
that referenced
this pull request
Oct 23, 2023
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
Vasu1105
added a commit
to inspec/inspec
that referenced
this pull request
Oct 23, 2023
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
clintoncwolfe
pushed a commit
to inspec/inspec
that referenced
this pull request
Oct 23, 2023
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 (#6815) Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
clintoncwolfe
pushed a commit
to inspec/inspec
that referenced
this pull request
Oct 23, 2023
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 (#6817) Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
clintoncwolfe
pushed a commit
to inspec/inspec
that referenced
this pull request
Oct 23, 2023
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 (#6818) Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
Nik08
pushed a commit
to inspec/inspec
that referenced
this pull request
Sep 13, 2024
…) which started breaking the test. Till we find if recent release is stable and don't break any functionality we are pinning thor to < 1.3.0 (#6815) Signed-off-by: Vasu1105 <vasundhara.jagdale@progress.com>
owst
added a commit
to bambooengineering/thor-zsh_completion
that referenced
this pull request
Jan 2, 2025
Thor >= 1.3 adds dashes to aliases: rails/thor#800 Therefore, depending on the thor version, aliases will either already have a dash or need one adding; one can't be added unconditionally. Shellcheck SC2316: https://github.com/koalaman/shellcheck/wiki/SC2316 catches a combination of local and readonly commands that doesn't do the intended thing (define a readonly, local variable named DEPTH). Using `local -r` does the right thing: https://zsh.sourceforge.io/Doc/Release/Shell-Builtin-Commands.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🌈
When option aliases are specified without dashes
they are correctly handled when parsing the command
However the usage information does not include the dash. This PR fixes that by moving alias normalization to happen sooner, so it applies to both the usage information generator and the actual command: