Skip to content

Commit 1c5b2b8

Browse files
Use buffer line numbers when copying file locations (#60781)
# Objective Fixes #54250 `editor: copy file location` copies the row you're on in the multi-buffer, not the row in the actual file. In a git diff view with collapsed lines it copies the wrong line number. ## Solution It read the rows straight off the selection, which is in multi-buffer coordinates. `ClipboardSelection::for_buffer` already handles this with `range_to_buffer_range`, so I used the same thing. Path and line now come from the same buffer. `range_to_buffer_range` returns nothing when the selection ends in a different buffer than it starts in, which you can do in a git diff view. Rather than copy nothing, it falls back to the cursor's buffer. ## Testing Three tests: a multi-buffer excerpt, a selection dragged across two files, and a plain single-file editor. The multi-buffer ones fail on main; the singleton one passes either way, so normal editors are unaffected. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed `editor: copy file location` copying the wrong line number in multibuffers ([#54250](#54250)). --------- Co-authored-by: Kirill Bulatov <kirill@zed.dev>
1 parent a12ed0f commit 1c5b2b8

2 files changed

Lines changed: 274 additions & 9 deletions

File tree

crates/editor/src/editor.rs

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8659,19 +8659,30 @@ impl Editor {
86598659
cx: &mut Context<Self>,
86608660
) {
86618661
let selection = self.selections.newest::<Point>(&self.display_snapshot(cx));
8662+
let multi_buffer_snapshot = self.buffer.read(cx).snapshot(cx);
8663+
8664+
if let Some(file_location) = maybe!({
8665+
let (buffer, range) = multi_buffer_snapshot
8666+
.range_to_buffer_range(selection.range())
8667+
.or_else(|| {
8668+
// A selection that spans multiple buffers has no single location,
8669+
// so fall back to the buffer the latest cursor is in.
8670+
let (buffer, point) =
8671+
multi_buffer_snapshot.point_to_buffer_point(selection.head())?;
8672+
Some((buffer, point..point))
8673+
})?;
86628674

8663-
let start_line = selection.start.row + 1;
8664-
let end_line = selection.end.row + 1;
8675+
let start_line = range.start.row + 1;
8676+
let end_line = range.end.row + 1;
86658677

8666-
let end_line = if selection.end.column == 0 && end_line > start_line {
8667-
end_line - 1
8668-
} else {
8669-
end_line
8670-
};
8678+
let end_line = if range.end.column == 0 && end_line > start_line {
8679+
end_line - 1
8680+
} else {
8681+
end_line
8682+
};
86718683

8672-
if let Some(file_location) = self.active_buffer(cx).and_then(|buffer| {
86738684
let project = self.project()?.read(cx);
8674-
let file = buffer.read(cx).file()?;
8685+
let file = buffer.file()?;
86758686
let path = file.path().display(project.path_style(cx));
86768687

86778688
let location = if start_line == end_line {

crates/editor/src/editor_tests.rs

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9853,6 +9853,260 @@ async fn test_clipboard_line_numbers_from_multibuffer(cx: &mut TestAppContext) {
98539853
);
98549854
}
98559855

9856+
#[gpui::test]
9857+
async fn test_copy_file_location_from_multibuffer(cx: &mut TestAppContext) {
9858+
init_test(cx, |_| {});
9859+
9860+
let fs = FakeFs::new(cx.executor());
9861+
fs.insert_tree(
9862+
path!("/root"),
9863+
json!({
9864+
"file.txt": "first line\nsecond line\nthird line\nfourth line\nfifth line\n",
9865+
}),
9866+
)
9867+
.await;
9868+
9869+
let project = Project::test(fs, [path!("/root").as_ref()], cx).await;
9870+
9871+
let buffer = project
9872+
.update(cx, |project, cx| {
9873+
project.open_local_buffer(path!("/root/file.txt"), cx)
9874+
})
9875+
.await
9876+
.unwrap();
9877+
9878+
let multibuffer = cx.new(|cx| {
9879+
let mut multibuffer = MultiBuffer::new(ReadWrite);
9880+
multibuffer.set_excerpts_for_path(
9881+
PathKey::sorted(0),
9882+
buffer.clone(),
9883+
[Point::new(2, 0)..Point::new(5, 0)],
9884+
0,
9885+
cx,
9886+
);
9887+
multibuffer
9888+
});
9889+
9890+
let (editor, cx) = cx.add_window_view(|window, cx| {
9891+
build_editor_with_project(project.clone(), multibuffer, window, cx)
9892+
});
9893+
9894+
editor.update_in(cx, |editor, window, cx| {
9895+
editor.change_selections(Default::default(), window, cx, |selections| {
9896+
selections.select_ranges([Point::new(0, 0)..Point::new(0, 0)]);
9897+
});
9898+
editor.copy_file_location(&CopyFileLocation, window, cx);
9899+
});
9900+
assert_eq!(
9901+
cx.read_from_clipboard().and_then(|item| item.text()),
9902+
Some("file.txt:3".to_string()),
9903+
"cursor on the first excerpt row should report its line in the original file"
9904+
);
9905+
9906+
editor.update_in(cx, |editor, window, cx| {
9907+
editor.change_selections(Default::default(), window, cx, |selections| {
9908+
selections.select_ranges([Point::new(1, 0)..Point::new(2, 0)]);
9909+
});
9910+
editor.copy_file_location(&CopyFileLocation, window, cx);
9911+
});
9912+
assert_eq!(
9913+
cx.read_from_clipboard().and_then(|item| item.text()),
9914+
Some("file.txt:4".to_string()),
9915+
"a selection ending at the start of a row should not include that row"
9916+
);
9917+
9918+
editor.update_in(cx, |editor, window, cx| {
9919+
editor.change_selections(Default::default(), window, cx, |selections| {
9920+
selections.select_ranges([Point::new(0, 0)..Point::new(2, 3)]);
9921+
});
9922+
editor.copy_file_location(&CopyFileLocation, window, cx);
9923+
});
9924+
assert_eq!(
9925+
cx.read_from_clipboard().and_then(|item| item.text()),
9926+
Some("file.txt:3-5".to_string()),
9927+
"a multi-row selection should report the original file's line range"
9928+
);
9929+
}
9930+
9931+
#[gpui::test]
9932+
async fn test_copy_file_location_across_buffers(cx: &mut TestAppContext) {
9933+
init_test(cx, |_| {});
9934+
9935+
let fs = FakeFs::new(cx.executor());
9936+
fs.insert_tree(
9937+
path!("/root"),
9938+
json!({
9939+
"one.txt": "one\ntwo\nthree\n",
9940+
"two.txt": "four\nfive\nsix\n",
9941+
}),
9942+
)
9943+
.await;
9944+
9945+
let project = Project::test(fs, [path!("/root").as_ref()], cx).await;
9946+
9947+
let buffer_1 = project
9948+
.update(cx, |project, cx| {
9949+
project.open_local_buffer(path!("/root/one.txt"), cx)
9950+
})
9951+
.await
9952+
.unwrap();
9953+
let buffer_2 = project
9954+
.update(cx, |project, cx| {
9955+
project.open_local_buffer(path!("/root/two.txt"), cx)
9956+
})
9957+
.await
9958+
.unwrap();
9959+
9960+
let multibuffer = cx.new(|cx| {
9961+
let mut multibuffer = MultiBuffer::new(ReadWrite);
9962+
multibuffer.set_excerpts_for_path(
9963+
PathKey::sorted(0),
9964+
buffer_1,
9965+
[Point::new(1, 0)..Point::new(3, 0)],
9966+
0,
9967+
cx,
9968+
);
9969+
multibuffer.set_excerpts_for_path(
9970+
PathKey::sorted(1),
9971+
buffer_2,
9972+
[Point::new(1, 0)..Point::new(3, 0)],
9973+
0,
9974+
cx,
9975+
);
9976+
multibuffer
9977+
});
9978+
9979+
let (editor, cx) = cx.add_window_view(|window, cx| {
9980+
build_editor_with_project(project.clone(), multibuffer, window, cx)
9981+
});
9982+
9983+
// The multi-buffer reads "two\nthree\n\nfive\nsix\n", so row 3 is the first
9984+
// row of the second buffer's excerpt. Select from the first excerpt into it.
9985+
editor.update_in(cx, |editor, window, cx| {
9986+
editor.change_selections(Default::default(), window, cx, |selections| {
9987+
selections.select_ranges([Point::new(0, 0)..Point::new(3, 2)]);
9988+
});
9989+
editor.copy_file_location(&CopyFileLocation, window, cx);
9990+
});
9991+
assert_eq!(
9992+
cx.read_from_clipboard().and_then(|item| item.text()),
9993+
Some("two.txt:2".to_string()),
9994+
"a selection spanning buffers should report the location of the cursor"
9995+
);
9996+
9997+
editor.update_in(cx, |editor, window, cx| {
9998+
editor.change_selections(Default::default(), window, cx, |selections| {
9999+
selections.select_ranges([Point::new(3, 2)..Point::new(0, 0)]);
10000+
});
10001+
editor.copy_file_location(&CopyFileLocation, window, cx);
10002+
});
10003+
assert_eq!(
10004+
cx.read_from_clipboard().and_then(|item| item.text()),
10005+
Some("one.txt:2".to_string()),
10006+
"a reversed selection spanning buffers should report the cursor's location in the first buffer"
10007+
);
10008+
}
10009+
10010+
#[gpui::test]
10011+
async fn test_copy_file_location_with_deleted_hunk(cx: &mut TestAppContext) {
10012+
init_test(cx, |_| {});
10013+
let mut cx = EditorTestContext::new(cx).await;
10014+
10015+
cx.set_state("ˇaaa\nbbb\nccc");
10016+
cx.set_head_text("aaa\nXXX\nbbb\nccc");
10017+
cx.run_until_parked();
10018+
cx.update_editor(|editor, window, cx| {
10019+
editor.expand_all_diff_hunks(&Default::default(), window, cx);
10020+
});
10021+
cx.run_until_parked();
10022+
10023+
// Multi-buffer rows: 0 "aaa", 1 "XXX" (deleted hunk), 2 "bbb", 3 "ccc"
10024+
cx.update_editor(|editor, window, cx| {
10025+
editor.change_selections(Default::default(), window, cx, |selections| {
10026+
selections.select_ranges([Point::new(0, 0)..Point::new(2, 2)]);
10027+
});
10028+
editor.copy_file_location(&CopyFileLocation, window, cx);
10029+
});
10030+
assert_eq!(
10031+
cx.read_from_clipboard().and_then(|item| item.text()),
10032+
Some("file:2".to_string()),
10033+
"a selection crossing a deleted hunk should report the location of the cursor"
10034+
);
10035+
10036+
cx.update_editor(|editor, window, cx| {
10037+
editor.change_selections(Default::default(), window, cx, |selections| {
10038+
selections.select_ranges([Point::new(0, 0)..Point::new(0, 0)]);
10039+
});
10040+
editor.copy_file_location(&CopyFileLocation, window, cx);
10041+
});
10042+
assert_eq!(
10043+
cx.read_from_clipboard().and_then(|item| item.text()),
10044+
Some("file:1".to_string())
10045+
);
10046+
10047+
cx.update_editor(|editor, window, cx| {
10048+
editor.change_selections(Default::default(), window, cx, |selections| {
10049+
selections.select_ranges([Point::new(1, 1)..Point::new(1, 1)]);
10050+
});
10051+
editor.copy_file_location(&CopyFileLocation, window, cx);
10052+
});
10053+
assert_eq!(
10054+
cx.read_from_clipboard().and_then(|item| item.text()),
10055+
Some("file:1".to_string()),
10056+
"a cursor inside a deleted hunk has no file location, so the clipboard is unchanged"
10057+
);
10058+
}
10059+
10060+
#[gpui::test]
10061+
async fn test_copy_file_location_in_singleton_buffer(cx: &mut TestAppContext) {
10062+
init_test(cx, |_| {});
10063+
10064+
let fs = FakeFs::new(cx.executor());
10065+
fs.insert_tree(
10066+
path!("/root"),
10067+
json!({
10068+
"file.txt": "first line\nsecond line\nthird line\n",
10069+
}),
10070+
)
10071+
.await;
10072+
10073+
let project = Project::test(fs, [path!("/root").as_ref()], cx).await;
10074+
10075+
let buffer = project
10076+
.update(cx, |project, cx| {
10077+
project.open_local_buffer(path!("/root/file.txt"), cx)
10078+
})
10079+
.await
10080+
.unwrap();
10081+
let multibuffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx));
10082+
10083+
let (editor, cx) = cx.add_window_view(|window, cx| {
10084+
build_editor_with_project(project.clone(), multibuffer, window, cx)
10085+
});
10086+
10087+
editor.update_in(cx, |editor, window, cx| {
10088+
editor.change_selections(Default::default(), window, cx, |selections| {
10089+
selections.select_ranges([Point::new(1, 0)..Point::new(1, 0)]);
10090+
});
10091+
editor.copy_file_location(&CopyFileLocation, window, cx);
10092+
});
10093+
assert_eq!(
10094+
cx.read_from_clipboard().and_then(|item| item.text()),
10095+
Some("file.txt:2".to_string())
10096+
);
10097+
10098+
editor.update_in(cx, |editor, window, cx| {
10099+
editor.change_selections(Default::default(), window, cx, |selections| {
10100+
selections.select_ranges([Point::new(0, 0)..Point::new(2, 4)]);
10101+
});
10102+
editor.copy_file_location(&CopyFileLocation, window, cx);
10103+
});
10104+
assert_eq!(
10105+
cx.read_from_clipboard().and_then(|item| item.text()),
10106+
Some("file.txt:1-3".to_string())
10107+
);
10108+
}
10109+
985610110
#[gpui::test]
985710111
async fn test_paste_multiline(cx: &mut TestAppContext) {
985810112
init_test(cx, |_| {});

0 commit comments

Comments
 (0)