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

Fix dashless option usage info #800

Merged
merged 2 commits into from
May 11, 2023

Conversation

sambostock
Copy link
Contributor

🌈

When option aliases are specified without dashes

option :from, aliases: [:f]

they are correctly handled when parsing the command

my_cli hello -f Loki Thor
from: Loki
Hello Thor

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:

 my_cli help hello  
 Usage:
   my_cli hello NAME

 Options:
-  f, [--from=FROM]  
+  -f, [--from=FROM]  

 say hello to NAME

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.
@@ -105,11 +105,11 @@ def option(name, options = {})

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")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@rafaelfranca rafaelfranca merged commit 5a6bd00 into rails:main May 11, 2023
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>
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>
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 this pull request may close these issues.

2 participants