Skip to content

Commit

Permalink
Turn off optparse backtracking to show help for the current command (p…
Browse files Browse the repository at this point in the history
  • Loading branch information
fsoikin authored Aug 31, 2024
1 parent 26ff517 commit 1f9ecda
Show file tree
Hide file tree
Showing 9 changed files with 208 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ Other improvements:
- always using forward slash as path separator in lockfile, regardless of the
platform, so that the lockfile doesn't keep changing when team members run
Spago on different platforms.
- when encountering a mistyped option for a command, Spago will show help for
that command, not root help.

## [0.21.0] - 2023-05-04

Expand Down
3 changes: 3 additions & 0 deletions bin/src/Main.purs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Node.Path as Path
import Node.Process as Process
import Options.Applicative (CommandFields, Mod, Parser, ParserPrefs(..))
import Options.Applicative as O
import Options.Applicative.Types (Backtracking(..))
import Optparse as Optparse
import Record as Record
import Registry.PackageName as PackageName
Expand Down Expand Up @@ -508,6 +509,8 @@ parseArgs = do
( p
{ prefShowHelpOnError = true
, prefShowHelpOnEmpty = true
-- Needed to avoid things like https://github.com/purescript/spago/issues/1146
, prefBacktrack = NoBacktrack
}
)
)
Expand Down
36 changes: 36 additions & 0 deletions test-fixtures/1146-cli-help/build.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
Invalid option `--bogus'

Usage: index.dev.js build [--migrate] [--monochrome|--no-color] [--offline]
[-q|--quiet] [-v|--verbose] [--backend-args ARGS]
[--ensure-ranges] [--json-errors] [--output DIR]
[--pedantic-packages] [--pure] [--purs-args ARGS]
[-p|--package PACKAGE] ([--verbose-stats] |
[--censor-stats]) [--strict]
Compile the project

Available options:
--migrate Migrate the spago.yaml file to the latest format
--monochrome,--no-color Force logging without ANSI color escape sequences
--offline Do not attempt to use the network. Warning: this will
fail if you don't have the necessary dependencies
already cached
-q,--quiet Suppress all spago logging
-v,--verbose Enable additional debug logging, e.g. printing `purs`
commands
--backend-args ARGS Arguments to pass to the running script
--ensure-ranges Add version bounds for all the dependencies of the
selected project
--json-errors Output compiler warnings/errors as JSON
--output DIR The output directory for compiled files
--pedantic-packages Check for redundant or missing packages in the config
and fail the build if any
--pure Use the package information from the current
lockfile, even if it is out of date
--purs-args ARGS Arguments to pass to purs compile. Wrap in quotes.
`--output` and `--json-errors` must be passed to
Spago directly.
-p,--package PACKAGE Select the local project to build
--verbose-stats Show counts for each warning type
--censor-stats Censor warning/error summary
--strict Promotes project sources' warnings to errors
-h,--help Show this help text
19 changes: 19 additions & 0 deletions test-fixtures/1146-cli-help/registry-search.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
Invalid option `--bogus'

Usage: index.dev.js registry search [--migrate] [--monochrome|--no-color]
[--offline] [-q|--quiet] [-v|--verbose]
[--json] PACKAGE
Search for package names in the Registry

Available options:
--migrate Migrate the spago.yaml file to the latest format
--monochrome,--no-color Force logging without ANSI color escape sequences
--offline Do not attempt to use the network. Warning: this will
fail if you don't have the necessary dependencies
already cached
-q,--quiet Suppress all spago logging
-v,--verbose Enable additional debug logging, e.g. printing `purs`
commands
--json Format the output as JSON
PACKAGE Package name
-h,--help Show this help text
28 changes: 28 additions & 0 deletions test-fixtures/1146-cli-help/root-error-command.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
Invalid argument `bogus'

Usage: index.dev.js (COMMAND | (-v|--version))
PureScript package manager and build tool

Available options:
-h,--help Show this help text
-v,--version Show the current version

Available commands:
init Initialise a new project
fetch Downloads all of the project's dependencies
install Compile the project's dependencies
uninstall Remove dependencies from a package
build Compile the project
run Run the project
test Test the project
bundle Bundle the project in a single file
sources List all the source paths (globs) for the
dependencies of the project
repl Start a REPL
publish Publish a package
upgrade Upgrade to the latest package set, or to the latest
versions of Registry packages
docs Generate docs for the project and its dependencies
registry Commands to interact with the Registry
ls List packages or dependencies
graph Generate a graph of modules or dependencies
28 changes: 28 additions & 0 deletions test-fixtures/1146-cli-help/root-error-option.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
Invalid option `--bogus'

Usage: index.dev.js (COMMAND | (-v|--version))
PureScript package manager and build tool

Available options:
-h,--help Show this help text
-v,--version Show the current version

Available commands:
init Initialise a new project
fetch Downloads all of the project's dependencies
install Compile the project's dependencies
uninstall Remove dependencies from a package
build Compile the project
run Run the project
test Test the project
bundle Bundle the project in a single file
sources List all the source paths (globs) for the
dependencies of the project
repl Start a REPL
publish Publish a package
upgrade Upgrade to the latest package set, or to the latest
versions of Registry packages
docs Generate docs for the project and its dependencies
registry Commands to interact with the Registry
ls List packages or dependencies
graph Generate a graph of modules or dependencies
26 changes: 26 additions & 0 deletions test-fixtures/1146-cli-help/root-help.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Usage: index.dev.js (COMMAND | (-v|--version))
PureScript package manager and build tool

Available options:
-h,--help Show this help text
-v,--version Show the current version

Available commands:
init Initialise a new project
fetch Downloads all of the project's dependencies
install Compile the project's dependencies
uninstall Remove dependencies from a package
build Compile the project
run Run the project
test Test the project
bundle Bundle the project in a single file
sources List all the source paths (globs) for the
dependencies of the project
repl Start a REPL
publish Publish a package
upgrade Upgrade to the latest package set, or to the latest
versions of Registry packages
docs Generate docs for the project and its dependencies
registry Commands to interact with the Registry
ls List packages or dependencies
graph Generate a graph of modules or dependencies
2 changes: 2 additions & 0 deletions test/Spago.purs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Effect (Effect)
import Effect.Aff (Milliseconds(..))
import Test.Spago.Build as Build
import Test.Spago.Bundle as Bundle
import Test.Spago.Cli as Cli
import Test.Spago.Docs as Docs
import Test.Spago.Errors as Errors
import Test.Spago.Glob as Glob
Expand Down Expand Up @@ -40,6 +41,7 @@ main = do
runSpecAndExitProcess' config [ Spec.Reporter.consoleReporter ] do
Spec.describe "spago" do
-- TODO: script
Cli.spec
Init.spec
Sources.spec
Install.spec
Expand Down
64 changes: 64 additions & 0 deletions test/Spago/Cli.purs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
module Test.Spago.Cli where

import Test.Prelude

import Data.String as String
import Data.String.Regex as Regex
import Data.String.Regex.Flags as RF
import Test.Spec (Spec)
import Test.Spec as Spec

spec :: Spec Unit
spec = Spec.around withTempDir do
Spec.describe "CLI command parsing" do
Spec.it "#1146 on mistyped command option, shows help for the current command, not root help" \{ spago, fixture } -> do
spago [ "build", "--bogus" ] >>= shouldBeFailureErr' (fixture "1146-cli-help/build.txt")
spago [ "registry", "search", "--bogus" ] >>= shouldBeFailureErr' (fixture "1146-cli-help/registry-search.txt")

Spec.it "#1146 on mistyped command or root option, shows root help" \{ spago, fixture } -> do
spago [ "bogus" ] >>= shouldBeFailureErr' (fixture "1146-cli-help/root-error-command.txt")
spago [ "--bogus" ] >>= shouldBeFailureErr' (fixture "1146-cli-help/root-error-option.txt")

Spec.it "#1146 can show help and version" \{ spago, fixture } -> do
spago [ "--help" ] >>= shouldBeSuccessOutput' (fixture "1146-cli-help/root-help.txt")
spago [ "--version" ] >>= shouldBeSuccess

where
shouldBeSuccessOutput' fixture = checkOutputsWithPatch isRight (Just fixture) Nothing
shouldBeFailureErr' fixture = checkOutputsWithPatch isLeft Nothing (Just fixture)

checkOutputsWithPatch result stdout stderr =
checkOutputs'
{ stdoutFile: stdout
, stderrFile: stderr
, result
, sanitize:
String.trim
>>> Regex.replace progNameRegex "Usage: index.dev.js"
>>> Regex.replace optionsLineRegex " $1"
}

-- On Windows progname has the full path like
-- "Usage: C:\whatever\index.dev.js", but on Unix
-- it's just "Usage: index.dev.js"
progNameRegex = unsafeFromRight $ Regex.regex "Usage: .*index\\.dev\\.js" RF.noFlags

-- This regex catches "hanging" lines that list possible options after a
-- command, like this:
--
-- Usage: index.dev.js build [--option] [--another-option]
-- [--third-option] [--foo]
-- [-f|--force] [--help]
--
-- The second and third line in this example are aligned to whererever the
-- command name happened to end, and this will be different between Unix and
-- Windows, because on Unix the command is just the file name `index.dev.js`,
-- but on Windows it includes the full path, and worse, it's going to be
-- different path on differemt machines with different configurations.
--
-- So to work around this we collapse those "hanging" lines with options by
-- replacing newline and subsequent wide whitespace with a single space, like:
--
-- Usage: index.dev.js build [--option] [--another-option] [--third-option] [--foo] [-f|--force] [--help]
--
optionsLineRegex = unsafeFromRight $ Regex.regex "\\n\\s+(\\(\\[-|\\[-|PACKAGE)" RF.global

0 comments on commit 1f9ecda

Please sign in to comment.