Skip to content

fix: add Focused / Default / Ask split shell source - #484

Open
VauntlekV wants to merge 19 commits into
Kuddev:mainfrom
VauntlekV:fix/split-focused-shell-picker
Open

VauntlekV wants to merge 19 commits into
Kuddev:mainfrom
VauntlekV:fix/split-focused-shell-picker

Conversation

@VauntlekV

@VauntlekV VauntlekV commented Oct 5, 2026 •

Copy link
Copy Markdown

Advertising, commercial promotion, and traffic solicitation are prohibited in PRs
and Issues. Business requests must go through the email in the
contribution policy.
PRs or Issues containing unauthorized promotion will be closed directly.
严禁利用 PR 或 Issue 推广、引流,未经授权的推广将直接关闭;商业需求请通过贡献指南中的邮箱沟通,已获批准的赞助按批准范围处理。

Result / 用户结果

Addresses the split-inheritance part of #372 with split_shell_source:

  • Default (factory default): configured shell/profile, inheriting only a host-visible directory.
  • Focused: focused pane’s Shell/Profile/SSH identity and live directory.
  • Ask: existing launcher; Escape cancels without creating a pane.

The setting applies to subsequent interactive requests. Ask captures the source pane and direction; closing the source never retargets the split.

Design / 设计边界

  • Responsibility and affected modules: nebula_settings owns values and persistence; workspace/splitting.rs resolves launch snapshots; settings reuse the shared capsule/help components.
  • Why this belongs here; interfaces that remain unchanged: reuse pane-origin and launch-copy rules adapted from fix(wsl): pin each pane's distro, keep guest cwd in splits/tabs/forks, pass guest paths safely #351. Runtime API splits remain immediate Focused splits; full-layout duplication is unchanged.
  • Dependency, data-format, threading, or lifetime changes (ADR if applicable): one additive settings key, no dependency or threading change. The source policy is recorded in architecture/notes/nebula_settings/split_shell_source/2026-10-06-three-way-source.md.
  • Compatibility and migration/fallback behavior: missing or invalid values select Default. Existing guest/user and same-host SSH directory boundaries remain in force.

Evidence / 验证依据

  • Commands and actual results (include unrun checks): local Linux cargo test -p nebula-settings --locked split_shell_source and cargo test -p nebula --bin pebrel --features gpui-test-support --locked gpui_shell::workspace::splitting:: passed. i18n contracts, baseline-relative architecture checks, CI-config contracts, cargo fmt --all -- --check and git diff --check also passed. Full native tests were not rerun locally; current CI is on the Checks page.
  • Regression tests: what fails before the fix? Focused identity assertions catch splits opening the default shell instead of inheriting the source. Pure tests cover source × mode, target directories, closed sources and captured direction; two GPUI smoke tests cover live shortcuts and Ask cancellation.
  • UI changes: screenshots, long translations, keyboard access, DPI checks: shared controls are reused; shortcut and Escape paths were tested. No new screenshots, large-font/narrow-window/DPI checks or Windows/macOS product acceptance were run for this revision.
  • Hot-path changes: allocation/work/load measurements, where applicable: not applicable; changes are limited to split creation and settings UI.

Required Review / 必须确认

  • I followed CONTRIBUTING.md, docs/architecture.md, and docs/project-constraints.md.
  • I split responsibilities, not arbitrary line ranges; no duplicate behavior authority was added.
  • python3 scripts/check_architecture.py --base <PR-base-commit> passes; budgets were not inflated to fit the change.
  • Tests cover success and failure; platform/feature coverage limitations are stated.
  • New messages use typed i18n IDs and matching placeholders; untranslated content has an explicit fallback.
  • Governance changes include a counterexample, corrected contract, tests, and a maintainer-reviewed decision.

The governance item is not applicable: this PR does not change governance rules.

Checkboxes explain the review; they do not replace CI or maintainer approval.

Preserve focused Shell/Profile/SSH launch identity instead of selecting
the terminal default again. Capture pane and direction while the shared
picker is open; cancellation or source closure never retargets a split.

Add the opt-in split_shell_picker preference with immediate application,
compatible persistence/reset handling and native GPUI regressions.
Runtime API split operations remain noninteractive.

Adapt WSL spawn snapshot and argv-safety rules from MomentDerek PR Kuddev#351
(c7089aa). Keep existing new-tab,
AI-fork and single-pane duplication policies; layout PR Kuddev#455 is excluded.

Addresses the split-inheritance part of Kuddev#372.

Co-authored-by: MomentDerek <40252940+MomentDerek@users.noreply.github.com>
@VauntlekV
VauntlekV requested a review from Kuddev as a code owner October 5, 2026 09:19

@Kuddev Kuddev left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

当前 head 的新增验收路径在上游 native CI 中失败,暂不批准合并:

  • Linux: gpui_shell::settings_pane::split_shell_tests::split_shell_picker_is_searchable_clickable_live_and_resettable 在 nebula_app/src/gpui_shell/settings_pane/split_shell_tests.rs:39 失败,断言 RuntimeSettings::load().split_shell_picker 为真未成立。
  • Windows x64: gpui_shell::workspace::splitting::tests::both_split_shortcuts_inherit_the_focused_pane_not_the_default_or_tab_identity 在 nebula_app/src/gpui_shell/workspace/splitting/tests.rs:151 失败,断言 !w.command_palette_open 未成立。

日志:https://github.com/Kuddev/pebrel/actions/runs/37289266644

这两项都位于本 PR 新增的真实交互回归,分别对应开关生效和默认无选择器分屏合同。请定位产品交互或 fixture 隔离的实际原因,并保留原验收断言完成当前 head 的原生验证;本轮没有凭日志猜测根因,也没有通过删测试/放宽断言代替修复。此为 CI 阻塞核查,不代表其余 34 文件已完成全面审阅。

Join the existing theme-studio fixture group for both new rendered test
modules. Their process-local mutex cannot protect the shared settings file
when nextest runs each case in a separate process.

Preserve all cases and assertions, default suite parallelism, zero retries
and the existing fixture policy. Update the exact registration contract
and record the reproducible before/after failure evidence for PR Kuddev#484.
Preserve upstream layout duplication and SSH remote cwd together with WSL guest-path protection. Validate the merged native suite without removing assertions or adding retries.
@VauntlekV VauntlekV closed this Oct 5, 2026
@VauntlekV VauntlekV reopened this Oct 5, 2026
@Kuddev

Kuddev commented Oct 5, 2026

Copy link
Copy Markdown
Owner

依赖的 #351 已合入 main(c1c499d3)。请先同步最新 main,让本 PR 的差异收敛到分屏继承与选择器本身,并保留已经合入的逐窗格发行版、目录及完整分屏复制逻辑。不要用旧堆叠版本覆盖主线修正。

当前 head 已更新,先前记录的 Linux/Windows 两项失败不再直接当作当前提交仍有缺陷的证据。请在同步后的精确 head 提供选择器开关持久化、快捷键打开/取消/选择、未开启选择器时直接继承聚焦 Shell 的测试结果,以及涉及测试隔离配置调整的原因;保留业务断言,不通过放宽断言消除失败。

按本轮小修复优先的安排,这项由作者完成同步和必要修改后复审。

“vauntlek” added 3 commits October 5, 2026 09:36
Keep upstream WSL startup-versus-exec quoting boundaries, pane distribution snapshots, configured startup directories and full-layout duplication. Integrate focused-shell split inheritance and the optional picker without dropping Duplicate/NewTab policies or their assertions.
@MomentDerek

Copy link
Copy Markdown
Contributor

关于分屏 Shell 的行为,提一个设计上的建议,供作者和维护者参考:

现在这个 PR 把分屏直接改成继承焦点窗格的启动身份,另外加了一个布尔开关 split_shell_picker 控制是否弹选择器。这样一来,"分屏总是用设置里的默认 Shell"这种现有行为就没法再选了。有些用户是有意依赖它的,比如在 WSL / SSH 窗格里分出一个本机 PowerShell 来跑宿主侧命令。

建议把这两件事合并成一个枚举设置,比如 split_shell_source:

取值 行为
focused 继承焦点窗格的 Shell/Profile/SSH 身份(即本 PR 的默认行为,WSL 沿用 #351 的发行版固定和 cwd 规则)
default 使用设置中的默认 Shell,只继承宿主机工作目录(即当前 main 的行为)
ask 每次弹出现有的 launcher 选择器(即本 PR 中 split_shell_picker = true 的行为)

这样做的好处:

  • 三种行为互斥,用一个枚举表达,比"布尔开关 + 隐含默认"更不容易出现组合歧义,设置页也只需要一个下拉;
  • 当前行为保留为可选项,升级后不想改变习惯的用户可以切回去;
  • 默认值选 focused 还是 default 可以由维护者定,迁移/重置/往返测试的写法和现在的 split_shell_picker 基本一样。

如果作者同意这个方向,我也可以在作者同步 main 后协助调整设置模型和测试。

@Kuddev

Kuddev commented Oct 5, 2026

Copy link
Copy Markdown
Owner

补充审阅要求:请保留“使用设置中的默认 Shell”这一现有选择,避免本次修复让需要从 WSL/SSH 分出本机 Shell 的用法消失。用一个明确的来源选项表达“跟随焦点 / 使用默认 / 每次选择”可以讨论,但默认值、行为变化和迁移需要单独说明,不在 bug 修复中隐式替换原有默认行为。请作者同步 main 后连同三条实际入口的验证一起提交;本轮继续按较大问题由作者调整的安排处理。

“vauntlek” added 2 commits October 5, 2026 18:54
Default to the configured shell with host-only cwd inheritance. Remove the old picker boolean from the model, parser, runtime cache, UI and reset contract without migration or fallback. Keep focused identity rules and the captured ask launcher lifecycle. Update regression source without running CI; build the Windows product for manual acceptance.
Persist a discoverable non-executable profile instead of accidentally launching the real platform default. Assert its exact identity and refocus the original distinct pane for both shortcuts. Restore the profile store with the existing shared settings guard. Keep production behavior, all business assertions, fixture-group isolation and zero retries unchanged. Record the first fork CI failure; seven selected native Windows regressions pass.
@VauntlekV VauntlekV changed the title fix: inherit focused shell for splits with an optional picker fix: add Focused / Default / Ask split shell source Oct 6, 2026
@VauntlekV

Copy link
Copy Markdown
Author

感谢 @Kuddev 和 @MomentDerek 的建议,本轮已更新为 Focused / Default / Ask 三项来源选择。

缺省为 Default,保留使用配置默认 Shell 的选择;旧布尔设置已完整移除,不读取、不迁移、不回退。

Focused 和 Ask 继续保留既有身份、目录及捕获来源的保护逻辑。

当前 head 为 610986f,五平台原生测试、架构和 PR 大小检查均已通过,没有删测试、放宽断言或增加重试。我也已在 Windows 上运行实际 GPUI exe,人工进行页面操作核验,未发现问题。

具体行为、测试结果和验证边界已补充到 PR 描述中。

@VauntlekV
VauntlekV requested a review from Kuddev October 6, 2026 18:20

@Kuddev Kuddev left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

已重新检查更新后的 head,而非沿用旧版本的 CI 结论:本轮三来源模型确实保留了 Default,Focused/Ask 复用了现有启动身份与来源窗格捕获;当前上游五项实际 native jobs 和十项必需检查均成功,旧 review 中两项 CI 失败不再作为这个 head 的依据。

继续整合前请处理与最新 main 的一致性:

  1. 同步当前 main 并保留 #504 的输入修复、#505 的短选项胶囊及按真实文本宽度布局。当前新增“跟随焦点 / 使用默认 / 每次选择”仍只走 select_row,未加入 segmented::supports;split_shell_tests 还断言固定宽度 dropdown。请将这组三个短选项接入现有胶囊组件,保留键盘选择、保存失败回退和恢复默认,而不是再造一套控件。
  2. PR 正文验收 head 仍写 610986f,实际为 024d41f,请对应更新,区分 fork 验证、上游 CI 与实际产品操作验证。
  3. 保存三模式的现有分屏回归,补充同步后实际 WSL/SSH 使用中的身份和目录验收记录;现有窗口测试使用不可执行的本地 shell fixture,不应被表述为已验证真实认证连接或所有 guest/user 组合。

本轮已检查 split_at、copy_launch、default_split_launch、选择器完成/取消与设置路径,未新增代码依赖。这个 PR 涉及多入口及设置行为,继续由作者整合,不与小型 bug 修复一起直接合并;也不据此关闭整个 #372。

@VauntlekV
VauntlekV requested a review from Kuddev October 7, 2026 09:22
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