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

Can't suppress abbr expansion when executing command with no args #8423

Closed
lilyball opened this issue Nov 9, 2021 · 10 comments
Closed

Can't suppress abbr expansion when executing command with no args #8423

lilyball opened this issue Nov 9, 2021 · 10 comments

Comments

@lilyball
Copy link
Contributor

lilyball commented Nov 9, 2021

Version: fish, version 3.3.1

I have an abbreviation abbr -a -g -- ls exa. If I want to run e.g. ls -a by itself, I can press Ctrl+Space to insert a space without expanding the abbreviation. Unfortunately there's no equivalent for "execute without expanding abbreviation". Even adding the space (e.g ls ) still expands it when I execute.

I'm also not sure if it's even possible for me to write my own binding here. The binding for \r is just execute, which implies that execution implicitly expands abbreviations (whereas if it were expand-abbr execute then I could have a binding to execute that skips the abbreviations).

@faho faho added this to the fish-future milestone Nov 9, 2021
@faho
Copy link
Member

faho commented Nov 9, 2021

Tbh I don't think execute should expand the abbreviations, and I think the binding shouldn't do it by default either.

I kept that when I introduced expand-abbr because it was hardcoded on space and \r before, and I didn't want to change the behavior as part of that.

(also it shouldn't be expanded if there's a space after - the idea was that you could press ctrl-space to insert a space and then do whatever, but I see that still triggers the expansion)

@lilyball
Copy link
Contributor Author

The default \r binding definitely should expand abbreviations, otherwise ls<return> and ls<space><return> would do different things and that would be very confusing. Also it breaks the notion of "abbreviations are for changing how interactive input is interpreted, without changing behavior from scripts or hiding this change in behavior from the log as aliases do".

Anyone who wants return to not expand can then rebind it. I just wish we could do Ctrl-Return to execute without expanding, but return is already \cM so that doesn't work.

@faho
Copy link
Member

faho commented Nov 11, 2021

With ls<return> what is shown in the commandline and what is executed differ. That's also confusing.

If e.g. you made "ls" an abbreviation for "exa", with ls<space> you would see it change to "exa" before you execute it, with ls<return> you wouldn't - it would look like you're executing "ls".

This breaks the notion that abbreviations change what is written in the commandline and, unlike aliases, allow you to change and inspect it before execution.

An alternative is to not execute if an abbreviation expands, so ls<return> would turn into ls --color=auto and a second return would execute it.

This would be bound like bind \r expand-abbr or execute.

@lilyball
Copy link
Contributor Author

100% of my usage of abbr expects foo<return> to expand the abbreviation. Yes, it does mean that what I typed and what gets executed is different, but I can see that it expanded when looking at the output. And hopefully I know my own abbreviations too, so I know what to expect!

If we change the \r binding to be expand-abbr execute then users who want different behavior certainly can rebind it to e.g. expand-abbr or execute. But please let's not break the current behavior of abbreviations.

@faho
Copy link
Member

faho commented Nov 11, 2021

100% of my usage of abbr expects foo to expand the abbreviation

I on the other hand have been surprised and annoyed by this.

If we change the \r binding to be expand-abbr execute then users who want different behavior certainly can rebind it to e.g. expand-abbr or execute. But please let's not break the current behavior of abbreviations.

And if we don't, anyone who wants this behavior can add the expand-abbr. Or press space.

@faho
Copy link
Member

faho commented Nov 11, 2021

How about we do the uncoupling first and then discuss whether to change the default separately?

@floam
Copy link
Member

floam commented Nov 11, 2021

I agree that abbreviation<ENTER> should transform and execute.

@zanchey
Copy link
Member

zanchey commented Nov 11, 2021

100% of my usage of abbr expects foo<return> to expand the abbreviation.

Likewise.

@faho
Copy link
Member

faho commented Nov 11, 2021

Okay, let's keep it then. Guess I have some customization to do!

krobelus added a commit to krobelus/fish-shell that referenced this issue Nov 14, 2021
On a commandline like "ls arg" (cursor at end) we do not expand
abbrevations on enter.  OTOH, on "ls " we do expand. This can be
frustrating because it means that the two obvious ways to suppress
abbrevation expansion (C-Space or post-expansion C-Z) cannot be used to
suppress expansion of a command without arguments.  (One workaround is
"ls #".)

Only expand-on-execute if the cursor is at the command name (no space
in between).

This is a strict improvement for realistic scenarios, because if there
is a space, the user has already expressed the intent to not expand
the abbreviation. (I hope no one is using recursive abbreviations.)

Closes fish-shell#8423
krobelus added a commit to krobelus/fish-shell that referenced this issue Nov 14, 2021
On a commandline like "ls arg" (cursor at end) we do not expand
abbrevations on enter.  OTOH, on "ls " we do expand. This can be
frustrating because it means that the two obvious ways to suppress
abbrevation expansion (C-Space or post-expansion C-Z) cannot be used to
suppress expansion of a command without arguments.  (One workaround is
"ls #".)

Only expand-on-execute if the cursor is at the command name (no space
in between).

This is a strict improvement for realistic scenarios, because if there
is a space, the user has already expressed the intent to not expand
the abbreviation. (I hope no one is using recursive abbreviations.)

Closes fish-shell#8423
@krobelus
Copy link
Member

I made it so we don't expand if there is a space, I think that makes sense?

I got a very spooky CI failure with asan, adding more sleeps helped..

@faho faho modified the milestones: fish-future, fish 3.4.0 Nov 27, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants