Completions shipped in dir incompatible with fisher 4

[ismay]

I've noticed that the completions tend to behave a little buggy for this plugin. See here for example, screenshot before and after pressing tab:

I suspect that the reason this is failing is because this plugin when installed with fisher, does not follow fisher's recommendation of shipping completions in a completions dir: https://github.com/jorgebucaran/fisher#creating-a-plugin. Should be simple enough to address, I can create a PR if you want.

I'm using fish 3.4.1, fisher 4.3.1, prompt is hydro, and kitty 0.24.4. Let me know if you need more info.

[krobelus]

I can create a PR if you want.

that would be nice, TIA

[ismay]

In looking through the source, it seems that it's been set up to allow for a variable command name, i.e. you can choose to use j or jo or anything you want instead of z.

That's not compatible with my suggestion of shipping the completions in the completions dir. If we want to do that there should be a fixed command name, because the completions filename should be identical to the command to complete:

Fish automatically searches through any directories in the list variable $fish_complete_path, and any completions defined are automatically loaded when needed. A completion file must have a filename consisting of the name of the command to complete and the suffix .fish. https://fishshell.com/docs/current/completions.html#where-to-put-completions

My suggestion would be to remove the logic that allows for a variable command name.

• It duplicates fish's native manner of aliasing a command. Fish functions used for aliasing can be lazy loaded.
• A fixed command name would allow the completions to be lazy loaded as well, instead of the current conf script that runs each time fish starts.

This is just from a quick look at the code, so let me know what you think of this suggestion.

[krobelus]

On Fri, Apr 01, 2022 at 02:27:57AM -0700, ismay wrote:

In looking through the source, it seems that it's been set up to allow for a variable command name, i.e. you can choose to use j or jo or anything you want instead of z.

That's not compatible with my suggestion of shipping the completions in the completions dir. If we want to do that there should be a fixed command name, because the completions filename should be identical to the command to complete:

Fish automatically searches through any directories in the list variable $fish_complete_path, and any completions defined are automatically loaded when needed. A completion file must have a filename consisting of the name of the command to complete and the suffix .fish. https://fishshell.com/docs/current/completions.html#where-to-put-completions

My suggestion would be to remove the logic that allows for a variable command name.

• It duplicates fish's native manner of aliasing a command. Fish functions used for aliasing can be lazy loaded. That's more performant.
• A fixed command name would allow the completions to be lazy loaded, instead of the current conf script that runs each time fish starts.

This is just from a cursory glance at the code, so let me know what you think of this suggestion.

I think you're right. The variable for the command name sounds like a feature that users can already achieve with a simple function myz --wraps z; z $argv; end That being said, I haven't looked into the details since I don't even use z myself at the moment (I'm happy with CDPATH). A quick search on github or grep.app should give an indication of how many user your change would break (hardly any, I'd assume) So I'd say go for it.

-- Reply to this email directly or view it on GitHub: https://github.com/jethrokuan/z/issues/107#issuecomment-1085674474 You are receiving this because you commented.

Message ID: @.***>

[ismay]

PR #108 fixes the bug. The potential solution we've discussed here is not necessary to fix this, but I think simplifying to a fixed command name would still be a good change for fisher compatibility. I'll see if I can create a separate PR for that.

[ismay]

I'll see if I can create a separate PR for that.

Done, see #109