reya/signed
1
0
Fork 0
Code Issues Pull Requests Actions 8 Releases Activity
Files
26653fa1245180b868d2afd9155056edb4020249
signed/docs/repo-view-refactor-plan.md
T
reya 33e9429cd0 update
2026-09-13 15:17:51 +07:00

16 KiB
Raw Blame History

Repo view refactor plan

Goal

Make crates/workspace/src/views/repo/ easy to navigate and change:

  • Each concern owns its own state (its own struct fields), instead of all concerns sharing one 38-field struct.
  • Shared UI moves to the module that consumes it, so sibling views stop importing from views::repo.
  • No behavior change. No new global state. No new store. The GPUI patterns already used in the repo (Entity + observe, TreeState, ComboboxState, VirtualListScrollHandle) stay the only patterns used.

Constraints

  • Follow .rules: no unwrap in production, no silently discarded errors, full-word names, comments explain "why" only.
  • Do not over-engineer. Files and History become entities because they already render and run async independently. Refs and Banners stay plain field groups on the shell.
  • Keep the shell as the single load/reconcile point. The clone/worktree and ref_generation belong to the shell, not to child views.
  • signed_ui does not depend on signed_git (verified in crates/signed_ui/Cargo.toml). Anything that takes a signed_git type cannot move there.

Current state (verified)

File Lines Content
mod.rs ~376 RepoDetailView struct (38 fields), constructors, render, panel impls, display_name
store.rs ~98 attach_store, apply_announcement, refresh_ready_statuses, refresh_statuses
actions.rs ~208 action methods + open_repo_panel / open_repo_item free functions
loading.rs ~435 load_repo, apply_repo_data, sync_ref_selector, clone_to_folder, load_repo_data
refs.rs ~279 switch_ref, restore_selection, reload_worktree, catch_up_worktree
files.rs ~480 file tree, previews, markdown/code state, eviction
history.rs ~228 commits tab render + per-file and full commit walks
header.rs ~715 header render, maintainers, fork row, clone URL
banners.rs ~315 ready/push suggestion banners
about.rs ~224 about dialog
init_dialog.rs ~202 publish-to-NIP-34 dialog
helpers.rs ~722 pub(crate) grab bag: tree building, diff rendering, discussion UI, share targets, commit rows

Problems

  1. RepoDetailView is a god object. 38 fields across six concerns. All 12 files are impl RepoDetailView, so any file can read/write any field. The file split added navigation cost without encapsulation.
  2. helpers.rs is an inverted dependency hub. views/issues/detail.rs and views/pull_requests/*.rs import from views::repo::helpers for discussion UI, diff rows and commit rows. Sibling views reaching into repo is backwards.
  3. Two regions have independent async + render lifecycles (file browser, commit history) but live as shell fields, sharing worktree and ref_generation by hand.

Existing good pattern

IssuesView (views/issues/mod.rs): own struct (~13 fields), cx.observe(&store, ..), rebuild() into local state, Render, no shell fields. The refactor brings RepoDetailView in line with this.

Target structure

graph TD
    Shell["RepoDetailView shell\nstore, dock_area, tabs, header,\nload orchestration, worktree, generation"] --> Files["Entity<RepoFilesView>\nfiles.rs"]
    Shell --> History["Entity<RepoHistoryView>\nhistory.rs"]
    Shell --> Refs["RefSwitcher (plain)\nrefs.rs"]
    Shell --> Banners["Banners (plain)\nbanners.rs"]
    Files --> Store["Entity<RepoStore>"]
    History --> Store

Field ownership after the refactor:

Concern Fields Owner
Files tree_state, worktree_paths, md, code, readme_name, selected_file, files, file_order, preview_bytes, loading_files, commits, pending_commits, loading_commits RepoFilesView
History all_commits, loading_all_commits, item_sizes, scroll_handle RepoHistoryView
Refs branch_select, tag_select, ref_branches, ref_tags, switching_ref RefSwitcher
Banners banner_dismissed, ready_requested, ready_head, ready_statuses, push_statuses Banners
Shell focus_handle, dock_area, store, repo_started, active_tab, loading, error, head_commit, worktree, ref_generation, _subscriptions RepoDetailView (11 fields)

Shared modules after Phase 1:

New / changed module Contents Consumers
views/tree.rs TreeItemSeed, tree_items, build_tree_items, sorted_worktree_paths + the 3 tree tests repo files/loading, commit_diff
views/commit_diff/mod.rs adds DiffRow, diff_rows, render_diff_row, render_diff_line, find_item, GUTTER_WIDTH, DIFF_ROW_HEIGHT, commit_row, COMMIT_ROW_HEIGHT commit_diff, PR new, repo history
views/discussion.rs sidebar_title, sidebar_section, comments_section, comment_form, issue_roots, pr_roots issues detail, PR detail
signed_ui/src/ref_selector.rs ref_selector_trigger repo header, PR new
repo/files.rs code_language, is_markdown_path (only used there) repo files
repo/header.rs ShareTargets, truncate_naddr_link (only used there) repo header

views/repo/helpers.rs is deleted at the end of Phase 1.

Phase 0 - baseline

No code. Record the current state so each later phase can be compared.

  1. cargo fmt --all -- --check
  2. cargo check --offline --workspace --all-targets
  3. cargo test --offline -p workspace
  4. cargo clippy --offline -p workspace --all-targets

Do not run plain cargo without --offline; the sandbox fails the git fetch and it looks like a dependency error.

Phase 1 - extract shared modules (dissolve helpers.rs)

Low risk, no state moves. Land it as one commit.

1.1 Create crates/workspace/src/views/tree.rs

Move from repo/helpers.rs: TreeItemSeed, tree_items, build_tree_items, sorted_worktree_paths, and the three tests (builds_nested_tree_from_flat_entries, tree_builder_handles_deep_nesting, tree_builder_merges_shared_prefixes).

  • Add pub(crate) mod tree; to views/mod.rs.
  • Update imports in repo/loading.rs, repo/refs.rs, commit_diff/mod.rs to crate::views::tree::....

1.2 Move diff and commit-row rendering into views/commit_diff/mod.rs

Move from repo/helpers.rs: GUTTER_WIDTH, DIFF_ROW_HEIGHT, DiffRow, diff_rows, render_diff_row, render_diff_line, find_item, COMMIT_ROW_HEIGHT, commit_row.

  • commit_diff/mod.rs already owns DiffPane and depends on signed_git, so this is its natural home and keeps signed_ui free of a signed_git dependency.
  • Update imports in views/pull_requests/new.rs and repo/history.rs.

1.3 Create crates/workspace/src/views/discussion.rs

Move from repo/helpers.rs: sidebar_title, sidebar_section, comments_section, comment_form, issue_roots, pr_roots.

  • Add pub(crate) mod discussion; to views/mod.rs.
  • Update imports in views/issues/detail.rs and views/pull_requests/detail.rs. After this, neither imports from views::repo.

1.4 Move ref_selector_trigger into signed_ui

It takes CustomIconName (from assets) and ComboboxTriggerContext (from gpui_component); both are already signed_ui dependencies, so no dependency changes.

  • Add crates/signed_ui/src/ref_selector.rs, export it from lib.rs.
  • Update imports in repo/header.rs and views/pull_requests/new.rs.

1.5 Move code_language and is_markdown_path into repo/files.rs

Only repo/files.rs uses them. Keep them private there.

1.6 Move ShareTargets and truncate_naddr_link into repo/header.rs

Only repo/header.rs uses them. Keep them private there.

1.7 Delete repo/helpers.rs

Remove pub(super) mod helpers; from repo/mod.rs. Confirm no use ...repo::helpers remains anywhere:

grep -rn "repo::helpers" crates/workspace/src

Phase 1 validation

cargo fmt --all, cargo check --offline -p workspace --all-targets, cargo test --offline -p workspace, cargo clippy --offline -p workspace --all-targets.

Phase 2 - extract Entity<RepoFilesView>

Largest win: removes 14 fields and most of the preview logic from the shell.

2.1 Define the view

In repo/files.rs, replace impl RepoDetailView with pub(super) struct RepoFilesView holding: tree_state, worktree, worktree_paths, md, code, readme_name, selected_file, files, file_order, preview_bytes, loading_files, commits, pending_commits, loading_commits.

Move the supporting types and helpers from the current files.rs into the view: FileContent, MarkdownView, CodeView, MAX_PREVIEW_BYTES, MAX_PREVIEWED_FILES, MAX_PREVIEW_CACHE_BYTES, source_hash, preview_spinner, render_tree_item, render_tree_column, render_content_column, set_markdown, markdown_element, set_code, code_element, open_file, drop_preview_of, evict_previews.

Move from repo/history.rs: load_commit, load_commits (the per-file commit map).

2.2 Define the view's interface

  • pub(super) fn new(window: &mut Window, cx: &mut Context<Self>) -> Self - creates the TreeState.
  • pub(super) fn set_worktree(&mut self, path: PathBuf).
  • pub(super) fn apply_entries(&mut self, tree: Vec<TreeItemSeed>, paths: Vec<String>, window, cx) - used by load_repo / reload_worktree / catch_up_worktree.
  • pub(super) fn set_readme(&mut self, path: Option<PathBuf>, bytes: Option<Vec<u8>>, cx).
  • pub(super) fn clear_previews(&mut self) - branch switch.
  • pub(super) fn catch_up(&mut self, snapshot, window, cx) -> bool - rebuild tree, drop removed previews, re-render README; returns whether anything changed.
  • impl Render for RepoFilesView.
  • pub(super) fn pane_title(&self) -> SharedString - selected_file or readme_name or "Overview".

2.3 Move the clone loading/error display out of the file view

render_content_column currently shows "Cloning repository..." / a load error from self.loading and self.error, which are shell state. Move that decision to the shell's render: while self.loading, render a spinner in the tab body; when self.error is set, the existing Alert already covers it. render_content_column then handles only file previews and the README.

2.4 Wire the shell

  • Add files: Entity<RepoFilesView> to RepoDetailView.
  • In new_common, let files = cx.new(|cx| RepoFilesView::new(window, cx));.
  • In render, the Files tab body becomes self.files.clone().
  • In load_repo (loading.rs) and reload_worktree / catch_up_worktree (refs.rs), replace direct field writes with calls on self.files.
  • Remove the now-unused files.rs imports from mod.rs and the moved fields from the struct and constructor.

Phase 2 validation

Same commands. Manual: open explore repo, click files in the tree, open the README, switch branch (previews clear), switch back, confirm no spinner sticks.

Phase 3 - extract Entity<RepoHistoryView>

3.1 Define the view

In repo/history.rs, replace the commits-tab methods with pub(super) struct RepoHistoryView holding: store: Entity<RepoStore>, dock_area: WeakEntity<DockArea>, worktree: Option<PathBuf>, all_commits, loading_all_commits, item_sizes, scroll_handle.

Move: render_commits_tab (becomes impl Render), load_all_commits, open_commit_diff.

3.2 Display name

open_commit_diff uses the shell's display_name. Extract the display_name logic from RepoDetailView into a free function in repo/mod.rs:

pub(super) fn repo_display_name(store: &RepoStore) -> SharedString

It keeps the local-path fallback that RepoStore::name() does not have. Use it in the shell's Panel::title, in the header, and in RepoHistoryView::open_commit_diff.

3.3 Interface

  • pub(super) fn new(store, dock_area, window, cx) -> Self.
  • pub(super) fn set_worktree(&mut self, path: Option<PathBuf>).
  • pub(super) fn reload(&mut self, cx) - clears all_commits and starts the walk (called when HEAD changes or the branch switches).
  • impl Render for RepoHistoryView.

3.4 Wire the shell

  • Add history: Entity<RepoHistoryView> to RepoDetailView; create it in new_common.
  • In render, tab 1 becomes self.history.clone().
  • Replace self.all_commits / self.loading_all_commits / self.item_sizes writes in load_repo, reload_worktree, catch_up_worktree, and the header-commit pill path with self.history.update(..) calls.
  • Remove the moved fields from the struct and constructor.

Phase 3 validation

Same commands. Manual: open the Commits tab, scroll a long history, click a commit (diff panel opens), switch branch and confirm the list reloads.

Phase 4 - group RefSwitcher and Banners

Plain structs on the shell. No entity, no observer changes.

4.1 RefSwitcher

Move into a struct RefSwitcher { branch_select, tag_select, ref_branches, ref_tags, switching_ref } field on the shell. Update refs.rs and loading.rs methods to read/write self.refs.*. switch_ref stays on the shell because it fans out to files, history and head_commit.

ref_generation stays on the shell: it is shared with the files and history loads.

4.2 Banners

Move into a struct Banners { dismissed, ready_requested, ready_head, ready_statuses, push_statuses } field. banners.rs and store.rs methods keep their impl RepoDetailView shape but read/write self.banners.*.

Phase 4 validation

Same commands. Manual: the ready-to-contribute banner appears and dismisses, the push banner appears for an owned repo, dismissing survives a store refresh.

Phase 5 - fold store.rs and tidy

  1. Move attach_store, apply_announcement, refresh_ready_statuses, refresh_statuses into mod.rs and delete repo/store.rs.
  2. Remove mod store; from repo/mod.rs.
  3. Confirm mod.rs reads as a shell: struct, constructors, load coordination, render, panel impls.
  4. Final validation:
cargo fmt --all
cargo check --offline --workspace --all-targets
cargo test --offline --workspace
cargo clippy --offline --workspace --all-targets

Validation (manual smoke, after each phase)

  • Open a repo from the explore list, then open an issue and a PR.
  • Deep-link straight to an issue / PR without visiting the repo panel.
  • Open a local repository (never announced).
  • Initialize a local repo to NIP-34, confirm it leaves the sidebar's local section.
  • Clone to folder; clone again before the first clone completes.
  • Switch a branch and a tag; confirm previews and the commit list reset.
  • Owned repo with unpushed commits: push banner, push, republish banner.

Boundary test for "done"

  • No file can touch fields it does not own.
  • repo/mod.rs is a shell, roughly 200 lines.
  • grep -rn "views::repo::helpers" crates/workspace/src returns nothing.
  • views/issues and views/pull_requests have no use ...views::repo.

Non-goals

  • No behavior change; no UI redesign.
  • No new global state, no new store, no changes to signed_state or dock.
  • No more impl RepoDetailView chapters. New files own structs, not fragments of one struct.
  • Do not move commit_row into signed_ui: it takes signed_git::FileCommit and signed_ui does not depend on signed_git.

Risks and open questions

  • Async generation. ref_generation discards stale loads. It stays on the shell; when the shell pushes a snapshot into a child view, the child must not start a new load that outlives the generation. Simplest rule: only the shell starts loads, child views only render and own per-file preview fetches keyed to the current worktree.
  • Files owns the per-file commit walk. load_commit/load_commits move with the preview state, so the shell no longer coordinates them. Confirm the README commit lookup still works after the move.
  • History is small. After moving load_commit/load_commits to Files, history.rs is ~150 lines. If an entity feels heavy for that, a plain struct History field is an acceptable fallback; the field ownership still improves.
  • RepoStore::name() vs display_name. RepoStore::name() returns Unknown for local repos. The extracted repo_display_name must keep the local-path fallback so titles are unchanged.
Reference in New Issue View Git Blame Copy Permalink