Skip to content

fix: avoid inserting completion config inside multi-line shell commands#796

Merged
jackwener merged 3 commits intojackwener:mainfrom
kaichen:fix/zshrc-multiline-insertion
Apr 5, 2026
Merged

fix: avoid inserting completion config inside multi-line shell commands#796
jackwener merged 3 commits intojackwener:mainfrom
kaichen:fix/zshrc-multiline-insertion

Conversation

@kaichen
Copy link
Copy Markdown
Contributor

@kaichen kaichen commented Apr 5, 2026

Problem

postinstall.js 中的 ensureZshFpath() 在用户 .zshrc 中插入 completion 配置时,会在多行命令块内部错误插入,导致 shell 配置语法损坏、zsh 启动失败。

Root Cause

插入逻辑逐行扫描 .zshrc,查找第一个匹配 /compinit/ 的非注释行,然后 lines.splice() 在该行之前插入 fpath=(...) 配置。但它完全没有考虑 shell 反斜杠续行\)——当匹配行位于一个多行命令块内部时,插入会将该命令劈成两半。

Reproduce

用户 .zshrc 包含 zinit 多行配置(zicompinit 匹配了 /compinit/):

zinit wait lucid light-mode for \
    atinit"ZINIT[COMPINIT_OPTS]=-C; zicompinit; zicdreplay" \
        zdharma-continuum/fast-syntax-highlighting \
    atload"_zsh_autosuggest_start" \
        zsh-users/zsh-autosuggestions \
    blockf atpull'zinit creinstall -q .' \
        zsh-users/zsh-completions

运行 npm install -g opencli 后,.zshrc 变为:

zinit wait lucid light-mode for \
# opencli completion                          ← ❌ 插在了续行块中间
fpath=(/Users/xxx/.zsh/completions $fpath)    ← ❌
    atinit"ZINIT[COMPINIT_OPTS]=-C; zicompinit; zicdreplay" \
        zdharma-continuum/fast-syntax-highlighting \
    ...

这导致 zsh 启动时 zinit 命令语法错误。

Fix

用末尾追加(appendFileSync)替代插入点查找(splice),与 bash 的处理策略对齐。

之前的 ensureZshFpath() 扫描 .zshrc 寻找 compinit 行再 splice 插入——这对任何多行命令块都不安全(反斜杠续行、heredoc 等)。新方案直接追加到文件末尾,同时检测是否已有 compinit 调用以避免重复添加。

三个 shell 的策略现在统一为:

Shell completion 文件 改 rc 文件 方式
zsh ~/.zsh/completions/_opencli .zshrc 末尾追加 (was: splice)
bash ~/.bash_completion.d/opencli .bashrc 末尾追加 (unchanged)
fish ~/.config/fish/completions/opencli.fish 不改

Test plan

  • 包含 zinit 多行块(含 zicompinit)的 .zshrc 不会被劈开
  • completion 配置追加到 .zshrc 末尾
  • 检测到已有 compinit 时不重复添加 autoload -Uz compinit && compinit
  • zsh -n ~/.zshrc 语法检查通过
  • 无 compinit 的 .zshrc 追加时包含 compinit 行
  • oh-my-zsh 用户场景验证

kaichen added 2 commits April 5, 2026 15:45
The postinstall zshrc insertion logic splits backslash-continued blocks
(e.g. zinit stanzas) when it finds a compinit match inside them, which
breaks the user's shell config. Walk backward past continuation lines
so the insertion lands before the entire logical command.
Replace the fragile compinit-searching splice logic with a simple
append, matching the strategy already used for bash. This avoids
breaking multi-line commands (e.g. zinit blocks with zicompinit).

Still detects existing compinit to avoid adding a duplicate call.
Copy link
Copy Markdown
Owner

@jackwener jackwener left a comment

Choose a reason for hiding this comment

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

Review: PR #796

Overall: 代码改动清晰,问题定位准确。用 append 替代 splice 确实从根本上解决了多行命令块被劈开的问题,且与 bash 策略统一,简化了代码。

✅ 优点

  1. Root cause 分析准确——/compinit/ 正则匹配到 zicompinit 导致在续行块中间插入
  2. Append 方案简单可靠,不需要处理续行、heredoc 等各种 shell 语法边界情况
  3. 仍保留了 compinit 存在性检测,避免重复添加

⚠️ 需要确认的一个语义变化

旧逻辑将 fpath=(...) 插入到 compinit 之前,这是有意义的——compinit 只在调用时扫描 $fpath,之后追加的路径不会被扫描到。

新逻辑追加到文件末尾,对 oh-my-zsh 用户意味着:

  • source $ZSH/oh-my-zsh.sh 先执行(其中调用了 compinit)
  • 然后才设置 fpath=(...)
  • 此时 compinit 已经执行过了,新的 fpath 不会被扫描

实际影响:oh-my-zsh 用户安装后需要手动 exec zsh 或重启终端,completions 才能生效。但因为 hasCompinit=true 时不会追加 autoload -Uz compinit && compinit,所以第二次 shell 启动时 oh-my-zsh 的 compinit 仍然只会在 fpath 设置之前执行。

建议:当 hasCompinit=true 时,在 fpath 之后也追加一行 compinit 调用,确保追加位置的 fpath 能被扫描到:

const addition = hasCompinit
  ? `\n${marker}\n${fpathLine}\ncompinit\n`
  : `\n${marker}\n${fpathLine}\n${autoloadLine}\n`;

或者接受当前行为(大多数用户重启终端后就没问题,因为 oh-my-zsh 的 compinit 运行时 fpath 已经在 .zshrc 顶部通过其他方式配置好了)。

这个 trade-off 需要 maintainer 决定。如果接受当前行为,PR 可以直接合入。

小建议

  • PR 描述中 oh-my-zsh 场景验证还未打勾,建议补充测试。

Result: Approve with minor suggestion

Replace the fragile .zshrc/.bashrc modification logic with a safer
approach: only write completion files and print setup instructions.

The previous approach tried to parse and splice into rc files, which
broke multi-line shell commands (e.g. zinit blocks with backslash
continuations matching /compinit/). Instead of attempting to fix the
parser, remove rc modification entirely — this matches the approach
used by rustup, homebrew, and other CLI tools.

Closes jackwener#788
@jackwener jackwener merged commit 97a547c into jackwener:main Apr 5, 2026
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.

2 participants