Skip to content

Use zsh chpwd hooks to look for .phpbrewrc instead of trap call#985

Merged
c9s merged 1 commit into
phpbrew:masterfrom
kevinnio:kp/use-zsh-hook-instead-of-debug-trap
Oct 28, 2018
Merged

Use zsh chpwd hooks to look for .phpbrewrc instead of trap call#985
c9s merged 1 commit into
phpbrew:masterfrom
kevinnio:kp/use-zsh-hook-instead-of-debug-trap

Conversation

@kevinnio

@kevinnio kevinnio commented Oct 26, 2018

Copy link
Copy Markdown
Contributor

If PHPBREW_RC_ENABLE is set, we look for .phpbrewrc after EVERY command, even though we only need to look for it when the current directory changes. That trap call may be needed in bash, but zsh provides a hook mechanism for various shell events, including changing directory.

This PR makes use of that hook mechanism to look for .phpbrewrc ONLY when the current directory changes. This affects only zsh users.

Pros of this approach:

  • Looks for .phpbrewrc ONLY when changing directories, instead of after EVERY command.
  • Plays nice with other tools who might have included their own functions in the same cd hook.
  • Integrates seamlessly into zsh ecosystems.

Cons of this approach:

  • Benefits only zsh users (at least for now).

You can check http://zsh.sourceforge.net/Doc/Release/Functions.html#Hook-Functions for more info on zsh hooks.

The original approach was looking for .phpbrewrc after EVERY command, when
in reality we only need to look for it when the current directory changes.
`trap` may be needed in bash, but zsh includes a hook mechanism for various
shell events, including changing directory.

This commit makes use of that hook mechanism to look for .phpbrewrc ONLY
when the current directory changes.

This affects only zsh users.

You can check http://zsh.sourceforge.net/Doc/Release/Functions.html#Hook-Functions
for more info on zsh hooks.
@kevinnio kevinnio force-pushed the kp/use-zsh-hook-instead-of-debug-trap branch from 514ff86 to 14521b3 Compare October 26, 2018 07:26
@kevinnio kevinnio changed the base branch from develop to master October 26, 2018 07:27
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.8%) to 52.515% when pulling 14521b3 on kevinnio:kp/use-zsh-hook-instead-of-debug-trap into ce35a8b on phpbrew:master.

1 similar comment
@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.8%) to 52.515% when pulling 14521b3 on kevinnio:kp/use-zsh-hook-instead-of-debug-trap into ce35a8b on phpbrew:master.

@coveralls

coveralls commented Oct 26, 2018

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.8%) to 52.541% when pulling 2e394ad on kevinnio:kp/use-zsh-hook-instead-of-debug-trap into ce35a8b on phpbrew:master.

@kevinnio kevinnio closed this Oct 26, 2018
@kevinnio kevinnio reopened this Oct 26, 2018
@kevinnio kevinnio force-pushed the kp/use-zsh-hook-instead-of-debug-trap branch from 2e394ad to 14521b3 Compare October 28, 2018 04:27
@c9s

c9s commented Oct 28, 2018

Copy link
Copy Markdown
Member

Looks good to me

@c9s c9s merged commit 68a2b1a into phpbrew:master Oct 28, 2018
@kevinnio kevinnio deleted the kp/use-zsh-hook-instead-of-debug-trap branch October 29, 2018 16:11
@morozov morozov added this to the Release 1.24 milestone Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants