Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Always use strings to represent paths over the wire

Browse source 
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 <nathan@zed.dev>
This commit is contained in:
Antonio Scandurra 2022-11-15 16:46:17 +01:00
parent 974ef967a3
commit 4b1dcf2d55
7 changed files with 32 additions and 35 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/call/src/room.rs
View file

@ -10,7 +10,7 @@ use gpui::{AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext
use live_kit_client::{LocalTrackPublication, LocalVideoTrack, RemoteVideoTrackUpdate}; use live_kit_client::{LocalTrackPublication, LocalVideoTrack, RemoteVideoTrackUpdate};
use postage::stream::Stream; use postage::stream::Stream;
use project::Project; use project::Project;
use std::{mem, os::unix::prelude::OsStrExt, sync::Arc}; use std::{mem, sync::Arc};
use util::{post_inc, ResultExt}; use util::{post_inc, ResultExt};
#[derive(Clone, Debug, PartialEq, Eq)] #[derive(Clone, Debug, PartialEq, Eq)]
@ -553,7 +553,7 @@ impl Room {
id: worktree.id().to_proto(), id: worktree.id().to_proto(),
root_name: worktree.root_name().into(), root_name: worktree.root_name().into(),
visible: worktree.is_visible(), 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(), .collect(),

6
crates/collab/src/db.rs
View file

@ -1446,7 +1446,7 @@ where
.bind(project_id) .bind(project_id)
.bind(worktree.id as i32) .bind(worktree.id as i32)
.bind(&worktree.root_name) .bind(&worktree.root_name)
.bind(&*String::from_utf8_lossy(&worktree.abs_path)) .bind(&worktree.abs_path)
.bind(worktree.visible) .bind(worktree.visible)
.bind(0) .bind(0)
.bind(false) .bind(false)
@ -1510,7 +1510,7 @@ where
.bind(project_id) .bind(project_id)
.bind(worktree.id as i32) .bind(worktree.id as i32)
.bind(&worktree.root_name) .bind(&worktree.root_name)
.bind(String::from_utf8_lossy(&worktree.abs_path).as_ref()) .bind(&worktree.abs_path)
.bind(worktree.visible) .bind(worktree.visible)
.bind(0) .bind(0)
.bind(false) .bind(false)
@ -1687,7 +1687,7 @@ where
worktree.entries.push(proto::Entry { worktree.entries.push(proto::Entry {
id: entry.id as u64, id: entry.id as u64,
is_dir: entry.is_dir, is_dir: entry.is_dir,
path: entry.path.into_bytes(), path: entry.path,
inode: entry.inode as u64, inode: entry.inode as u64,
mtime: Some(proto::Timestamp { mtime: Some(proto::Timestamp {
seconds: entry.mtime_seconds as u64, seconds: entry.mtime_seconds as u64,

4
crates/collab/src/rpc.rs
View file

@ -954,7 +954,7 @@ impl Server {
id: id.to_proto(), id: id.to_proto(),
root_name: worktree.root_name.clone(), root_name: worktree.root_name.clone(),
visible: worktree.visible, visible: worktree.visible,
abs_path: worktree.abs_path.as_bytes().to_vec(), abs_path: worktree.abs_path.clone(),
}) })
.collect::<Vec<_>>(); .collect::<Vec<_>>();
@ -995,7 +995,7 @@ impl Server {
let message = proto::UpdateWorktree { let message = proto::UpdateWorktree {
project_id: project_id.to_proto(), project_id: project_id.to_proto(),
worktree_id: worktree_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, root_name: worktree.root_name,
updated_entries: worktree.entries, updated_entries: worktree.entries,
removed_entries: Default::default(), removed_entries: Default::default(),

14
crates/project/src/project.rs
View file

@ -44,12 +44,10 @@ use std::{
cell::RefCell, cell::RefCell,
cmp::{self, Ordering}, cmp::{self, Ordering},
convert::TryInto, convert::TryInto,
ffi::OsString,
hash::Hash, hash::Hash,
mem, mem,
num::NonZeroU32, num::NonZeroU32,
ops::Range, ops::Range,
os::unix::{ffi::OsStrExt, prelude::OsStringExt},
path::{Component, Path, PathBuf}, path::{Component, Path, PathBuf},
rc::Rc, rc::Rc,
str, str,
@ -837,7 +835,7 @@ impl Project {
.request(proto::CreateProjectEntry { .request(proto::CreateProjectEntry {
worktree_id: project_path.worktree_id.to_proto(), worktree_id: project_path.worktree_id.to_proto(),
project_id, project_id,
path: project_path.path.as_os_str().as_bytes().to_vec(), path: project_path.path.to_string_lossy().into(),
is_directory, is_directory,
}) })
.await?; .await?;
@ -881,7 +879,7 @@ impl Project {
.request(proto::CopyProjectEntry { .request(proto::CopyProjectEntry {
project_id, project_id,
entry_id: entry_id.to_proto(), 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?; .await?;
let entry = response let entry = response
@ -924,7 +922,7 @@ impl Project {
.request(proto::RenameProjectEntry { .request(proto::RenameProjectEntry {
project_id, project_id,
entry_id: entry_id.to_proto(), 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?; .await?;
let entry = response let entry = response
@ -4606,7 +4604,7 @@ impl Project {
let entry = worktree let entry = worktree
.update(&mut cx, |worktree, cx| { .update(&mut cx, |worktree, cx| {
let worktree = worktree.as_local_mut().unwrap(); 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) worktree.create_entry(path, envelope.payload.is_directory, cx)
}) })
.await?; .await?;
@ -4630,7 +4628,7 @@ impl Project {
let worktree_scan_id = worktree.read_with(&cx, |worktree, _| worktree.scan_id()); let worktree_scan_id = worktree.read_with(&cx, |worktree, _| worktree.scan_id());
let entry = worktree let entry = worktree
.update(&mut cx, |worktree, cx| { .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 worktree
.as_local_mut() .as_local_mut()
.unwrap() .unwrap()
@ -4658,7 +4656,7 @@ impl Project {
let worktree_scan_id = worktree.read_with(&cx, |worktree, _| worktree.scan_id()); let worktree_scan_id = worktree.read_with(&cx, |worktree, _| worktree.scan_id());
let entry = worktree let entry = worktree
.update(&mut cx, |worktree, cx| { .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 worktree
.as_local_mut() .as_local_mut()
.unwrap() .unwrap()

6
crates/project/src/project_tests.rs
View file

@ -2166,7 +2166,11 @@ async fn test_rescan_and_remote_updates(
proto::WorktreeMetadata { proto::WorktreeMetadata {
id: initial_snapshot.id().to_proto(), id: initial_snapshot.id().to_proto(),
root_name: initial_snapshot.root_name().into(), 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, visible: true,
}, },
rpc.clone(), rpc.clone(),

21
crates/project/src/worktree.rs
View file

@ -40,7 +40,6 @@ use std::{
future::Future, future::Future,
mem, mem,
ops::{Deref, DerefMut}, ops::{Deref, DerefMut},
os::unix::prelude::{OsStrExt, OsStringExt},
path::{Path, PathBuf}, path::{Path, PathBuf},
sync::{atomic::AtomicUsize, Arc}, sync::{atomic::AtomicUsize, Arc},
task::Poll, task::Poll,
@ -221,7 +220,7 @@ impl Worktree {
let root_name = worktree.root_name.clone(); let root_name = worktree.root_name.clone();
let visible = worktree.visible; 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 { let snapshot = Snapshot {
id: WorktreeId(remote_id as usize), id: WorktreeId(remote_id as usize),
abs_path: Arc::from(abs_path.deref()), abs_path: Arc::from(abs_path.deref()),
@ -656,7 +655,7 @@ impl LocalWorktree {
id: self.id().to_proto(), id: self.id().to_proto(),
root_name: self.root_name().to_string(), root_name: self.root_name().to_string(),
visible: self.visible, 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 { let update = proto::UpdateWorktree {
project_id, project_id,
worktree_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(), root_name: snapshot.root_name().to_string(),
updated_entries: snapshot updated_entries: snapshot
.entries_by_path .entries_by_path
@ -1381,7 +1380,7 @@ impl LocalSnapshot {
proto::UpdateWorktree { proto::UpdateWorktree {
project_id, project_id,
worktree_id: self.id().to_proto(), 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, root_name,
updated_entries: self.entries_by_path.iter().map(Into::into).collect(), updated_entries: self.entries_by_path.iter().map(Into::into).collect(),
removed_entries: Default::default(), removed_entries: Default::default(),
@ -1449,7 +1448,7 @@ impl LocalSnapshot {
proto::UpdateWorktree { proto::UpdateWorktree {
project_id, project_id,
worktree_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(), root_name: self.root_name().to_string(),
updated_entries, updated_entries,
removed_entries, removed_entries,
@ -2928,7 +2927,7 @@ impl<'a> From<&'a Entry> for proto::Entry {
Self { Self {
id: entry.id.to_proto(), id: entry.id.to_proto(),
is_dir: entry.is_dir(), 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, inode: entry.inode,
mtime: Some(entry.mtime.into()), mtime: Some(entry.mtime.into()),
is_symlink: entry.is_symlink, is_symlink: entry.is_symlink,
@ -2946,14 +2945,10 @@ impl<'a> TryFrom<(&'a CharBag, proto::Entry)> for Entry {
EntryKind::Dir EntryKind::Dir
} else { } else {
let mut char_bag = *root_char_bag; let mut char_bag = *root_char_bag;
char_bag.extend( char_bag.extend(entry.path.chars().map(|c| c.to_ascii_lowercase()));
String::from_utf8_lossy(&entry.path)
.chars()
.map(|c| c.to_ascii_lowercase()),
);
EntryKind::File(char_bag) EntryKind::File(char_bag)
}; };
let path: Arc<Path> = PathBuf::from(OsString::from_vec(entry.path)).into(); let path: Arc<Path> = PathBuf::from(entry.path).into();
Ok(Entry { Ok(Entry {
id: ProjectEntryId::from_proto(entry.id), id: ProjectEntryId::from_proto(entry.id),
kind, kind,

12
crates/rpc/proto/zed.proto
View file

@ -279,26 +279,26 @@ message UpdateWorktree {
repeated uint64 removed_entries = 5; repeated uint64 removed_entries = 5;
uint64 scan_id = 6; uint64 scan_id = 6;
bool is_last_update = 7; bool is_last_update = 7;
bytes abs_path = 8; string abs_path = 8;
} }
message CreateProjectEntry { message CreateProjectEntry {
uint64 project_id = 1; uint64 project_id = 1;
uint64 worktree_id = 2; uint64 worktree_id = 2;
bytes path = 3; string path = 3;
bool is_directory = 4; bool is_directory = 4;
} }
message RenameProjectEntry { message RenameProjectEntry {
uint64 project_id = 1; uint64 project_id = 1;
uint64 entry_id = 2; uint64 entry_id = 2;
bytes new_path = 3; string new_path = 3;
} }
message CopyProjectEntry { message CopyProjectEntry {
uint64 project_id = 1; uint64 project_id = 1;
uint64 entry_id = 2; uint64 entry_id = 2;
bytes new_path = 3; string new_path = 3;
} }
message DeleteProjectEntry { message DeleteProjectEntry {
@ -884,7 +884,7 @@ message File {
message Entry { message Entry {
uint64 id = 1; uint64 id = 1;
bool is_dir = 2; bool is_dir = 2;
bytes path = 3; string path = 3;
uint64 inode = 4; uint64 inode = 4;
Timestamp mtime = 5; Timestamp mtime = 5;
bool is_symlink = 6; bool is_symlink = 6;
@ -1068,7 +1068,7 @@ message WorktreeMetadata {
uint64 id = 1; uint64 id = 1;
string root_name = 2; string root_name = 2;
bool visible = 3; bool visible = 3;
bytes abs_path = 4; string abs_path = 4;
} }
message UpdateDiffBase { message UpdateDiffBase {