Skip to content

refactor(ziextract): declare →zinit-extract once from a body variable - #3

Draft
vladdoster with Copilot wants to merge 5 commits into
mainfrom
copilot/maint-reduce-duplicate-zinit-extract-declaration
Draft

refactor(ziextract): declare →zinit-extract once from a body variable#3
vladdoster with Copilot wants to merge 5 commits into
mainfrom
copilot/maint-reduce-duplicate-zinit-extract-declaration

Conversation

Copilot AI commented Jul 24, 2026

Copy link
Copy Markdown

ziextract's extension case statement repeated the →zinit-extract() { … } declaration in all 15 branches. Each branch now sets only the function body, and the function is declared in a single place.

Case statement

  • Every branch assigns a single-quoted body string to a new local body instead of declaring a function. No expansion happens at assignment time; $file, ZINIT[tmp], and grep patterns resolve at call time as before.

Single declaration point

  • →zinit-extract is created once after esac.
  • [[ -n $body ]] replaces the $(typeset -f + →zinit-extract) subshell check that gated the extraction/warning=1 paths.
case "${${ext:+.$ext}:-$file}" in
    ((#i)*.zip)
        body='→zinit-check unzip "$file" || return 1; command unzip -qq -o "$file"'
        ;;
    #
esac

if [[ -n $body ]] {
    builtin eval "→zinit-extract() { $body }"
    .zinit-extract-wrapper "$file" →zinit-extract || { … }

Notes for reviewers

  • builtin eval is used rather than functions[→zinit-extract]=$body: zsh 5.9 mangles the multi-byte in identifiers when parsing a body assigned through the functions array, breaking the →zinit-check calls. The evaluated string is a static literal.
  • The .gz/.bz2/.xz branches keep their pre-existing file-rename logic outside the body assignment.
  • Net −10 LOC.

Copilot AI changed the title [WIP] Refactor ziextract case statement to reduce duplicate declaration refactor(ziextract): declare →zinit-extract once from a body variable Jul 24, 2026
Copilot AI requested a review from vladdoster July 24, 2026 19:18
@vladdoster
vladdoster requested a review from Copilot July 24, 2026 19:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors ziextract to avoid repeating the →zinit-extract() function declaration across each archive-type case branch by storing per-branch extraction logic in a body variable and declaring the function once after the case.

Changes:

  • Introduces a local body variable and updates each case branch to assign the extractor function body instead of redefining →zinit-extract().
  • Declares →zinit-extract() once after esac (gated by [[ -n $body ]]) and runs extraction via .zinit-extract-wrapper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread zinit-install.zsh
Comment on lines +1730 to 1732
if [[ -n $body ]] {
builtin eval "→zinit-extract() { $body }"
.zinit-extract-wrapper "$file" →zinit-extract || {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a newline terminator before the closing } when building the eval string in 378a439.

Comment thread zinit-install.zsh Outdated
Comment on lines 1709 to 1711
command cp -Rf ${attached_vol:-${TMPDIR:-/tmp}/acb321GEF}/*(D) .
retval=$?
command hdiutil detach $attached_vol

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoted $attached_vol in both cp (keeping the glob unquoted so it still expands) and hdiutil detach in 378a439.

- zinit-install.zsh: guard Makefile glob in ∞zinit-make-base-hook
  with (N.) qualifier so it doesn't fail with 'no matches found'
  when configure didn't produce a Makefile
- tests/plugins.zunit: use 'build' ice for tmux on all platforms
  (removes darwin-specific configure'--disable-utf8proc' that
  fails with current tmux autogen/configure)"
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.

[maint]: reduce duplicate →zinit-extract declaration in ziextract case statement

3 participants