git: Fix bug where opening a git blob from historic commit view could fail (#44226)

Anthony Eid , Cole Miller , cameron , and xipengjin created

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 <cole@zed.dev>
Co-authored-by: cameron <cameron.studdstreet@gmail.com>
Co-authored-by: xipengjin <jinxp18@gmail.com>

Change summary

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 
crates/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(-)

Detailed changes

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)
     }

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)
                 }

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()

crates/git_ui/src/commit_view.rs 🔗

@@ -69,7 +69,7 @@ struct GitBlob {
     path: RepoPath,
     worktree_id: WorktreeId,
     is_deleted: bool,
-    display_name: Arc<str>,
+    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<str> =
-                    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<RelPath> {
@@ -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<RelPath> {
-//         &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<dyn File>,

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

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,
         }
     }
 

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 {

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
                             });

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<ProjectPath> {
-        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(),
         })

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 {

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 }