Skip to content

Conversation

@jkarni
Copy link
Contributor

@jkarni jkarni commented Nov 24, 2023

Explicit shell/run arguments

Previously we just tacked on $@ to the end of the shell script. But often this is completely the wrong place for it, and for basically anything that is a proper shell script (e.g. editGarnConfig) rather than single command, makes it impossible to correctly pass arguments down.

This PR changes behavior so that you have to explicitly use $@/$*. This is exactly like any bash script, so I don't think it should be too confusing.

Stacked on #437

Fixes #363.

Previously you had to pass `--` *twice* in order to get it to an
underlying executable. This PR fixes that by inlining subparsers.

A further advantage of this change is that you can now mix global and
local (subcommand-specific) options. It seems like the old behavior was
only there for backwards-compatibility reasons (see
pcapriotti/optparse-applicative#433 (comment)).
Previously we just tacked on $@ to the end of the shell script. But
often this is completely the wrong place for it, and for basically
anything that is a proper shell script (e.g. editGarnConfig) rather than
single command, makes it impossible to correctly pass arguments down.

This PR changes behavior so that you have to explicitly use $@/$*. This
is exactly like any bash script, so I don't think it should be too
confusing.
Copy link
Contributor

@soenkehahn soenkehahn left a comment

Choose a reason for hiding this comment

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

Yeah, I think this is probably better than the current behavior. Although it's a breaking change and has some downsides, e.g. something like garn.shell("eslint") doesn't allow passing in arguments anymore. Still, I think an improvement.

cc: @alexdavid

Base automatically changed from jkarni/shell-subcommand-args to main November 24, 2023 19:48
Co-authored-by: Sönke Hahn <soenkehahn@gmail.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.

Support passing CLI arguments to garn run that begin with -

3 participants