From 4b1dcf2d55002ded81dfccc4ed93193c51be184c Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 15 Nov 2022 16:46:17 +0100 Subject: [PATCH] Always use strings to represent paths over the wire Previously, the protocol used a mix of strings and bytes without any consistency. When we go to multiple platforms, we won't be able to mix encodings of paths anyway. We don't know this is the right approach, but it at least makes things consistent and easy to read in the database, on the wire, etc. Really, we should be using entry ids etc to refer to entries on the wire anyway, but there's a chance this is the wrong decision. Co-Authored-By: Nathan Sobo --- crates/call/src/room.rs | 4 ++-- crates/collab/src/db.rs | 6 +++--- crates/collab/src/rpc.rs | 4 ++-- crates/project/src/project.rs | 14 ++++++-------- crates/project/src/project_tests.rs | 6 +++++- crates/project/src/worktree.rs | 21 ++++++++------------- crates/rpc/proto/zed.proto | 12 ++++++------ 7 files changed, 32 insertions(+), 35 deletions(-) diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index 4ba8d8effc..8c1b0d9de0 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -10,7 +10,7 @@ use gpui::{AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext use live_kit_client::{LocalTrackPublication, LocalVideoTrack, RemoteVideoTrackUpdate}; use postage::stream::Stream; use project::Project; -use std::{mem, os::unix::prelude::OsStrExt, sync::Arc}; +use std::{mem, sync::Arc}; use util::{post_inc, ResultExt}; #[derive(Clone, Debug, PartialEq, Eq)] @@ -553,7 +553,7 @@ impl Room { id: worktree.id().to_proto(), root_name: worktree.root_name().into(), visible: worktree.is_visible(), - abs_path: worktree.abs_path().as_os_str().as_bytes().to_vec(), + abs_path: worktree.abs_path().to_string_lossy().into(), } }) .collect(), diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 6db4ad101b..4cd3ce3a7c 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1446,7 +1446,7 @@ where .bind(project_id) .bind(worktree.id as i32) .bind(&worktree.root_name) - .bind(&*String::from_utf8_lossy(&worktree.abs_path)) + .bind(&worktree.abs_path) .bind(worktree.visible) .bind(0) .bind(false) @@ -1510,7 +1510,7 @@ where .bind(project_id) .bind(worktree.id as i32) .bind(&worktree.root_name) - .bind(String::from_utf8_lossy(&worktree.abs_path).as_ref()) + .bind(&worktree.abs_path) .bind(worktree.visible) .bind(0) .bind(false) @@ -1687,7 +1687,7 @@ where worktree.entries.push(proto::Entry { id: entry.id as u64, is_dir: entry.is_dir, - path: entry.path.into_bytes(), + path: entry.path, inode: entry.inode as u64, mtime: Some(proto::Timestamp { seconds: entry.mtime_seconds as u64, diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 3c7d4ec61b..5fcb8d5f9c 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -954,7 +954,7 @@ impl Server { id: id.to_proto(), root_name: worktree.root_name.clone(), visible: worktree.visible, - abs_path: worktree.abs_path.as_bytes().to_vec(), + abs_path: worktree.abs_path.clone(), }) .collect::>(); @@ -995,7 +995,7 @@ impl Server { let message = proto::UpdateWorktree { project_id: project_id.to_proto(), worktree_id: worktree_id.to_proto(), - abs_path: worktree.abs_path.as_bytes().to_vec(), + abs_path: worktree.abs_path.clone(), root_name: worktree.root_name, updated_entries: worktree.entries, removed_entries: Default::default(), diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index c59b19de8f..9ac10d1406 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -44,12 +44,10 @@ use std::{ cell::RefCell, cmp::{self, Ordering}, convert::TryInto, - ffi::OsString, hash::Hash, mem, num::NonZeroU32, ops::Range, - os::unix::{ffi::OsStrExt, prelude::OsStringExt}, path::{Component, Path, PathBuf}, rc::Rc, str, @@ -837,7 +835,7 @@ impl Project { .request(proto::CreateProjectEntry { worktree_id: project_path.worktree_id.to_proto(), project_id, - path: project_path.path.as_os_str().as_bytes().to_vec(), + path: project_path.path.to_string_lossy().into(), is_directory, }) .await?; @@ -881,7 +879,7 @@ impl Project { .request(proto::CopyProjectEntry { project_id, entry_id: entry_id.to_proto(), - new_path: new_path.as_os_str().as_bytes().to_vec(), + new_path: new_path.to_string_lossy().into(), }) .await?; let entry = response @@ -924,7 +922,7 @@ impl Project { .request(proto::RenameProjectEntry { project_id, entry_id: entry_id.to_proto(), - new_path: new_path.as_os_str().as_bytes().to_vec(), + new_path: new_path.to_string_lossy().into(), }) .await?; let entry = response @@ -4606,7 +4604,7 @@ impl Project { let entry = worktree .update(&mut cx, |worktree, cx| { let worktree = worktree.as_local_mut().unwrap(); - let path = PathBuf::from(OsString::from_vec(envelope.payload.path)); + let path = PathBuf::from(envelope.payload.path); worktree.create_entry(path, envelope.payload.is_directory, cx) }) .await?; @@ -4630,7 +4628,7 @@ impl Project { let worktree_scan_id = worktree.read_with(&cx, |worktree, _| worktree.scan_id()); let entry = worktree .update(&mut cx, |worktree, cx| { - let new_path = PathBuf::from(OsString::from_vec(envelope.payload.new_path)); + let new_path = PathBuf::from(envelope.payload.new_path); worktree .as_local_mut() .unwrap() @@ -4658,7 +4656,7 @@ impl Project { let worktree_scan_id = worktree.read_with(&cx, |worktree, _| worktree.scan_id()); let entry = worktree .update(&mut cx, |worktree, cx| { - let new_path = PathBuf::from(OsString::from_vec(envelope.payload.new_path)); + let new_path = PathBuf::from(envelope.payload.new_path); worktree .as_local_mut() .unwrap() diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index ca274b18b8..77d2a610d5 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -2166,7 +2166,11 @@ async fn test_rescan_and_remote_updates( proto::WorktreeMetadata { id: initial_snapshot.id().to_proto(), root_name: initial_snapshot.root_name().into(), - abs_path: initial_snapshot.abs_path().as_os_str().as_bytes().to_vec(), + abs_path: initial_snapshot + .abs_path() + .as_os_str() + .to_string_lossy() + .into(), visible: true, }, rpc.clone(), diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 9e4ec3ffb9..ddd4a7a6c8 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -40,7 +40,6 @@ use std::{ future::Future, mem, ops::{Deref, DerefMut}, - os::unix::prelude::{OsStrExt, OsStringExt}, path::{Path, PathBuf}, sync::{atomic::AtomicUsize, Arc}, task::Poll, @@ -221,7 +220,7 @@ impl Worktree { let root_name = worktree.root_name.clone(); let visible = worktree.visible; - let abs_path = PathBuf::from(OsString::from_vec(worktree.abs_path)); + let abs_path = PathBuf::from(worktree.abs_path); let snapshot = Snapshot { id: WorktreeId(remote_id as usize), abs_path: Arc::from(abs_path.deref()), @@ -656,7 +655,7 @@ impl LocalWorktree { id: self.id().to_proto(), root_name: self.root_name().to_string(), visible: self.visible, - abs_path: self.abs_path().as_os_str().as_bytes().to_vec(), + abs_path: self.abs_path().as_os_str().to_string_lossy().into(), } } @@ -990,7 +989,7 @@ impl LocalWorktree { let update = proto::UpdateWorktree { project_id, worktree_id, - abs_path: snapshot.abs_path().as_os_str().as_bytes().to_vec(), + abs_path: snapshot.abs_path().to_string_lossy().into(), root_name: snapshot.root_name().to_string(), updated_entries: snapshot .entries_by_path @@ -1381,7 +1380,7 @@ impl LocalSnapshot { proto::UpdateWorktree { project_id, worktree_id: self.id().to_proto(), - abs_path: self.abs_path().as_os_str().as_bytes().to_vec(), + abs_path: self.abs_path().to_string_lossy().into(), root_name, updated_entries: self.entries_by_path.iter().map(Into::into).collect(), removed_entries: Default::default(), @@ -1449,7 +1448,7 @@ impl LocalSnapshot { proto::UpdateWorktree { project_id, worktree_id, - abs_path: self.abs_path().as_os_str().as_bytes().to_vec(), + abs_path: self.abs_path().to_string_lossy().into(), root_name: self.root_name().to_string(), updated_entries, removed_entries, @@ -2928,7 +2927,7 @@ impl<'a> From<&'a Entry> for proto::Entry { Self { id: entry.id.to_proto(), is_dir: entry.is_dir(), - path: entry.path.as_os_str().as_bytes().to_vec(), + path: entry.path.to_string_lossy().into(), inode: entry.inode, mtime: Some(entry.mtime.into()), is_symlink: entry.is_symlink, @@ -2946,14 +2945,10 @@ impl<'a> TryFrom<(&'a CharBag, proto::Entry)> for Entry { EntryKind::Dir } else { let mut char_bag = *root_char_bag; - char_bag.extend( - String::from_utf8_lossy(&entry.path) - .chars() - .map(|c| c.to_ascii_lowercase()), - ); + char_bag.extend(entry.path.chars().map(|c| c.to_ascii_lowercase())); EntryKind::File(char_bag) }; - let path: Arc = PathBuf::from(OsString::from_vec(entry.path)).into(); + let path: Arc = PathBuf::from(entry.path).into(); Ok(Entry { id: ProjectEntryId::from_proto(entry.id), kind, diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 8aed5ef5cf..30c1c89e8f 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -279,26 +279,26 @@ message UpdateWorktree { repeated uint64 removed_entries = 5; uint64 scan_id = 6; bool is_last_update = 7; - bytes abs_path = 8; + string abs_path = 8; } message CreateProjectEntry { uint64 project_id = 1; uint64 worktree_id = 2; - bytes path = 3; + string path = 3; bool is_directory = 4; } message RenameProjectEntry { uint64 project_id = 1; uint64 entry_id = 2; - bytes new_path = 3; + string new_path = 3; } message CopyProjectEntry { uint64 project_id = 1; uint64 entry_id = 2; - bytes new_path = 3; + string new_path = 3; } message DeleteProjectEntry { @@ -884,7 +884,7 @@ message File { message Entry { uint64 id = 1; bool is_dir = 2; - bytes path = 3; + string path = 3; uint64 inode = 4; Timestamp mtime = 5; bool is_symlink = 6; @@ -1068,7 +1068,7 @@ message WorktreeMetadata { uint64 id = 1; string root_name = 2; bool visible = 3; - bytes abs_path = 4; + string abs_path = 4; } message UpdateDiffBase {