oak-team/oak-gpui
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix ordering of multibuffer excerpts (#39476)

Browse Source 
The ordering of path-based excerpts in multibuffers regressed with
#38744, because we changed the `path` field of `PathKey` to be a string
(from `std::path::Path`) and used the derived `Ord` implementation,
which doesn't agree with the path-based order of worktree traversals.
This PR fixes that by using `RelPath` for `PathKey`. Instead of using
`File::full_path`, which can be absolute, we always use `File::path` and
distinguish different worktrees using their ID.

Release Notes:

- N/A

---------

Co-authored-by: Lukas Wirth <me@lukaswirth.dev>
This commit is contained in:
Cole Miller
2025-10-03 22:17:31 +00:00
committed by GitHub
co-authored by Lukas Wirth
parent 2859cbdba9
commit aced13bc9f
6 changed files with 50 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/collab/src/tests/git_tests.rs
+10 -2
View File
@@ -84,7 +84,11 @@ async fn test_project_diff(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext)
diff.update(cx_b, |diff, cx| {
assert_eq!(
diff.excerpt_paths(cx),
vec!["changed.txt", "deleted.txt", "created.txt"]
vec![
rel_path("changed.txt").into_arc(),
rel_path("deleted.txt").into_arc(),
rel_path("created.txt").into_arc()
]
);
});
@@ -121,7 +125,11 @@ async fn test_project_diff(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext)
diff.update(cx_b, |diff, cx| {
assert_eq!(
diff.excerpt_paths(cx),
vec!["deleted.txt", "unchanged.txt", "created.txt"]
vec![
rel_path("deleted.txt").into_arc(),
rel_path("unchanged.txt").into_arc(),
rel_path("created.txt").into_arc()
]
);
});
}
crates/editor/src/editor_tests.rs
+3 -6
View File
@@ -16515,7 +16515,7 @@ async fn test_following_with_multiple_excerpts(cx: &mut TestAppContext) {
leader.update(cx, |leader, cx| {
leader.buffer.update(cx, |multibuffer, cx| {
multibuffer.set_excerpts_for_path(
PathKey::namespaced(1, "b.txt".into()),
PathKey::namespaced(1, rel_path("b.txt").into_arc()),
buffer_1.clone(),
vec![
Point::row_range(0..3),
@@ -16526,7 +16526,7 @@ async fn test_following_with_multiple_excerpts(cx: &mut TestAppContext) {
cx,
);
multibuffer.set_excerpts_for_path(
PathKey::namespaced(1, "a.txt".into()),
PathKey::namespaced(1, rel_path("a.txt").into_arc()),
buffer_2.clone(),
vec![Point::row_range(0..6), Point::row_range(8..12)],
0,
@@ -21025,10 +21025,7 @@ async fn test_display_diff_hunks(cx: &mut TestAppContext) {
for buffer in &buffers {
let snapshot = buffer.read(cx).snapshot();
multibuffer.set_excerpts_for_path(
PathKey::namespaced(
0,
buffer.read(cx).file().unwrap().path().as_unix_str().into(),
),
PathKey::namespaced(0, buffer.read(cx).file().unwrap().path().clone()),
buffer.clone(),
vec![text::Anchor::MIN.to_point(&snapshot)..text::Anchor::MAX.to_point(&snapshot)],
2,
crates/git_ui/src/commit_view.rs
+4 -4
View File
@@ -43,8 +43,8 @@ struct CommitMetadataFile {
worktree_id: WorktreeId,
}
const COMMIT_METADATA_NAMESPACE: u32 = 0;
const FILE_NAMESPACE: u32 = 1;
const COMMIT_METADATA_NAMESPACE: u64 = 0;
const FILE_NAMESPACE: u64 = 1;
impl CommitView {
pub fn open(
@@ -145,7 +145,7 @@ impl CommitView {
});
multibuffer.update(cx, |multibuffer, cx| {
multibuffer.set_excerpts_for_path(
PathKey::namespaced(COMMIT_METADATA_NAMESPACE, file.title.as_unix_str().into()),
PathKey::namespaced(COMMIT_METADATA_NAMESPACE, file.title.clone()),
buffer.clone(),
vec![Point::zero()..buffer.read(cx).max_point()],
0,
@@ -193,7 +193,7 @@ impl CommitView {
.collect::<Vec<_>>();
let path = snapshot.file().unwrap().path().clone();
let _is_newly_added = multibuffer.set_excerpts_for_path(
PathKey::namespaced(FILE_NAMESPACE, path.as_unix_str().into()),
PathKey::namespaced(FILE_NAMESPACE, path),
buffer,
diff_hunk_ranges,
multibuffer_context_lines(cx),
crates/git_ui/src/project_diff.rs
+11 -10
View File
@@ -73,9 +73,9 @@ struct DiffBuffer {
file_status: FileStatus,
}
const CONFLICT_NAMESPACE: u32 = 1;
const TRACKED_NAMESPACE: u32 = 2;
const NEW_NAMESPACE: u32 = 3;
const CONFLICT_NAMESPACE: u64 = 1;
const TRACKED_NAMESPACE: u64 = 2;
const NEW_NAMESPACE: u64 = 3;
impl ProjectDiff {
pub(crate) fn register(workspace: &mut Workspace, cx: &mut Context<Workspace>) {
@@ -243,7 +243,7 @@ impl ProjectDiff {
TRACKED_NAMESPACE
};
let path_key = PathKey::namespaced(namespace, entry.repo_path.as_unix_str().into());
let path_key = PathKey::namespaced(namespace, entry.repo_path.0);
self.move_to_path(path_key, window, cx)
}
@@ -397,7 +397,7 @@ impl ProjectDiff {
} else {
TRACKED_NAMESPACE
};
let path_key = PathKey::namespaced(namespace, entry.repo_path.as_unix_str().into());
let path_key = PathKey::namespaced(namespace, entry.repo_path.0.clone());
previous_paths.remove(&path_key);
let load_buffer = self
@@ -531,11 +531,12 @@ impl ProjectDiff {
}
#[cfg(any(test, feature = "test-support"))]
pub fn excerpt_paths(&self, cx: &App) -> Vec<String> {
pub fn excerpt_paths(&self, cx: &App) -> Vec<std::sync::Arc<util::rel_path::RelPath>> {
self.multibuffer
.read(cx)
.excerpt_paths()
.map(|key| key.path().to_string())
.map(|key| key.path())
.cloned()
.collect()
}
}
@@ -1361,7 +1362,7 @@ mod tests {
use settings::SettingsStore;
use std::path::Path;
use unindent::Unindent as _;
use util::path;
use util::{path, rel_path::rel_path};
use super::*;
@@ -1467,7 +1468,7 @@ mod tests {
let editor = cx.update_window_entity(&diff, |diff, window, cx| {
diff.move_to_path(
PathKey::namespaced(TRACKED_NAMESPACE, "foo".into()),
PathKey::namespaced(TRACKED_NAMESPACE, rel_path("foo").into_arc()),
window,
cx,
);
@@ -1488,7 +1489,7 @@ mod tests {
let editor = cx.update_window_entity(&diff, |diff, window, cx| {
diff.move_to_path(
PathKey::namespaced(TRACKED_NAMESPACE, "bar".into()),
PathKey::namespaced(TRACKED_NAMESPACE, rel_path("bar").into_arc()),
window,
cx,
);
crates/multi_buffer/src/multi_buffer.rs
+17 -8
View File
@@ -49,7 +49,7 @@ use text::{
subscription::{Subscription, Topic},
};
use theme::SyntaxTheme;
use util::post_inc;
use util::{post_inc, rel_path::RelPath};
const NEWLINES: &[u8] = &[b'\n'; u8::MAX as usize];
@@ -161,24 +161,33 @@ impl MultiBufferDiffHunk {
#[derive(PartialEq, Eq, Ord, PartialOrd, Clone, Hash, Debug)]
pub struct PathKey {
namespace: u32,
path: Arc<str>,
namespace: Option<u64>,
path: Arc<RelPath>,
}
impl PathKey {
pub fn namespaced(namespace: u32, path: Arc<str>) -> Self {
Self { namespace, path }
pub fn namespaced(namespace: u64, path: Arc<RelPath>) -> Self {
Self {
namespace: Some(namespace),
path,
}
}
pub fn for_buffer(buffer: &Entity<Buffer>, cx: &App) -> Self {
if let Some(file) = buffer.read(cx).file() {
Self::namespaced(1, file.full_path(cx).to_string_lossy().into_owned().into())
Self::namespaced(file.worktree_id(cx).to_proto(), file.path().clone())
} else {
Self::namespaced(0, buffer.entity_id().to_string().into())
Self {
namespace: None,
path: RelPath::unix(&buffer.entity_id().to_string())
.unwrap()
.into_arc(),
}
}
}
pub fn path(&self) -> &Arc<str> {
#[cfg(any(test, feature = "test-support"))]
pub fn path(&self) -> &Arc<RelPath> {
&self.path
}
}
crates/multi_buffer/src/multi_buffer_tests.rs
+5 -4
View File
@@ -8,6 +8,7 @@ use rand::prelude::*;
use settings::SettingsStore;
use std::env;
use util::RandomCharIter;
use util::rel_path::rel_path;
use util::test::sample_text;
#[ctor::ctor]
@@ -1524,7 +1525,7 @@ fn test_set_excerpts_for_buffer_ordering(cx: &mut TestAppContext) {
cx,
)
});
let path1: PathKey = PathKey::namespaced(0, "/".into());
let path1: PathKey = PathKey::namespaced(0, rel_path("root").into_arc());
let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite));
multibuffer.update(cx, |multibuffer, cx| {
@@ -1619,7 +1620,7 @@ fn test_set_excerpts_for_buffer(cx: &mut TestAppContext) {
cx,
)
});
let path1: PathKey = PathKey::namespaced(0, "/".into());
let path1: PathKey = PathKey::namespaced(0, rel_path("root").into_arc());
let buf2 = cx.new(|cx| {
Buffer::local(
indoc! {
@@ -1638,7 +1639,7 @@ fn test_set_excerpts_for_buffer(cx: &mut TestAppContext) {
cx,
)
});
let path2 = PathKey::namespaced(1, "/".into());
let path2 = PathKey::namespaced(1, rel_path("root").into_arc());
let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite));
multibuffer.update(cx, |multibuffer, cx| {
@@ -1815,7 +1816,7 @@ fn test_set_excerpts_for_buffer_rename(cx: &mut TestAppContext) {
cx,
)
});
let path: PathKey = PathKey::namespaced(0, "/".into());
let path: PathKey = PathKey::namespaced(0, rel_path("root").into_arc());
let buf2 = cx.new(|cx| {
Buffer::local(
indoc! {