The Wayback Machine - https://web.archive.org/web/20201201191709/https://github.com/sindresorhus/pure/pull/459
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

Skip grep fork, use native zsh matching #459

Merged
merged 1 commit into from Feb 5, 2019
Merged

Conversation

@xPMo
Copy link
Contributor

@xPMo xPMo commented Feb 1, 2019

Method taken from http://zdharma.org/Zsh-100-Commits-Club/Zsh-Native-Scripting-Handbook.html

Just a small optimization, I obsess over this kind of stuff. Thanks for pure btw, I've adapted it quite a bit for my own purposes.

Copy link
Collaborator

@mafredri mafredri left a comment

Thanks, it's a good idea. Normally I wouldn't be too bothered by piping a command once, but I do feel this makes Pure more portable as well 😄.

pure.zsh Outdated
@@ -517,7 +517,9 @@ prompt_pure_state_setup() {
who_out=$(who -m 2>/dev/null)
if (( $? )); then
# Who am I not supported, fallback to plain who.
who_out=$(who 2>/dev/null | grep ${TTY#/dev/})
declare -a who_in

This comment has been minimized.

@mafredri

mafredri Feb 1, 2019
Collaborator

This is a bit pedantic but could we change declare to local here? That, or typeset. The reason I'm suggesting it is that Pure doesn't use declare anywhere else, and for anyone reading this code, the mixture of declarations might just cause confusion. I.e. why use declare here, does it have a special purpose? I personally like local due to the additional information the name conveys (this is a local variable) albeit using typeset (or declare for that matter) in this context produces the exact same result.

Apologies for the wall of text 😅.

This comment has been minimized.

@xPMo

xPMo Feb 1, 2019
Author Contributor

Whoops, I thought there was something going on with the local keyword and -a, but apparently I misread -g as -a in the manpage. GJ me. Committing.

pure.zsh Outdated
who_out=$(who 2>/dev/null | grep ${TTY#/dev/})
declare -a who_in
who_in=( ${(f)"$(who 2>/dev/null)"} )
who_out="${(M)who_in:#*${TTY#/dev/}*}"

This comment has been minimized.

@mafredri

mafredri Feb 1, 2019
Collaborator

I noticed a bug in the previous implementation which we could solve by changing the matching ever so slightly => *${TTY#/dev/}[^0-9]*. I did a quick test and this seems to work, can you find any problems with it or a better approach?

So the problem is that pts/1 matches pts/12 (as an example), in both this PR and previous implementation, this would produce multiple matches which is not what we want.

This comment has been minimized.

@xPMo

xPMo Feb 1, 2019
Author Contributor

*[:space:]${TTY#/dev/}[:space:]* would be safe to the point of paranoid, which is always good.

@xPMo
Copy link
Contributor Author

@xPMo xPMo commented Feb 1, 2019

I'll squash and --force push if you're okay with this.

@mafredri
Copy link
Collaborator

@mafredri mafredri commented Feb 1, 2019

Looks good, go ahead 👍.

@xPMo xPMo force-pushed the xPMo:optimize branch from d01c39e to a558fdb Feb 1, 2019
pure.zsh Outdated
who_out=$(who 2>/dev/null | grep ${TTY#/dev/})
local -a who_in
who_in=( ${(f)"$(who 2>/dev/null)"} )
who_out="${(M)who_in:#*[:space:]${TTY#/dev/}[:space:]*}"

This comment has been minimized.

@mafredri

mafredri Feb 1, 2019
Collaborator

Hmm, [:space:] doesn't seem to work in my shell, does it require some option to be enabled?

This comment has been minimized.

@mafredri

mafredri Feb 1, 2019
Collaborator

Ah, seems to work with [[:space:]].

This comment has been minimized.

@xPMo

xPMo Feb 1, 2019
Author Contributor

Ah, found it in the manual:

Note that the square brackets are additional to those enclosing the whole set of characters, so to test for a single alphanumeric character you need [[:alnum:]]. Named character sets can be used alongside other types, e.g. [[:alpha:]0-9].

@xPMo xPMo force-pushed the xPMo:optimize branch from a558fdb to f8d0c98 Feb 1, 2019
@mafredri mafredri merged commit 47f9bfd into sindresorhus:master Feb 5, 2019
@mafredri
Copy link
Collaborator

@mafredri mafredri commented Feb 5, 2019

@xPMo went ahead and merged this although I don't have a busybox system to test this with at the moment, but I can't think of anything that could go wrong.

Thanks again! 😄

filipekiss added a commit to filipekiss/pure that referenced this pull request Apr 26, 2019
* upstream/master:
  Add pure-pwsh to the ports section of the readme (sindresorhus#467)
  Skip grep fork, use native zsh matching (sindresorhus#459)
  Add pure-now to Ports section in the readme (sindresorhus#458)
  1.9.0
  Update to zsh-async 1.7.1 and recover from unexpected worker death (sindresorhus#454)
  Add conda environment name to precmd (sindresorhus#440)
  Add Mímir to Ports section in the readme (sindresorhus#438)
  Avoid calling zle reset-prompt in precmd (sindresorhus#431)
  Simplify async tasks by not passing $PWD (sindresorhus#430)
  Abort git check if pwd has changed after invocation (sindresorhus#428)
  Update zsh-async to 1.7.0 (sindresorhus#429)
  1.8.0
  Advice against enabling incompatible Oh-My-Zsh plugins
  Show warning when Oh My Zsh themes are enabled (sindresorhus#426)
  Add support for VI-mode indicator (sindresorhus#405)
  Remove unused function for computing string length (sindresorhus#418)
  Update fpath-reference link (sindresorhus#417)
  Prevent IPv6 regexp from capturing the time (sindresorhus#413)
  Prevent hostname from showing up in local X sessions (sindresorhus#398)
  Make sure local HUP trap is unset during git fetch
  Improve the debug prompt (PS4) (sindresorhus#396)
  Show options as a table in readme (sindresorhus#407)
  Remove unfrequent FAQs from readme (sindresorhus#406)
  Prevent interactive prompts during git fetch (sindresorhus#397)
  Set title via atomic print statement (sindresorhus#399)
  Revert local prompt_opts, breaks promptinit
  Fix for setopt not taking effect when sourcing pure
  Fix prompt_pure_state on older versions of zsh
  Fix wrong placement of localoptions during prompt init
  Fix line erasure when terminal output does not end in newline (sindresorhus#391)
  Try to detech SSH connection when SSH_CONNECTION is unset (sindresorhus#393)
  Always force BatchMode for the Git SSH command (sindresorhus#392)
  1.7.0
  Avoid setting title over serial console (sindresorhus#388)
  Hide virtualenv when explicitly disabled by the user (sindresorhus#381)
  Add install guide for Zplugin (sindresorhus#386)
  Update URL to Droid Sans Mono font (sindresorhus#387)
  Prevent multiple prompt resets in one execution cycle (sindresorhus#368)
  More thorough handling (hiding) of match results
  Avoid implicit creation of global var prompt_pure_git_arrows
  1.6.0
  Link to pure.zsh and async.zsh for better clarity (sindresorhus#358)
  Readme tweaks
  Link to a Pure-inspired prompt done in Rust
  Avoid implicit global var creation and cleanup (sindresorhus#347)
kgrz added a commit to kgrz/pure that referenced this pull request Jul 18, 2019
* upstream/master: (25 commits)
  Fix incorrect color name in the readme (sindresorhus#482)
  1.10.3
  Fix username prompt expansion
  1.10.2
  Bump version number in pure state
  Fix bad set of key/value pairs on old versions of ZSH
  1.10.1
  Suppress warning of WARN_CREATE_GLOBAL option (sindresorhus#479)
  1.10.0
  Update the zsh-async version to reflect the code
  Improve code comments
  Avoid performing prompt reset in zle-line-finish hook (sindresorhus#477)
  Add support for configuring colors with zstyle (sindresorhus#472)
  Improve the style of the continuation prompt (PS2) (sindresorhus#323)
  Fix username color (sindresorhus#450)
  Add `prompt_pure_system_report` for reporting issues (sindresorhus#468)
  Add pure10k to the list of ports (sindresorhus#474)
  Create funding.yml
  Add pure-pwsh to the ports section of the readme (sindresorhus#467)
  Skip grep fork, use native zsh matching (sindresorhus#459)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.