clean up

crates/workspace/src/views/repo/files.rs

@@ -59,7 +59,6 @@ pub(super) struct RepoFilesView {
     commits: HashMap<String, FileCommit>,
     pending_commits: Vec<String>,
     loading_commits: bool,
-    generation: u64,
     tasks: Vec<Task<Result<(), Error>>>,
 }
 
@@ -80,7 +79,6 @@ impl RepoFilesView {
             commits: HashMap::new(),
             pending_commits: Vec::new(),
             loading_commits: false,
-            generation: 0,
             tasks: Vec::new(),
         }
     }
@@ -142,7 +140,6 @@ impl RepoFilesView {
         self.md = None;
         self.code = None;
         self.readme_name = None;
-        self.generation += 1;
     }
 
     /// Refresh after the mirror caught up with the remote.
@@ -466,7 +463,6 @@ impl RepoFilesView {
         let path = path.to_string();
 
         self.load_commit(&path, cx);
-        let generation = self.generation;
 
         let task: Task<Result<(), Error>> = cx.spawn_in(window, async move |this, cx| {
             let path_for_read = path.clone();
@@ -496,11 +492,6 @@ impl RepoFilesView {
                 .await;
 
             this.update_in(cx, |this, window, cx| {
-                if generation != this.generation {
-                    this.loading_files.remove(&path);
-                    return;
-                }
-
                 this.loading_files.remove(&path);
 
                 match content {
@@ -616,7 +607,6 @@ impl RepoFilesView {
         self.loading_commits = true;
 
         let paths = std::mem::take(&mut self.pending_commits);
-        let generation = self.generation;
 
         let task: Task<Result<(), Error>> = cx.spawn(async move |this, cx| {
             let rels: Vec<PathBuf> = paths.iter().map(PathBuf::from).collect();
@@ -629,9 +619,7 @@ impl RepoFilesView {
             this.update(cx, |this, cx| {
                 this.loading_commits = false;
 
-                if generation == this.generation
-                    && let Ok(found) = result
-                {
+                if let Ok(found) = result {
                     for (path, commit) in found {
                         this.commits
                             .insert(path.to_string_lossy().into_owned(), commit);

crates/workspace/src/views/repo/history.rs

@@ -23,8 +23,6 @@ pub(super) struct RepoHistoryView {
     loading_all_commits: bool,
     scroll_handle: VirtualListScrollHandle,
     item_sizes: Rc<Vec<Size<Pixels>>>,
-    /// Bumped on reload, so an in-flight walk of the previous HEAD is discarded.
-    generation: u64,
     tasks: Vec<Task<Result<(), Error>>>,
 }
 
@@ -38,7 +36,6 @@ impl RepoHistoryView {
             loading_all_commits: false,
             scroll_handle: VirtualListScrollHandle::new(),
             item_sizes: Rc::new(Vec::new()),
-            generation: 0,
             tasks: Vec::new(),
         }
     }
@@ -54,7 +51,6 @@ impl RepoHistoryView {
 
     /// Drop the current list and walk HEAD again.
     pub(super) fn reload(&mut self, cx: &mut Context<Self>) {
-        self.generation += 1;
         self.all_commits = None;
         self.loading_all_commits = false;
         self.load(cx);
@@ -70,7 +66,6 @@ impl RepoHistoryView {
         };
 
         self.loading_all_commits = true;
-        let generation = self.generation;
 
         let task: Task<Result<(), Error>> = cx.spawn(async move |this, cx| {
             let result = cx
@@ -78,10 +73,6 @@ impl RepoHistoryView {
                 .await;
 
             this.update(cx, |this, cx| {
-                if generation != this.generation {
-                    return;
-                }
-
                 if let Ok(list) = result {
                     let count = list.commits.len();
                     this.item_sizes = Rc::new(vec![size(px(0.), px(COMMIT_ROW_HEIGHT)); count]);

crates/workspace/src/views/repo/mod.rs

@@ -91,7 +91,6 @@ pub struct RepoDetailView {
     error: Option<SharedString>,
     head_commit: Option<FileCommit>,
     refs: RefSwitcher,
-    ref_generation: u64,
     banners: Banners,
     tasks: Vec<Task<Result<(), Error>>>,
     _subscriptions: Vec<Subscription>,
@@ -191,7 +190,6 @@ impl RepoDetailView {
             error: None,
             head_commit: None,
             refs,
-            ref_generation: 0,
             tasks: Vec::new(),
             banners: Banners::default(),
             focus_handle: cx.focus_handle(),
@@ -342,16 +340,13 @@ impl RepoDetailView {
         let Some(announcement) = announcement else {
             return;
         };
+
         self.repo_started = true;
 
         let cache = GitStore::global(cx).cache().clone();
         let addr = announcement.addr();
         let clone_urls: Vec<Url> = announcement.clone.clone();
 
-        // Captured before the loads start.
-        // A branch/tag switch bumps the generation, discarding the refresh below.
-        let refresh_generation = self.ref_generation;
-
         let disk = {
             let cache = cache.clone();
             let addr = addr.clone();
@@ -363,7 +358,7 @@ impl RepoDetailView {
             })
         };
 
-        let task: gpui::Task<Result<(), Error>> = cx.spawn_in(window, async move |this, cx| {
+        let task: Task<Result<(), Error>> = cx.spawn_in(window, async move |this, cx| {
             let disk = disk.await;
             let had_clone = matches!(&disk, Ok(Some(_)));
 
@@ -393,7 +388,6 @@ impl RepoDetailView {
 
             // Refresh the clone from the network in the background.
             // When it completes, update the refs and commit list.
-            // Loads started before a branch/tag switch are discarded via the generation.
             if !had_clone {
                 return Ok(());
             }
@@ -408,15 +402,10 @@ impl RepoDetailView {
                     };
 
                     // Best-effort, a fetch failure, e.g. offline, keeps the cached state.
-                    // The state is already shown.
                     signed_git::fetch_all(&repo).ok();
 
                     let worktree = repo.workdir().map(Path::to_path_buf);
-                    // A fetch never moves a mirror's local branches.
-                    // A push landing on the grasp servers would never show up.
-                    // That covers own repo pushes from a checkout and updates fetched here.
-                    // Fast-forward branches from the remote, like `git pull --ff-only`.
-                    // Only the checked-out branch's worktree can change on disk.
+
                     let moved = match &worktree {
                         Some(worktree) => {
                             signed_git::fast_forward_branches(worktree).unwrap_or(false)
@@ -441,10 +430,6 @@ impl RepoDetailView {
             .await;
 
             this.update_in(cx, |this, window, cx| {
-                if refresh_generation != this.ref_generation {
-                    return;
-                }
-
                 if let Ok(Some((moved, branches, tags, current_branch, head_commit))) = refresh {
                     let branches: Vec<SharedString> = branches.iter().map(Into::into).collect();
                     let tags: Vec<SharedString> = tags.iter().map(Into::into).collect();
@@ -619,7 +604,6 @@ impl RepoDetailView {
         }
 
         self.refs.switching_ref = true;
-        self.ref_generation += 1;
         cx.notify();
 
         let checkout_name = name.clone();

docs/repo-view-refactor-plan.md

@@ -1,291 +0,0 @@
-# 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
-
-```mermaid
-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`:
-
-```rust
-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.