From 12dbbdd1d339d6de7be57015f4210127a017f2fe Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Fri, 19 Dec 2025 18:55:17 -0500 Subject: [PATCH] git: Fix bug where opening a git blob from historic commit view could fail (#44226) The failure would happen if the current version of the file was open as an editor. This happened because the git blob and current version of the buffer would have the same `ProjectPath`. The fix was adding a new `DiskState::Historic` variant to represent buffers that are past versions of a file (usually a snapshot from version control). Historic buffers don't return a `ProjectPath` because the file isn't real, thus there isn't and shouldn't be a `ProjectPath` to it. (At least with the current way we represent a project path) I also change the display name to use the local OS's path style instead of being hardcoded to Posix, and cleaned up some code too. Release Notes: - N/A --------- Co-authored-by: Cole Miller Co-authored-by: cameron Co-authored-by: xipengjin --- crates/action_log/src/action_log.rs | 8 +- crates/agent_ui/src/agent_diff.rs | 4 +- crates/editor/src/items.rs | 6 +- crates/git_ui/src/commit_view.rs | 54 +----- crates/image_viewer/src/image_viewer.rs | 4 +- crates/language/src/buffer.rs | 15 ++ crates/multi_buffer/src/multi_buffer.rs | 8 +- .../project/src/debugger/breakpoint_store.rs | 4 +- crates/project/src/project.rs | 6 +- crates/proto/proto/worktree.proto | 161 +++++++++--------- crates/worktree/src/worktree.rs | 9 +- 11 files changed, 130 insertions(+), 149 deletions(-) diff --git a/crates/action_log/src/action_log.rs b/crates/action_log/src/action_log.rs index 6eb18a4f12325f0c181928f99b4eb921265dbf9c..994780c40b9bd1cde45bfe6ba26630771a3040c3 100644 --- a/crates/action_log/src/action_log.rs +++ b/crates/action_log/src/action_log.rs @@ -6,7 +6,7 @@ use futures::{FutureExt, StreamExt, channel::mpsc}; use gpui::{ App, AppContext, AsyncApp, Context, Entity, SharedString, Subscription, Task, WeakEntity, }; -use language::{Anchor, Buffer, BufferEvent, DiskState, Point, ToPoint}; +use language::{Anchor, Buffer, BufferEvent, Point, ToPoint}; use project::{Project, ProjectItem, lsp_store::OpenLspBufferHandle}; use std::{cmp, ops::Range, sync::Arc}; use text::{Edit, Patch, Rope}; @@ -150,7 +150,7 @@ impl ActionLog { if buffer .read(cx) .file() - .is_some_and(|file| file.disk_state() == DiskState::Deleted) + .is_some_and(|file| file.disk_state().is_deleted()) { // If the buffer had been edited by a tool, but it got // deleted externally, we want to stop tracking it. @@ -162,7 +162,7 @@ impl ActionLog { if buffer .read(cx) .file() - .is_some_and(|file| file.disk_state() != DiskState::Deleted) + .is_some_and(|file| !file.disk_state().is_deleted()) { // If the buffer had been deleted by a tool, but it got // resurrected externally, we want to clear the edits we @@ -769,7 +769,7 @@ impl ActionLog { tracked.version != buffer.version && buffer .file() - .is_some_and(|file| file.disk_state() != DiskState::Deleted) + .is_some_and(|file| !file.disk_state().is_deleted()) }) .map(|(buffer, _)| buffer) } diff --git a/crates/agent_ui/src/agent_diff.rs b/crates/agent_ui/src/agent_diff.rs index 91d345b7ebb9dae5225626d7a054d0de1882dfe0..8d2bd534cac9b429cc1789e6ae0d5e7cd2f6e617 100644 --- a/crates/agent_ui/src/agent_diff.rs +++ b/crates/agent_ui/src/agent_diff.rs @@ -17,7 +17,7 @@ use gpui::{ Global, SharedString, Subscription, Task, WeakEntity, Window, prelude::*, }; -use language::{Buffer, Capability, DiskState, OffsetRangeExt, Point}; +use language::{Buffer, Capability, OffsetRangeExt, Point}; use multi_buffer::PathKey; use project::{Project, ProjectItem, ProjectPath}; use settings::{Settings, SettingsStore}; @@ -192,7 +192,7 @@ impl AgentDiffPane { && buffer .read(cx) .file() - .is_some_and(|file| file.disk_state() == DiskState::Deleted) + .is_some_and(|file| file.disk_state().is_deleted()) { editor.fold_buffer(snapshot.text.remote_id(), cx) } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index cfbb7c975c844f08d76a5568f1e02dfe3d7d74f1..34b54795455a9612da04b54f43101f3dcf00efd9 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -17,8 +17,8 @@ use gpui::{ ParentElement, Pixels, SharedString, Styled, Task, WeakEntity, Window, point, }; use language::{ - Bias, Buffer, BufferRow, CharKind, CharScopeContext, DiskState, LocalFile, Point, - SelectionGoal, proto::serialize_anchor as serialize_text_anchor, + Bias, Buffer, BufferRow, CharKind, CharScopeContext, LocalFile, Point, SelectionGoal, + proto::serialize_anchor as serialize_text_anchor, }; use lsp::DiagnosticSeverity; use multi_buffer::MultiBufferOffset; @@ -722,7 +722,7 @@ impl Item for Editor { .read(cx) .as_singleton() .and_then(|buffer| buffer.read(cx).file()) - .is_some_and(|file| file.disk_state() == DiskState::Deleted); + .is_some_and(|file| file.disk_state().is_deleted()); h_flex() .gap_2() diff --git a/crates/git_ui/src/commit_view.rs b/crates/git_ui/src/commit_view.rs index 0f5420fec4169f8e3d945dd8bd0987ebbaba8d19..7a4fdc2c73741e5834ac172df28ba5ac023ec20b 100644 --- a/crates/git_ui/src/commit_view.rs +++ b/crates/git_ui/src/commit_view.rs @@ -69,7 +69,7 @@ struct GitBlob { path: RepoPath, worktree_id: WorktreeId, is_deleted: bool, - display_name: Arc, + display_name: String, } const COMMIT_MESSAGE_SORT_PREFIX: u64 = 0; @@ -243,9 +243,8 @@ impl CommitView { .path .file_name() .map(|name| name.to_string()) - .unwrap_or_else(|| file.path.display(PathStyle::Posix).to_string()); - let display_name: Arc = - Arc::from(format!("{short_sha} - {file_name}").into_boxed_str()); + .unwrap_or_else(|| file.path.display(PathStyle::local()).to_string()); + let display_name = format!("{short_sha} - {file_name}"); let file = Arc::new(GitBlob { path: file.path.clone(), @@ -661,15 +660,13 @@ impl language::File for GitBlob { } fn disk_state(&self) -> DiskState { - if self.is_deleted { - DiskState::Deleted - } else { - DiskState::New + DiskState::Historic { + was_deleted: self.is_deleted, } } fn path_style(&self, _: &App) -> PathStyle { - PathStyle::Posix + PathStyle::local() } fn path(&self) -> &Arc { @@ -697,45 +694,6 @@ impl language::File for GitBlob { } } -// No longer needed since metadata buffer is not created -// impl language::File for CommitMetadataFile { -// fn as_local(&self) -> Option<&dyn language::LocalFile> { -// None -// } -// -// fn disk_state(&self) -> DiskState { -// DiskState::New -// } -// -// fn path_style(&self, _: &App) -> PathStyle { -// PathStyle::Posix -// } -// -// fn path(&self) -> &Arc { -// &self.title -// } -// -// fn full_path(&self, _: &App) -> PathBuf { -// self.title.as_std_path().to_path_buf() -// } -// -// fn file_name<'a>(&'a self, _: &'a App) -> &'a str { -// self.title.file_name().unwrap_or("commit") -// } -// -// fn worktree_id(&self, _: &App) -> WorktreeId { -// self.worktree_id -// } -// -// fn to_proto(&self, _cx: &App) -> language::proto::File { -// unimplemented!() -// } -// -// fn is_private(&self) -> bool { -// false -// } -// } - async fn build_buffer( mut text: String, blob: Arc, diff --git a/crates/image_viewer/src/image_viewer.rs b/crates/image_viewer/src/image_viewer.rs index d7c2341723e798b18d559895d6ea478b491eeaf7..4474b2a9bf7d8d4fe117575d9323e7fee8df1eb2 100644 --- a/crates/image_viewer/src/image_viewer.rs +++ b/crates/image_viewer/src/image_viewer.rs @@ -11,7 +11,7 @@ use gpui::{ InteractiveElement, IntoElement, ObjectFit, ParentElement, Render, Styled, Task, WeakEntity, Window, canvas, div, fill, img, opaque_grey, point, size, }; -use language::{DiskState, File as _}; +use language::File as _; use persistence::IMAGE_VIEWER; use project::{ImageItem, Project, ProjectPath, image_store::ImageItemEvent}; use settings::Settings; @@ -195,7 +195,7 @@ impl Item for ImageView { } fn has_deleted_file(&self, cx: &App) -> bool { - self.image_item.read(cx).file.disk_state() == DiskState::Deleted + self.image_item.read(cx).file.disk_state().is_deleted() } fn buffer_kind(&self, _: &App) -> workspace::item::ItemBufferKind { workspace::item::ItemBufferKind::Singleton diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 5f46340b41a876443f1d12724450d2d8b30f9b33..c919aeca15714864e5a54f8b56d3f8517994cb56 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -427,6 +427,9 @@ pub enum DiskState { Present { mtime: MTime }, /// Deleted file that was previously present. Deleted, + /// An old version of a file that was previously present + /// usually from a version control system. e.g. A git blob + Historic { was_deleted: bool }, } impl DiskState { @@ -436,6 +439,7 @@ impl DiskState { DiskState::New => None, DiskState::Present { mtime } => Some(mtime), DiskState::Deleted => None, + DiskState::Historic { .. } => None, } } @@ -444,6 +448,16 @@ impl DiskState { DiskState::New => false, DiskState::Present { .. } => true, DiskState::Deleted => false, + DiskState::Historic { .. } => false, + } + } + + /// Returns true if this state represents a deleted file. + pub fn is_deleted(&self) -> bool { + match self { + DiskState::Deleted => true, + DiskState::Historic { was_deleted } => *was_deleted, + _ => false, } } } @@ -2274,6 +2288,7 @@ impl Buffer { None => true, }, DiskState::Deleted => false, + DiskState::Historic { .. } => false, } } diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 0c0e87b60a7b8950f7461228c929503d516791e0..3e96a81b387fabeee77c6790dd66433da99b3985 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -19,9 +19,9 @@ use gpui::{App, Context, Entity, EntityId, EventEmitter}; use itertools::Itertools; use language::{ AutoindentMode, BracketMatch, Buffer, BufferChunks, BufferRow, BufferSnapshot, Capability, - CharClassifier, CharKind, CharScopeContext, Chunk, CursorShape, DiagnosticEntryRef, DiskState, - File, IndentGuideSettings, IndentSize, Language, LanguageScope, OffsetRangeExt, OffsetUtf16, - Outline, OutlineItem, Point, PointUtf16, Selection, TextDimension, TextObject, ToOffset as _, + CharClassifier, CharKind, CharScopeContext, Chunk, CursorShape, DiagnosticEntryRef, File, + IndentGuideSettings, IndentSize, Language, LanguageScope, OffsetRangeExt, OffsetUtf16, Outline, + OutlineItem, Point, PointUtf16, Selection, TextDimension, TextObject, ToOffset as _, ToPoint as _, TransactionId, TreeSitterOptions, Unclipped, language_settings::{LanguageSettings, language_settings}, }; @@ -2980,7 +2980,7 @@ impl MultiBuffer { *is_dirty |= buffer.is_dirty(); *has_deleted_file |= buffer .file() - .is_some_and(|file| file.disk_state() == DiskState::Deleted); + .is_some_and(|file| file.disk_state().is_deleted()); *has_conflict |= buffer.has_conflict(); } if edited { diff --git a/crates/project/src/debugger/breakpoint_store.rs b/crates/project/src/debugger/breakpoint_store.rs index 42663ab9852a5dc2e9850d20dd20940c6723d03c..d5a144ee8a83e0064b3afdca123c33e4e8da3e46 100644 --- a/crates/project/src/debugger/breakpoint_store.rs +++ b/crates/project/src/debugger/breakpoint_store.rs @@ -23,7 +23,7 @@ use super::session::ThreadId; mod breakpoints_in_file { use collections::HashMap; - use language::{BufferEvent, DiskState}; + use language::BufferEvent; use super::*; @@ -82,7 +82,7 @@ mod breakpoints_in_file { BufferEvent::FileHandleChanged => { let entity_id = buffer.entity_id(); - if buffer.read(cx).file().is_none_or(|f| f.disk_state() == DiskState::Deleted) { + if buffer.read(cx).file().is_none_or(|f| f.disk_state().is_deleted()) { breakpoint_store.breakpoints.retain(|_, breakpoints_in_file| { breakpoints_in_file.buffer.entity_id() != entity_id }); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 25a19788fdb464f5f289ef3bc3513f21743e3a9a..5111bc04fa256f35feae40464e405d3dd926b286 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -83,7 +83,7 @@ use gpui::{ Task, WeakEntity, Window, }; use language::{ - Buffer, BufferEvent, Capability, CodeLabel, CursorShape, Language, LanguageName, + Buffer, BufferEvent, Capability, CodeLabel, CursorShape, DiskState, Language, LanguageName, LanguageRegistry, PointUtf16, ToOffset, ToPointUtf16, Toolchain, ToolchainMetadata, ToolchainScope, Transaction, Unclipped, language_settings::InlayHintKind, proto::split_operations, @@ -5671,7 +5671,9 @@ impl ProjectItem for Buffer { } fn project_path(&self, cx: &App) -> Option { - self.file().map(|file| ProjectPath { + let file = self.file()?; + + (!matches!(file.disk_state(), DiskState::Historic { .. })).then(|| ProjectPath { worktree_id: file.worktree_id(cx), path: file.path().clone(), }) diff --git a/crates/proto/proto/worktree.proto b/crates/proto/proto/worktree.proto index 5873cfc10c1c6af24520705c27781b916dfda3d0..7917962cf16b3f7e042e6584faf9ab7334e8ae3d 100644 --- a/crates/proto/proto/worktree.proto +++ b/crates/proto/proto/worktree.proto @@ -2,161 +2,162 @@ syntax = "proto3"; package zed.messages; message Timestamp { - uint64 seconds = 1; - uint32 nanos = 2; + uint64 seconds = 1; + uint32 nanos = 2; } message File { - uint64 worktree_id = 1; - optional uint64 entry_id = 2; - string path = 3; - Timestamp mtime = 4; - bool is_deleted = 5; + uint64 worktree_id = 1; + optional uint64 entry_id = 2; + string path = 3; + Timestamp mtime = 4; + bool is_deleted = 5; + bool is_historic = 6; } message Entry { - uint64 id = 1; - bool is_dir = 2; - string path = 3; - uint64 inode = 4; - Timestamp mtime = 5; - bool is_ignored = 7; - bool is_external = 8; - reserved 6; - reserved 9; - bool is_fifo = 10; - optional uint64 size = 11; - optional string canonical_path = 12; - bool is_hidden = 13; + uint64 id = 1; + bool is_dir = 2; + string path = 3; + uint64 inode = 4; + Timestamp mtime = 5; + bool is_ignored = 7; + bool is_external = 8; + reserved 6; + reserved 9; + bool is_fifo = 10; + optional uint64 size = 11; + optional string canonical_path = 12; + bool is_hidden = 13; } message AddWorktree { - string path = 1; - uint64 project_id = 2; - bool visible = 3; + string path = 1; + uint64 project_id = 2; + bool visible = 3; } message AddWorktreeResponse { - uint64 worktree_id = 1; - string canonicalized_path = 2; + uint64 worktree_id = 1; + string canonicalized_path = 2; } message RemoveWorktree { - uint64 worktree_id = 1; + uint64 worktree_id = 1; } message GetPathMetadata { - uint64 project_id = 1; - string path = 2; + uint64 project_id = 1; + string path = 2; } message GetPathMetadataResponse { - bool exists = 1; - string path = 2; - bool is_dir = 3; + bool exists = 1; + string path = 2; + bool is_dir = 3; } message WorktreeMetadata { - uint64 id = 1; - string root_name = 2; - bool visible = 3; - string abs_path = 4; + uint64 id = 1; + string root_name = 2; + bool visible = 3; + string abs_path = 4; } message ProjectPath { - uint64 worktree_id = 1; - string path = 2; + uint64 worktree_id = 1; + string path = 2; } message ListRemoteDirectoryConfig { - bool is_dir = 1; + bool is_dir = 1; } message ListRemoteDirectory { - uint64 dev_server_id = 1; - string path = 2; - ListRemoteDirectoryConfig config = 3; + uint64 dev_server_id = 1; + string path = 2; + ListRemoteDirectoryConfig config = 3; } message EntryInfo { - bool is_dir = 1; + bool is_dir = 1; } message ListRemoteDirectoryResponse { - repeated string entries = 1; - repeated EntryInfo entry_info = 2; + repeated string entries = 1; + repeated EntryInfo entry_info = 2; } message CreateProjectEntry { - uint64 project_id = 1; - uint64 worktree_id = 2; - string path = 3; - bool is_directory = 4; - optional bytes content = 5; + uint64 project_id = 1; + uint64 worktree_id = 2; + string path = 3; + bool is_directory = 4; + optional bytes content = 5; } message RenameProjectEntry { - uint64 project_id = 1; - uint64 entry_id = 2; - string new_path = 3; - uint64 new_worktree_id = 4; + uint64 project_id = 1; + uint64 entry_id = 2; + string new_path = 3; + uint64 new_worktree_id = 4; } message CopyProjectEntry { - uint64 project_id = 1; - uint64 entry_id = 2; - string new_path = 3; - uint64 new_worktree_id = 5; - reserved 4; + uint64 project_id = 1; + uint64 entry_id = 2; + string new_path = 3; + uint64 new_worktree_id = 5; + reserved 4; } message DeleteProjectEntry { - uint64 project_id = 1; - uint64 entry_id = 2; - bool use_trash = 3; + uint64 project_id = 1; + uint64 entry_id = 2; + bool use_trash = 3; } message ExpandProjectEntry { - uint64 project_id = 1; - uint64 entry_id = 2; + uint64 project_id = 1; + uint64 entry_id = 2; } message ExpandProjectEntryResponse { - uint64 worktree_scan_id = 1; + uint64 worktree_scan_id = 1; } message ExpandAllForProjectEntry { - uint64 project_id = 1; - uint64 entry_id = 2; + uint64 project_id = 1; + uint64 entry_id = 2; } message ExpandAllForProjectEntryResponse { - uint64 worktree_scan_id = 1; + uint64 worktree_scan_id = 1; } message ProjectEntryResponse { - optional Entry entry = 1; - uint64 worktree_scan_id = 2; + optional Entry entry = 1; + uint64 worktree_scan_id = 2; } message UpdateWorktreeSettings { - uint64 project_id = 1; - uint64 worktree_id = 2; - string path = 3; - optional string content = 4; - optional LocalSettingsKind kind = 5; + uint64 project_id = 1; + uint64 worktree_id = 2; + string path = 3; + optional string content = 4; + optional LocalSettingsKind kind = 5; } enum LocalSettingsKind { - Settings = 0; - Tasks = 1; - Editorconfig = 2; - Debug = 3; + Settings = 0; + Tasks = 1; + Editorconfig = 2; + Debug = 3; } message UpdateUserSettings { - uint64 project_id = 1; - string contents = 2; + uint64 project_id = 1; + string contents = 2; } message TrustWorktrees { diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index f5f632e65d71b683d1a491b1fc9e9a612f5c24a5..ca5500249e95f0b9d3fb25f75393ab5c9ca5be9b 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -3235,7 +3235,8 @@ impl language::File for File { entry_id: self.entry_id.map(|id| id.to_proto()), path: self.path.as_ref().to_proto(), mtime: self.disk_state.mtime().map(|time| time.into()), - is_deleted: self.disk_state == DiskState::Deleted, + is_deleted: self.disk_state.is_deleted(), + is_historic: matches!(self.disk_state, DiskState::Historic { .. }), } } @@ -3296,7 +3297,11 @@ impl File { "worktree id does not match file" ); - let disk_state = if proto.is_deleted { + let disk_state = if proto.is_historic { + DiskState::Historic { + was_deleted: proto.is_deleted, + } + } else if proto.is_deleted { DiskState::Deleted } else if let Some(mtime) = proto.mtime.map(&Into::into) { DiskState::Present { mtime }