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

handlr 0.12.0 completion scripts is faulty #89

Closed
R3G3N3R4T0R opened this issue Oct 27, 2024 · 14 comments · Fixed by #92
Closed

handlr 0.12.0 completion scripts is faulty #89

R3G3N3R4T0R opened this issue Oct 27, 2024 · 14 comments · Fixed by #92

Comments

@R3G3N3R4T0R
Copy link

Current Behavior

The generated completer is faulty. For open and mime subcommands it only complete options and will not complete paths, and completes nothing if the current word doesn't match the options. For launch and get it only complete extensions, not options, and continues to do so after typing 2 dashes.

The new created script also seems to hard code the path to handlr by prepending cwd to argv[0] unless argv[0] is not a path. If the completion script is generated before installation the path would be invalid and suppress the original completer. Might be better to document it in the README.

Expected Behavior

It should generate completion for both options and paths/extensions. If a dash or 2 dashes are at the front of the completing word then it should complete options, and if ./ is at the front of the current word it should complete only paths.

Distribution and Version

handlr version : 0.12.0
Distribution : Artix Linux using Arch Linux extra repository

I do not see any commit to the code after the tag release so I did not compile it myself.

@Anomalocaridid
Copy link
Owner

What terminal shell are you using?

The new created script also seems to hard code the path to handlr by prepending cwd to argv[0] unless argv[0] is not a path. If the completion script is generated before installation the path would be invalid and suppress the original completer. Might be better to document it in the README.

I am already aware of this and I plan to update the README.md soon. I just have not gotten around to doing so yet. Sorry if that caused any inconvenience.

@R3G3N3R4T0R
Copy link
Author

zsh and bash but I have replicated it with fish

@Anomalocaridid
Copy link
Owner

I can confirm that I can replicate this issue on my end. I'll look into it further a bit later.

@Anomalocaridid
Copy link
Owner

@R3G3N3R4T0R What's the version of the Arch package you are using, including the pkgrel number?

@R3G3N3R4T0R
Copy link
Author

0.12.0-2
0.12.0-1 to 0.12.0-2 should just be renaming the command in completion files.
I don't know about 0.12.0 without pkgrel.
I have also built the current PKGBUILD locally but the behavior didn't change, and there is no warning about anything wrong.

@Anomalocaridid
Copy link
Owner

Sorry about being slow to respond.

What happens if you run the following?

  • source $(COMPLETION=bash) in bash
  • source $(COMPLETION=zsh) in zsh
  • eval (COMPLETION=fish) in fish

@R3G3N3R4T0R
Copy link
Author

I can understand being late, I am also quite busy lately.
I am not sure what they are supposed to do since there is no command.
bash

bash: source: filename argument required
source: usage: source filename [arguments]

zsh

source: not enough arguments

fish

fish: Unsupported use of '='. In fish, please use 'set COMPLETION fish'.
eval (COMPLETION=fish)
      ^~~~~~~~~~~~~~^

Of course adding handlr to it only printed the help page before the error and if I interpret it as loading those functions
in the session with process substitution source <(COMPLETE=shell handlr), then the same behavior happen as before. It wasn't that the code wasn't loaded before since completion is still happening, it is just errant for some reason. I install the old completion scripts to the same location when I am not testing.

Something strange happened that the launch and get subcommands seems to be working properly on zsh and fish? It may be bash specific but I don't have elvish to check, and the version is still 0.12.0-2.

@Anomalocaridid
Copy link
Owner

I am not sure what they are supposed to do since there is no command.

My bad, I meant to put handlr in the commands.

Of course adding handlr to it only printed the help page before the error and if I interpret it as loading those functions
in the session with process substitution source <(COMPLETE=shell handlr), then the same behavior happen as before. It wasn't that the code wasn't loaded before since completion is still happening, it is just errant for some reason. I install the old completion scripts to the same location when I am not testing.

Strange. I can replicate this. I'll have to look into what's causing this.

Something strange happened that the launch and get subcommands seems to be working properly on zsh and fish? It may be bash specific but I don't have elvish to check, and the version is still 0.12.0-2.

It was fixed because you sourced the completion generated directly from handlr and for some reason it works better for zsh and fish (except for bash for some reason).

In handlr-regex's Arch package, the completion scripts are patched to remove the erroneously hard-coded binary path. I'm guessing there's something going wrong here that's causing some of this.

I'll continue trying to figure out a way to get clap_complete to not hard-code the binary path in the completion scripts to hopefully eliminate the need for something like that.

@Anomalocaridid
Copy link
Owner

I finally got around to fixing this in the fix-completion-scripts branch. Would you mind testing it to see if the completions work on your system as well?

handlr open and handlr launch should complete paths as expected and completion scripts should no longer hard-code the binary's entire path.

@R3G3N3R4T0R
Copy link
Author

Can confirm open works as expected now, mime still haven't changed but once I replaced the path completer in cli.rs with the one in Open it also work as expected. I also do not need to fix the completion scripts for completions to appear.

launch and get still suffer the same problem in bash, as previously reported zsh and fish has no problem. I found that the options are in the list but they are at the bottom and so not visible until you scroll all the way down. It seems to know to complete for extensions and mime types instead of options if the word does not start with -, but does not whittle down the list based on typed characters and instead list all known extensions and mime types. Typing - does not make it complete only options either. Observably, if the first character of the word following launch is -, then I have 2153 possibilities; if it is not -, I have 2149 possibilities.

@Anomalocaridid
Copy link
Owner

Sorry, totally missed mime. Thanks for catching it.

I think I have narrowed down the cause of the issues with launch and get. Both use custom ArgValueCompleters.

Now I need to figure out whether this issue is on my end or on clap_complete's end. Worst case scenario, it's on clap_complete's end and I can't fix it until the clap developers fix it.

@Anomalocaridid
Copy link
Owner

Anomalocaridid commented Dec 18, 2024

So the bug is probably my fault. In any case, I think I fixed the completions in bash. Would you mind giving it a try on your system?

Thank you for your patience and cooperation, by the way.

@R3G3N3R4T0R
Copy link
Author

It works on my system now, also checked zsh and fish with the 4 sub-commands to see if there is any regression (not that it is likely).

I just realized the path completion is faulty though, basically clap-rs/clap/#5239, but I can replicate it on bash and zsh, only fish is immune. I am not sure how it reappeared when it is added to CI/CD and we are using clap 4.5.
There is also lack of ~ expansion and variable expansion, the former is probably doable, latter not sure. All of these may just be with clap_complete though since you only imported the completer. This issue can probably be closed, unless it is again some issue of the code here.

I do find it weird I am the only one reporting this though, maybe not many people use it directly? I often open apps from the shell instead of the file manager so I kinda have a stake in it, only if I know how to read code quickly.

@Anomalocaridid
Copy link
Owner

I just realized the path completion is faulty though, basically clap-rs/clap/#5239, but I can replicate it on bash and zsh, only fish is immune. I am not sure how it reappeared when it is added to CI/CD and we are using clap 4.5.

I am guessing that ValueHint and ArgValueCompleter have separate implementations, since the former is used for "static" completions and the latter is used for "dynamic" completions. So fixing a bug in one would not necessarily fix the other.

Or maybe the dynamic completions are unstable enough there are still significant holes in test coverage.

In any case, I might have to issue a bug report.

There is also lack of ~ expansion and variable expansion, the former is probably doable, latter not sure. All of these may just be with clap_complete though since you only imported the completer. This issue can probably be closed, unless it is again some issue of the code here.

I think those might be more issues with clap_complete, especially since it appears to still be kind of a work in progress a work in progress.

I do find it weird I am the only one reporting this though, maybe not many people use it directly? I often open apps from the shell instead of the file manager so I kinda have a stake in it, only if I know how to read code quickly.

That might be possible. To my understanding, users tend to mainly use handlr-regex for scripts and keybinds and mainly only directly use it in a command line for editing or viewing mimeapps.list.

Regardless, I am glad someone noticed it and reported it so that others with your use case no longer have to deal with it.

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 a pull request may close this issue.

2 participants