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 cli package from mangling option arguments with equal signs #3925

Merged
merged 5 commits into from
Nov 23, 2021

Conversation

ergl
Copy link
Member

@ergl ergl commented Nov 22, 2021

The current command parser in the cli package cuts option arguments of String type at the first equal sign. This release fixes the problem for long options (--option=foo=bar) and for short options such as -O=foo=bar. Short options such as -Ofoo=bar will continue to raise an "ambiguous args" error.

The code below shows the bug, with the option argument being cut short at the first equal sign. The code below prints "foo" instead of the complete value, "foo=bar". The same is true when uses the short version of the option, like -t=foo=bar.

use "cli"

actor Main
  new create(env: Env) =>
    try
      let cs =
        CommandSpec.leaf("simple", "", [
          OptionSpec.string("test" where short' = 't')
        ])?
      
      let args: Array[String] = [
        "ignored"; "--test=foo=bar"
      ]
      let cmdErr = CommandParser(cs).parse(args) as Command
      env.out.print(cmdErr.option("test").string())
    end

@ergl ergl added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Nov 22, 2021
@ergl ergl changed the title Fix cli package from mangling options with equal signs Fix cli package from mangling option arguments with equal signs Nov 22, 2021
@ergl ergl force-pushed the ergl/fix_cli_equal branch from 7057c2d to e6caefd Compare November 22, 2021 14:39
@@ -320,16 +336,17 @@ class iso _TestShortsNext is UnitTest
h.assert_eq[String]("bbb", stringso.string_seq()(1)?)

class iso _TestLongsEq is UnitTest
fun name(): String => "ponycli/shorts_eq"
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed a repeated test name as I went along.

@jemc jemc merged commit c197ab0 into main Nov 23, 2021
@jemc jemc deleted the ergl/fix_cli_equal branch November 23, 2021 19:09
github-actions bot pushed a commit that referenced this pull request Nov 23, 2021
github-actions bot pushed a commit that referenced this pull request Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants