Restore the ability to drag and drop images into the editor (#31009)

Kirill Bulatov created

`ImageItem`'s `file` is returning `""` as its `path` for single-filed
worktrees like the ones are created for the images dropped from the OS.
`ImageItem::load_image_metadata` had used that `path` in FS operations
and the other method tried to use for icon resolving.

Rework the code to use a more specific, `worktree::File` instead and
always use the `abs_path` when dealing with paths from this `file`.

Release Notes:

- Fixed images not opening on drag and drop into the editor

Change summary

Cargo.lock                              |  1 
crates/image_viewer/Cargo.toml          |  1 
crates/image_viewer/src/image_viewer.rs |  9 ++--
crates/project/src/image_store.rs       | 50 +++++++++++---------------
crates/worktree/src/worktree.rs         | 11 +++++
5 files changed, 39 insertions(+), 33 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -7946,6 +7946,7 @@ dependencies = [
  "editor",
  "file_icons",
  "gpui",
+ "language",
  "log",
  "project",
  "schemars",

crates/image_viewer/Cargo.toml 🔗

@@ -21,6 +21,7 @@ db.workspace = true
 editor.workspace = true
 file_icons.workspace = true
 gpui.workspace = true
+language.workspace = true
 log.workspace = true
 project.workspace = true
 schemars.workspace = true

crates/image_viewer/src/image_viewer.rs 🔗

@@ -11,6 +11,7 @@ use gpui::{
     InteractiveElement, IntoElement, ObjectFit, ParentElement, Render, Styled, Task, WeakEntity,
     Window, canvas, div, fill, img, opaque_grey, point, size,
 };
+use language::File as _;
 use persistence::IMAGE_VIEWER;
 use project::{ImageItem, Project, ProjectPath, image_store::ImageItemEvent};
 use settings::Settings;
@@ -104,7 +105,7 @@ impl Item for ImageView {
     }
 
     fn tab_tooltip_text(&self, cx: &App) -> Option<SharedString> {
-        let abs_path = self.image_item.read(cx).file.as_local()?.abs_path(cx);
+        let abs_path = self.image_item.read(cx).abs_path(cx)?;
         let file_path = abs_path.compact().to_string_lossy().to_string();
         Some(file_path.into())
     }
@@ -149,10 +150,10 @@ impl Item for ImageView {
     }
 
     fn tab_icon(&self, _: &Window, cx: &App) -> Option<Icon> {
-        let path = self.image_item.read(cx).path();
+        let path = self.image_item.read(cx).abs_path(cx)?;
         ItemSettings::get_global(cx)
             .file_icons
-            .then(|| FileIcons::get_icon(path, cx))
+            .then(|| FileIcons::get_icon(&path, cx))
             .flatten()
             .map(Icon::from_path)
     }
@@ -274,7 +275,7 @@ impl SerializableItem for ImageView {
         cx: &mut Context<Self>,
     ) -> Option<Task<gpui::Result<()>>> {
         let workspace_id = workspace.database_id()?;
-        let image_path = self.image_item.read(cx).file.as_local()?.abs_path(cx);
+        let image_path = self.image_item.read(cx).abs_path(cx)?;
 
         Some(cx.background_spawn({
             async move {

crates/project/src/image_store.rs 🔗

@@ -12,10 +12,10 @@ pub use image::ImageFormat;
 use image::{ExtendedColorType, GenericImageView, ImageReader};
 use language::{DiskState, File};
 use rpc::{AnyProtoClient, ErrorExt as _};
-use std::ffi::OsStr;
 use std::num::NonZeroU64;
 use std::path::Path;
 use std::sync::Arc;
+use std::{ffi::OsStr, path::PathBuf};
 use util::ResultExt;
 use worktree::{LoadedBinaryFile, PathChange, Worktree};
 
@@ -96,7 +96,7 @@ impl ImageColorInfo {
 
 pub struct ImageItem {
     pub id: ImageId,
-    pub file: Arc<dyn File>,
+    pub file: Arc<worktree::File>,
     pub image: Arc<gpui::Image>,
     reload_task: Option<Task<()>>,
     pub image_metadata: Option<ImageMetadata>,
@@ -109,22 +109,11 @@ impl ImageItem {
         cx: &mut AsyncApp,
     ) -> Result<ImageMetadata> {
         let (fs, image_path) = cx.update(|cx| {
-            let project_path = image.read(cx).project_path(cx);
-
-            let worktree = project
-                .read(cx)
-                .worktree_for_id(project_path.worktree_id, cx)
-                .ok_or_else(|| anyhow!("worktree not found"))?;
-            let worktree_root = worktree.read(cx).abs_path();
-            let image_path = image.read(cx).path();
-            let image_path = if image_path.is_absolute() {
-                image_path.to_path_buf()
-            } else {
-                worktree_root.join(image_path)
-            };
-
             let fs = project.read(cx).fs().clone();
-
+            let image_path = image
+                .read(cx)
+                .abs_path(cx)
+                .context("absolutizing image file path")?;
             anyhow::Ok((fs, image_path))
         })??;
 
@@ -157,14 +146,14 @@ impl ImageItem {
         }
     }
 
-    pub fn path(&self) -> &Arc<Path> {
-        self.file.path()
+    pub fn abs_path(&self, cx: &App) -> Option<PathBuf> {
+        Some(self.file.as_local()?.abs_path(cx))
     }
 
-    fn file_updated(&mut self, new_file: Arc<dyn File>, cx: &mut Context<Self>) {
+    fn file_updated(&mut self, new_file: Arc<worktree::File>, cx: &mut Context<Self>) {
         let mut file_changed = false;
 
-        let old_file = self.file.as_ref();
+        let old_file = &self.file;
         if new_file.path() != old_file.path() {
             file_changed = true;
         }
@@ -251,7 +240,7 @@ impl ProjectItem for ImageItem {
     }
 
     fn entry_id(&self, _: &App) -> Option<ProjectEntryId> {
-        worktree::File::from_dyn(Some(&self.file))?.entry_id
+        self.file.entry_id
     }
 
     fn project_path(&self, cx: &App) -> Option<ProjectPath> {
@@ -387,6 +376,12 @@ impl ImageStore {
                 entry.insert(rx.clone());
 
                 let project_path = project_path.clone();
+                // TODO kb this is causing another error, and we also pass a worktree nearby — seems ok to pass "" here?
+                // let image_path = worktree
+                //     .read(cx)
+                //     .absolutize(&project_path.path)
+                //     .map(Arc::from)
+                //     .unwrap_or_else(|_| project_path.path.clone());
                 let load_image = self
                     .state
                     .open_image(project_path.path.clone(), worktree, cx);
@@ -604,9 +599,7 @@ impl LocalImageStore {
         };
 
         image.update(cx, |image, cx| {
-            let Some(old_file) = worktree::File::from_dyn(Some(&image.file)) else {
-                return;
-            };
+            let old_file = &image.file;
             if old_file.worktree != *worktree {
                 return;
             }
@@ -639,7 +632,7 @@ impl LocalImageStore {
                 }
             };
 
-            if new_file == *old_file {
+            if new_file == **old_file {
                 return;
             }
 
@@ -672,9 +665,10 @@ impl LocalImageStore {
     }
 
     fn image_changed_file(&mut self, image: Entity<ImageItem>, cx: &mut App) -> Option<()> {
-        let file = worktree::File::from_dyn(Some(&image.read(cx).file))?;
+        let image = image.read(cx);
+        let file = &image.file;
 
-        let image_id = image.read(cx).id;
+        let image_id = image.id;
         if let Some(entry_id) = file.entry_id {
             match self.local_image_ids_by_entry_id.get(&entry_id) {
                 Some(_) => {

crates/worktree/src/worktree.rs 🔗

@@ -107,6 +107,15 @@ pub struct LoadedBinaryFile {
     pub content: Vec<u8>,
 }
 
+impl fmt::Debug for LoadedBinaryFile {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("LoadedBinaryFile")
+            .field("file", &self.file)
+            .field("content_bytes", &self.content.len())
+            .finish()
+    }
+}
+
 pub struct LocalWorktree {
     snapshot: LocalSnapshot,
     scan_requests_tx: channel::Sender<ScanRequest>,
@@ -3293,7 +3302,7 @@ impl fmt::Debug for Snapshot {
     }
 }
 
-#[derive(Clone, PartialEq)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct File {
     pub worktree: Entity<Worktree>,
     pub path: Arc<Path>,