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

Use chpwd instead of precmd hook, and add-zsh-hook #215

Closed
wants to merge 1 commit into from

Conversation

ericbn
Copy link
Contributor

@ericbn ericbn commented Oct 16, 2017

This triggers z at every working directory change, instead of at every prompt, in zsh. Use add-zsh-hook to set up the _z_chpwd function.

Also, update the async job call inside _z_chpwd. The parenthesis, which do the call in a subshell, are not necessary, and &! makes sure the job is immediately disowned (and there's no output of the process ID or it's termination status). See http://zsh.sourceforge.net/Doc/Release/Jobs-_0026-Signals.html

This code is compatible with zsh versions >= 5.0.

Also update man page and README.

Closes #210

@ericbn
Copy link
Contributor Author

ericbn commented Oct 20, 2017

add-zsh-hook guarantees that the same hook function is not added twice, so removed the [ -n "${precmd_functions[(r)_z_precmd]}" ] test.

@ericbn
Copy link
Contributor Author

ericbn commented Nov 1, 2017

I see that using cmd &! instead of (cmd &) breaks sourcing z.sh in other shells, like in sh. Keeping the original (cmd &) construct is probably a better idea here.

This triggers z at every working directory change, instead of at every
prompt, in zsh. Use add-zsh-hook to set up the _z_chpwd function.

Using `cmd &!` instead of `(cmd &)` breaks sourcing z.sh in other
shells, like in sh, so keeping that syntax.
See http://zsh.sourceforge.net/Doc/Release/Jobs-_0026-Signals.html

This code is compatible with zsh versions >= 5.0.

Also update man page and README.

Closes rupa#210
@mcornella
Copy link
Contributor

I would keep using the previous construct to add the function to chpwd, since it supports earlier zsh versions and you don't need to autoload add-zsh-hook. We could use a better, clearer syntax though:

chpwd_functions+=(_z_chpwd)

I have to say that I haven't checked it in other shells though.

@mcornella mcornella mentioned this pull request Aug 29, 2018
@mcornella
Copy link
Contributor

Indeed, dash chokes on that syntax because the parentheses makes it look like a function. We could use the old syntax that we know works everywhere, or we could do

typeset -a chpwd_functions
chpwd_functions+=_z_chpwd

The typeset is to force declare chpwd_functions as an array, otherwise we could get a string if it wasn't defined previously. This one's confirmed to pass the syntax check in bash and dash.

@rupa
Copy link
Owner

rupa commented Aug 31, 2018

I've been following this discussion with interest - as a bash user, wistful about having a chpwd hook - but for this utility, I think we should stick with prompt command. We're trying to weight by "time spent" in a directory, not just how many times we visited

@ericbn
Copy link
Contributor Author

ericbn commented Sep 11, 2018

Fair enough. I'm closing this then.

@ericbn ericbn closed this Sep 11, 2018
@ericbn ericbn deleted the zsh branch September 11, 2018 00:44
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.

3 participants