Skip to content

Conversation

@CatsDeservePets
Copy link
Collaborator

No description provided.

@CatsDeservePets
Copy link
Collaborator Author

Hello @joelim-work.
This is just a proof of concept. It allows overriding builtin commands and still call their original counterparts.
I am fully aware that this could lead to problems due to states not getting updated through function calls etc.

What it could solve: Altering the behaviour of builtin commands while also inheriting their command line completions. This could allow for better integration of tools like zoxide. It would also allow to solve the “annoyance” I have with maps and map being different commands (and instead switch the behaviour depending on whether parameters are present or not).

Just wanting to know whether this idea might be worth to explore further or not.

@joelim-work
Copy link
Collaborator

This sounds a lot like commmand ... used in shells to execute external commands, bypassing alternate definitions like aliases or shell functions.

I tried your patch but doesn't allow redefining :map since it is parsed as a mapExpr object. Note that the parser expects mapExpr objects to be in a particular form (i.e. map <keys> Expr), so even if you override the command, I assume it can't be called if the parser cannot generate the appropriate command object.

@CatsDeservePets
Copy link
Collaborator Author

It was intended to work just like the command in shells.

You are right about the map expression, I did not consider it (that was the one thing I did not test and just assumed...).

@joelim-work
Copy link
Collaborator

Apart from map which doesn't work, how useful is this feature in practice? I don't remember seeing any issues from users about wanting to define a custom command but the name is already used by a built-in command.

@CatsDeservePets
Copy link
Collaborator Author

Apart from map which doesn't work, how useful is this feature in practice? I don't remember seeing any issues from users about wanting to define a custom command but the name is already used by a built-in command.

Think of advanced aliases that are functions and not just text substitution (that is how aliases work in fish shell for example). This way one can modify the behaviour of builtin commands without the need of remembering a new name and while still having dedicated completions. I am completely fine with not adding this, I wanted to investigate what is possible thought it may be useful.
Personally, I would definitely use it to alias cd to zoxide.

@joelim-work
Copy link
Collaborator

It sounds like you're not satisfied with the existing cd command so you want to override it with your own custom command while keeping both the name and completions. Although I don't think being forced to use a different name is much of an issue even if it longer, as push can be used to type it out quickly (e.g. map z push :cd-zoxide<space>.

As for keeping completions, this probably is just part of the larger feature request of having custom completions, whether it is by allowing the user to define their own from scratch, or by piggybacking off an existing command. The idea of having custom completions does sounds rather niche to me (I barely even use the command line myself during everyday usage, let alone tab completions), and I don't recall anyone actually requesting such a feature. If this kind of feature is implemented, will you consider other niche use cases, such as redefining cd but with a different completion behavior, or defining another command to have the same completion as cd? The thing about adding features is that often the initial implementation will only be sufficient to cover some use cases, then other users will come along and request extensions, and over time the feature will become far more complex than originally intended. Customizing the ruler (and other parts of the UI) is a good example of this.

@CatsDeservePets
Copy link
Collaborator Author

This PR was more about replacing builtin commands while also inheriting its completion. I do like and use completions all the time (as you can probably tell by now). In terms of improving them for custom commands, I have this completely separate idea, which would also allow for custom command descriptions, which is something users actually request:
#758 (comment)

I completely agree with your points and really don't want to add features without thinking them through which will only every benefit myself and bloat lf's codebase in the long term.

In general, my mind always seems to race from one topic to another (also besides programming). I love exploring many different things. That is why I often come up with many different ideas (which sometimes even contradict each other). I hope I don't bother you too much will all the drafts etc.

@joelim-work
Copy link
Collaborator

I'm going to assume that the main purpose of this PR is to inherit completions, and not about overriding existing command names which can be worked around by choosing a different name like my-cd. I can't speak for other users, but the reason I don't use the command line or completions much is because it is slower than keybindings. For commands that require an argument like cd, 'completing' them using a tool like fzf integration requires less keystrokes (e.g. <c-f>foobar<enter> vs. :cd ~/foo<tab>/bar<tab><enter>). But anyway that is just my preference.

I'm not saying that it's impossible to implement more advanced behavior for customizing completion logic - any kind of feature can be implemented if code is written for it. It's just that I don't value it as much when considering the cost-benefit ratio. In comparison, yazi doesn't even support command line completions (operations like cd and zoxide are implemented as builtin widgets instead of a generic command line), and it seems to do fine.

You are welcome to keep this PR around and/or start a discussion to see if other users have anything else to say on the topic.


A couple of other points in this PR I forgot to mention:

  1. Replacing cd with a custom command would mean that the new command inherits the completions from cd. To be precise, the completion module is simply not aware that the cd command has been replaced, so the solution appears slightly hacky to me. In any case I would then expect that builtin cd (the original command) support the same completions too.
  2. I notice that you ignore existing overrides (open/paste/delete/rename) to avoid having to deal with them. I'm not sure if I'm a fan of maintaining two separate mechanisms for overriding a command. In fact, I'm not even sure why a builtin command would have to be overridden just to provide alternative functionality. For example configuring an alternate rename could be done as:
    map r %{{
        # alternate behavior
    }}
    
    Or:
    cmd rename-file %{{
        # alternate behavior
    }}
    
    map r rename-file
    

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.

2 participants