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

Add limit option for zoxide query #655

Closed
wants to merge 1 commit into from
Closed

Conversation

shouya
Copy link

@shouya shouya commented Dec 8, 2023

This PR adds a simple option to specify the number of directories to output in zoxide query.

This option is useful when you only want to peek what the matching directories look like without listing them all. I'm currently expecting to use it in a hand-crafted fish completion script.

I currently write the completion script as:

complete -c z -x -a '(zoxide query -l -- $argv 2>/dev/null | head -n 20)'

The redirection is needed because otherwise you get a panic due to EPIPE. Now with this limit option I can just write:

complete -c z -x -a '(zoxide query -l -n 20 $argv)'

@ajeetdsouza ajeetdsouza force-pushed the main branch 2 times, most recently from d72d9ce to cbb8e77 Compare February 12, 2024 22:37
@ajeetdsouza
Copy link
Owner

What version of zoxide are you running? I'm seeing no panics with zoxide query -ls | head -2 on the latest version, the exit code is 0 as well.

@ajeetdsouza ajeetdsouza added the waiting-for-response Waiting for a response from the issue author. label Feb 15, 2024
@shouya
Copy link
Author

shouya commented Feb 16, 2024

Weirdly I can no longer reproduce the error, even though I still run the same version when I met the error.

From strace I can still see SIGPIPE is sent to the process, but the process no longer crashes.

$ strace -e 'trace=!all' zoxide query -l | head -n 1
/home/shou/tmp
--- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=326529, si_uid=1000} ---
--- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=326529, si_uid=1000} ---
+++ exited with 0 +++

Then I noticed the BrokenPipeHandler is already in-place two years ago, long before I attempt to fix the issue. I'm puzzled now on what went on back when I submit the PR.

But anyway, since we no longer see the panic and the stream is also efficiently consumed with head, I see no reason to add the limiting feature. Feel free to close this PR if you agree.

@ajeetdsouza
Copy link
Owner

Strange. Perhaps you had an older version of zoxide installed? Either way, I'll close the PR for now. Do let me know if it crops up again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-for-response Waiting for a response from the issue author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants