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

Replace command-not-found event with a known function #7293

Closed
faho opened this issue Aug 28, 2020 · 2 comments
Closed

Replace command-not-found event with a known function #7293

faho opened this issue Aug 28, 2020 · 2 comments

Comments

@faho
Copy link
Member

faho commented Aug 28, 2020

Fish has long had the fish_command_not_found event that is triggered when a command isn't known.

It's always been a bit weird to me that this used the event machinery because

  • it makes it awkward to replace the handler (because it has already been registered by the time you try)
  • there is no real use for multiple handlers for this (unlike eg. pre-exec)

So really, this should just be a function that is invoked by name, fish_command_not_found, and if that isn't defined the default "fish: command 'choochoo' not found" one should take over.

This would also allow us to remove some rather weird logic from __fish_config_interactive and just put it in a normal function file.

@ridiculousfish ridiculousfish added this to the fish-future milestone Aug 29, 2020
@zanchey
Copy link
Member

zanchey commented Aug 31, 2020

Sounds good. There's a reasonable number of event handlers in the wild, so perhaps as an initial implementation a fish_command_not_found function should just emit the event?

@faho
Copy link
Member Author

faho commented Aug 31, 2020

Thankfully this is even easier. To allow overriding, we currently already check for a function of a certain name (because you don't want 2 handlers at the same time):

# This can be overridden by defining a new __fish_command_not_found_handler function
if not type -q __fish_command_not_found_handler

So we can just check that and either call that function or copy it.

(i.e. in practice it was already a function of a given name anyway, but I don't want to reuse that name)

faho added a commit to faho/fish-shell that referenced this issue Sep 4, 2020
Previously, when a command wasn't found, fish would emit the
"fish_command_not_found" *event*.

This was annoying as it was hard to override (the code ended up
checking for a function called `__fish_command_not_found_handler`
anyway!), the setup was ugly,
and it's useless - there is no use case for multiple command-not-found handlers.

Instead, let's just call a function `fish_command_not_found` if it
exists, or print the default message otherwise.

The event is completely removed, but because a missing event is not an error
(MEISNAE in C++-speak) this isn't an issue.

Note that, for backwards-compatibility, we still keep the default
handler function around even tho the new one is hard-coded in C++.

Also, if we detect a previous handler, the new handler just calls it.

This way, the backwards-compatible way to install a custom handler is:

```fish
function __fish_command_not_found_handler --on-event fish_command_not_found
    # do a little dance, make a little love, get down tonight
end
```

and the new hotness is

```fish
function fish_command_not_found
    # do the thing
end
```

Fixes fish-shell#7293.
@faho faho closed this as completed in 340de73 Sep 6, 2020
@krobelus krobelus modified the milestones: fish-future, fish 3.2.0 Sep 6, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants