Skip to content

feat: rendered-markdown preview toggle for remote (SSH) files#280

Open
TranscriptionFactory wants to merge 1 commit into
zerx-lab:mainfrom
TranscriptionFactory:pr/remote-markdown-preview
Open

feat: rendered-markdown preview toggle for remote (SSH) files#280
TranscriptionFactory wants to merge 1 commit into
zerx-lab:mainfrom
TranscriptionFactory:pr/remote-markdown-preview

Conversation

@TranscriptionFactory

Copy link
Copy Markdown

What

Makes the code pane's Rendered | Raw toggle work for remote (SSH) markdown files. Previously the toggle did nothing useful for remote buffers.

Why

Remote markdown files bypassed markdown routing:

  • The toggle detected markdown only via local_path() / tab.path, both None for remote buffers.
  • The Rendered action emitted ReplaceWithFilePaneFileNotebookView, which is local-only.

So opening a remote .md over SSH gave no rendered preview.

How (inline render within CodeView)

  • Detect markdown via the active tab's location.language_path() (works for both local and remote tabs).
  • For remote tabs, the toggle builds a read-only RichTextEditorView (Selectable) from a snapshot of the loaded buffer text via reset_with_markdown, stored per-tab, and shown in render() in place of the editor.
  • Local behavior is unchanged (still routes through RenderMarkdownReplaceWithFilePane).
  • Toggle selected-state is synced to the active tab's rendered state on tab switch.

Snapshot semantics

The preview does not auto-refresh on buffer sync; toggling Raw → Rendered re-renders. (A follow-up could subscribe to buffer changes to live-refresh.)

Testing

cargo check -p warp passes. Adds a unit test for remote markdown detection. Please run ./script/presubmit for the full app crate + clippy/fmt in CI.

Remote (SSH) markdown files bypassed markdown routing: the code pane's
Rendered|Raw toggle detected markdown only via local_path()/tab.path,
both None for remote buffers, and the Rendered action emitted
ReplaceWithFilePane -> FileNotebookView, which is local-only.

Inline-render within CodeView (Option B): detect markdown via the active
tab's location.language_path(); for remote tabs the toggle builds a
read-only RichTextEditorView (Selectable) from a snapshot of the loaded
buffer text via reset_with_markdown, stored per-tab, shown in render()
in place of the editor. Local behavior is unchanged (still routes
through RenderMarkdown -> ReplaceWithFilePane). Toggle selected-state is
synced to the active tab's rendered state on tab switch.

Snapshot semantics: preview does not auto-refresh on buffer sync; toggle
Raw->Rendered re-renders. Adds a unit test for remote markdown detection.

Next: optionally subscribe to buffer changes to live-refresh the preview.
(cherry picked from commit afb91f5)

@zerx-lab zerx-lab 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.

(详见 inline comments 和下方总结 comment)

— 由 Claude Routine 自动生成;如需复评请 @ 维护者人工触发。


Generated by Claude Code

Comment thread app/src/code/view.rs
// machinery. `RichTextEditorView` is referenced by the always-present `TabData` field, so
// it must be in scope unconditionally; the construction helpers are only used on the
// `local_fs` toggle path.
use crate::notebooks::editor::view::RichTextEditorView;

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.

Question: 非 local_fs 构建下 RichTextEditorView 是否可用?

这条 use 没有 #[cfg(feature = "local_fs")] 守卫,注释声称「必须无条件在作用域内」(因为 TabData::rendered_markdown_view 字段持有它)。但如果 RichTextEditorView 本身只在 local_fs feature 下存在,非 local_fs 构建就会报 unresolved import。

请确认:crate::notebooks::editor::view::RichTextEditorView 在没有 local_fs 时是否也会被编译进来?如果不是,rendered_markdown_view 字段和这条 use 都需要加 #[cfg(feature = "local_fs")] 守卫(render 路径里的 if let Some(rendered) 分支也同步门控即可,因为字段在非 local_fs 下不存在)。


Generated by Claude Code

));
assert!(is_markdown_file(
remote("/home/user/doc.markdown").language_path()
));

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.

Question: CHANGELOG(无扩展名)是否会被 is_markdown_file 识别为 Markdown?

CHANGELOG 文件通常以多种格式存在(纯文本、Markdown、rst 等),没有统一的扩展名。is_markdown_file 函数是否明确地将无扩展名的 CHANGELOG 作为 Markdown 处理?如果它的实现只匹配 .md / .markdown 扩展名,这个断言会 panic 并导致 CI 失败。

建议先确认 is_markdown_file 对无扩展名文件的行为,如果它不支持,就把这条断言改为 assert!(!is_markdown_file(...)) 或移除。


Generated by Claude Code

Copy link
Copy Markdown
Owner

Review 总结

整体判断:Comment(需确认两处问题后可合并)。核心思路可行,在 CodeView 内联复用 RichTextEditorView 渲染远端 Markdown 的方案合理;本地 Markdown 路径不变,不引入回归。


阻塞问题

  1. local_fs 构建下的编译问题(见 view.rs inline comment):use crate::notebooks::editor::view::RichTextEditorView 没有 #[cfg(feature = "local_fs")] 守卫,但 TabData::rendered_markdown_view 字段也未门控。若 RichTextEditorView 只在 local_fs feature 下存在,非 local_fs 构建会报 unresolved import。需要确认该类型在所有构建目标下是否可用,若不是,字段和 import 都需要加 cfg 门控。

建议

  • CHANGELOG 测试断言(见 buffer_location.rs inline comment):CHANGELOG(无扩展名)是否会被 is_markdown_file 识别为 Markdown 取决于函数的实现。若它只匹配 .md / .markdown 扩展名,此断言会在 CI 中 panic。建议先验证函数行为再确定断言方向。

看起来不错的地方

  • 本地 Markdown 的 ReplaceWithFilePane 路径完全不变,无回归风险
  • rendered_markdown_view 按 tab 独立存储,切换 tab 时分段控件选中态正确同步
  • 快照语义(preview 不随 buffer sync 刷新)已在 PR 描述中明确说明,MVP 边界清晰
  • CurrentRenderWindowGuard 模式(来自 PR feat: per-window themes ("This window" theme scope) #279)在 set_remote_markdown_rendered 中不需要,因为 RichTextEditorView 构造用的是 ctx.add_typed_action_view,已在当前 window context 下

— 由 Claude Routine 自动生成;如需复评请 @ 维护者人工触发。


Generated by Claude Code

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