Conversation
|
Also see #2705 |
|
I have read that PR and think the main issue back then was that standard commands could be overridden. This PR does not allow that (like Git). |
michaelkuhn
force-pushed
the
aliases
branch
3 times, most recently
from
119510a to
32ebbc0
Compare
|
Looks like I'm hitting some strange test failures due to using |
|
I like it! Go for it! |
|
This is ready from my side. If you have any more comments, please let me know. |
|
@tgamblin @adamjstewart ping |
|
I don't usually merge core changes, but I do agree that this is a nice feature to add. Maybe @becker33 or @scheibelp can review? |
|
@tgamblin @becker33 @scheibelp ping I have fixed a conflict, so this should be ready again. |
|
@tgamblin @becker33 @scheibelp ping ping |
|
This has been finished for some time from my point of view, I would really appreciate a review. |
|
How does this work for subsubcommands, like |
|
It currently only supports creating aliases on a subcommand level. You can, however, map them so subsubcommands. For instance, the following works: Edit: Looking at the code again, it might be possible to support them on a subsubcommand level. I'm not sure whether that's a good idea, though. |
This comment has been minimized.
This comment has been minimized.
It is probably helpful sometimes to spit out a warning but that’s as far as I’d go. Something like “alias ‘foo’ would run ‘bar’ which is not a spack command”, but again it could be annoying going back in time — if you alias a recent spack command then use an older version of spack. |
I am having a hard time thinking of when I’d actually want a space… and we have no commands with spaces. Let’s disallow it. I can’t decide if it should just warn or if it should throw an error. Leaning towards error. |
What about something like: config:
aliases:
comp a: compiler add
comp f: compiler find
... |
|
@adamjstewart I think we should save that for later, for subcommand aliases. Right now that won't be parsed the way I think you're expecting -- you'd have to quote it like |
|
So I guess, given that, how about a warning for aliases with spaces in them -- so that old versions of spack aren't too prickly if you end up with a configured subcommand alias for some future spack version. |
|
I've pushed a new commit that will print a warning for aliases containing a space. And FYI, #40895 implements the "did you mean" functionality. Edit: Added one more commit to emit a warning for aliases attempting to override built-in commands. |
Or aliases of aliases... 😉 |
Aliases can be used to define new commands. For instance, `sp: spec -I` will define a new command `sp` that will execute `spec` with the `-I` argument. Aliases cannot override existing commands.
The test makes sure that aliases cannot override Spack commands. This also fixes a small bug in SpackCommand and delays the `add_command` call until it is actually necessary; otherwise, it will complain about aliases it does not know about.
Print a warning for aliases containing a space.
|
3 years later. Glad we finally got this in! |
The shell wrapper currently does not preserve spaces in commands. For example, `bin/spack 'foo bar'` has `'foo bar'` interpreted as its command, while `spack 'foo bar'` turns this into `bin/spack foo bar`. This is more of a theoretical problem, but while we are at it, incorporate a few more suggestions given by ShellCheck. See: spack#17229 (comment)
Add a new config section: `config:aliases`, which is a dictionary mapping aliases
to commands.
For instance:
```yaml
config:
aliases:
sp: spec -I
```
will define a new command `sp` that will execute `spec` with the `-I`
argument.
Aliases cannot override existing commands, and this is ensured with a test.
We cannot currently alias subcommands. Spack will warn about any aliases
containing a space, but will not error, which leaves room for subcommand
aliases in the future.
---------
Co-authored-by: Todd Gamblin <[email protected]>
Add a new config section: `config:aliases`, which is a dictionary mapping aliases
to commands.
For instance:
```yaml
config:
aliases:
sp: spec -I
```
will define a new command `sp` that will execute `spec` with the `-I`
argument.
Aliases cannot override existing commands, and this is ensured with a test.
We cannot currently alias subcommands. Spack will warn about any aliases
containing a space, but will not error, which leaves room for subcommand
aliases in the future.
---------
Co-authored-by: Todd Gamblin <[email protected]>
The shell wrapper currently does not preserve spaces in commands. For example, `bin/spack 'foo bar'` has `'foo bar'` interpreted as its command, while `spack 'foo bar'` turns this into `bin/spack foo bar`. This is more of a theoretical problem, but while we are at it, incorporate a few more suggestions given by ShellCheck. See: spack#17229 (comment)
7 participants
Aliases can be used to define new commands. For instance,
sp: spec -Iwill define a new commandspthat will executespecwith the-Iargument. Aliases cannot override existing commands.This is still WIP and is missing some things, including schema adaptions and porting the old aliases to the new system. It already works in general. For convenience, there is an example alias
sp(which I will remove for the final version). I would just like to get some feedback on whether this is something of interest.@adamjstewart @tgamblin @alalazo
Closes #2705