Skip to content

Commit 38b6910

Browse files
committed
feat(diff): open diff view at file:line on either side
Both entry points for the two-file diff view (CLI `--diff` and the `:diff-open` command) already parsed the optional `file:line` or `file:line:col` suffix via `parse_file`, but discarded the position. Opening `hx --diff a.rs:42 b.rs:58` landed both panes at line 1. Keep the position and apply it after each `editor.open`, using the same pos_at_coords + set_selection + align_view pattern that the non-diff file-open loop uses. In application.rs the work goes through a small `apply_diff_positions` helper so both sides share one code path. Integration tests cover the four cases that matter: - `:diff-open a:3 b:5` positions both panes - `:diff-open a:3 b` positions one pane, leaves the other at the top - `:diff-open a b` with no suffixes keeps both at line 1 (regression guard) - `hx --diff a:3 b:5` via `AppBuilder::with_diff` proves the CLI branch lands each cursor in its own pane The CLI test was mutation-checked by commenting out both `apply_diff_positions` calls and confirming the assertion fails.
1 parent 13ac3f4 commit 38b6910

6 files changed

Lines changed: 241 additions & 8 deletions

File tree

book/src/generated/typable-cmd.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
| `:pipe`, `:\|` | Pipe each selection to the shell command. |
8888
| `:pipe-to` | Pipe each selection to the shell command, ignoring output. |
8989
| `:run-shell-command`, `:sh`, `:!` | Run a shell command |
90-
| `:diff-open`, `:diffs` | Open two files side-by-side in diff mode with aligned hunks. |
90+
| `:diff-open`, `:diffs` | Open two files side-by-side in diff mode with aligned hunks. Each path accepts a file:line or file:line:col suffix. |
9191
| `:reset-diff-change`, `:diffget`, `:diffg` | In a diff session: pull changes from the partner buffer. Outside a diff session: reset the diff change at the cursor position to the VCS base. |
9292
| `:diff-put`, `:diffput`, `:diffp` | In a diff session: push changes from the current buffer to the partner buffer at the cursor position. |
9393
| `:diff-off`, `:diffoff` | End the diff session for the current view. Both views continue as independent buffers. |

helix-term/src/application.rs

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use arc_swap::{access::Map, ArcSwap};
22
use futures_util::Stream;
3-
use helix_core::{diagnostic::Severity, pos_at_coords, syntax, Range, Selection};
3+
use helix_core::{diagnostic::Severity, pos_at_coords, syntax, Position, Range, Selection};
44
use helix_lsp::{
55
lsp::{self, notification::Notification},
66
util::lsp_range_to_range,
@@ -81,6 +81,27 @@ pub struct Application {
8181
theme_mode: Option<theme::Mode>,
8282
}
8383

84+
fn apply_diff_positions(
85+
editor: &mut Editor,
86+
view_id: helix_view::ViewId,
87+
doc_id: helix_view::DocumentId,
88+
positions: &[Position],
89+
) {
90+
if positions.is_empty() {
91+
return;
92+
}
93+
let doc = doc_mut!(editor, &doc_id);
94+
let selection = positions
95+
.iter()
96+
.map(|coords| Range::point(pos_at_coords(doc.text().slice(..), *coords, true)))
97+
.collect();
98+
doc.set_selection(view_id, selection);
99+
if editor.tree.focus == view_id {
100+
let (view, doc) = current!(editor);
101+
align_view(doc, view, Align::Center);
102+
}
103+
}
104+
84105
#[cfg(feature = "integration")]
85106
fn setup_integration_logging() {
86107
let level = std::env::var("HELIX_LOG_LEVEL")
@@ -163,13 +184,16 @@ impl Application {
163184
if files.len() != 2 {
164185
anyhow::bail!("--diff requires exactly two file paths");
165186
}
166-
let (path_a, _) = &files[0];
167-
let (path_b, _) = &files[1];
187+
let (path_a, pos_a) = &files[0];
188+
let (path_b, pos_b) = &files[1];
168189

169190
let doc_a = editor.open(path_a, Action::VerticalSplit)?;
170191
let view_a = editor.tree.focus;
192+
apply_diff_positions(&mut editor, view_a, doc_a, pos_a);
193+
171194
let doc_b = editor.open(path_b, Action::VerticalSplit)?;
172195
let view_b = editor.tree.focus;
196+
apply_diff_positions(&mut editor, view_b, doc_b, pos_b);
173197

174198
let rope_a = editor.documents[&doc_a].text().clone();
175199
let rope_b = editor.documents[&doc_b].text().clone();

helix-term/src/commands/typed.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2592,16 +2592,28 @@ fn diff_open(cx: &mut compositor::Context, args: Args, event: PromptEvent) -> an
25922592
bail!("Expected exactly 2 file paths: :diff-open file1 file2");
25932593
}
25942594

2595-
let (path_a, _) = crate::args::parse_file(&args[0]);
2596-
let (path_b, _) = crate::args::parse_file(&args[1]);
2595+
let (path_a, pos_a) = crate::args::parse_file(&args[0]);
2596+
let (path_b, pos_b) = crate::args::parse_file(&args[1]);
25972597
let path_a = helix_stdx::path::expand_tilde(path_a);
25982598
let path_b = helix_stdx::path::expand_tilde(path_b);
25992599

26002600
let doc_a = cx.editor.open(&path_a, Action::Replace)?;
26012601
let view_a = cx.editor.tree.focus;
2602+
{
2603+
let (view, doc) = current!(cx.editor);
2604+
let sel = Selection::point(pos_at_coords(doc.text().slice(..), pos_a, true));
2605+
doc.set_selection(view.id, sel);
2606+
align_view(doc, view, Align::Center);
2607+
}
26022608

26032609
let doc_b = cx.editor.open(&path_b, Action::VerticalSplit)?;
26042610
let view_b = cx.editor.tree.focus;
2611+
{
2612+
let (view, doc) = current!(cx.editor);
2613+
let sel = Selection::point(pos_at_coords(doc.text().slice(..), pos_b, true));
2614+
doc.set_selection(view.id, sel);
2615+
align_view(doc, view, Align::Center);
2616+
}
26052617

26062618
let rope_a = cx.editor.documents[&doc_a].text().clone();
26072619
let rope_b = cx.editor.documents[&doc_b].text().clone();
@@ -4088,7 +4100,7 @@ pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[
40884100
TypableCommand {
40894101
name: "diff-open",
40904102
aliases: &["diffs"],
4091-
doc: "Open two files side-by-side in diff mode with aligned hunks.",
4103+
doc: "Open two files side-by-side in diff mode with aligned hunks. Each path accepts a file:line or file:line:col suffix.",
40924104
fun: diff_open,
40934105
completer: CommandCompleter::positional(&[completers::filename, completers::filename]),
40944106
signature: Signature {

helix-term/src/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ FLAGS:
7272
--log <file> Specify a file to use for logging
7373
(default file: {})
7474
-V, --version Print version information
75-
--diff, -d Open two files side-by-side in diff mode
75+
--diff, -d Open two files side-by-side in diff mode.
76+
Each path accepts a file:line or file:line:col suffix.
7677
--vsplit Split all given files vertically into different windows
7778
--hsplit Split all given files horizontally into different windows
7879
-w, --working-dir <path> Specify an initial working directory

helix-term/tests/test/diff_mode.rs

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,195 @@ use super::*;
22

33
use helix_stdx::path;
44

5+
#[tokio::test(flavor = "multi_thread")]
6+
async fn test_diff_open_applies_line_numbers_to_both_sides() -> anyhow::Result<()> {
7+
let file1 = helpers::temp_file_with_contents("a1\na2\na3\na4\na5\na6\n")?;
8+
let file2 = helpers::temp_file_with_contents("b1\nb2\nb3\nb4\nb5\nb6\n")?;
9+
10+
let mut app = helpers::AppBuilder::new().build()?;
11+
12+
test_key_sequences(
13+
&mut app,
14+
vec![
15+
(
16+
Some(&format!(
17+
":diff-open {}:3 {}:5<ret>",
18+
file1.path().to_string_lossy(),
19+
file2.path().to_string_lossy()
20+
)),
21+
Some(&|app| {
22+
assert_eq!(1, app.editor.diff_sessions.len());
23+
helpers::assert_status_not_error(&app.editor);
24+
25+
let norm1 = path::normalize(file1.path());
26+
let norm2 = path::normalize(file2.path());
27+
28+
let cursor_line = |target: &std::path::Path| -> usize {
29+
for (view, _focused) in app.editor.tree.views() {
30+
let doc = &app.editor.documents[&view.doc];
31+
if doc.path() == Some(&path::normalize(target)) {
32+
let head = doc.selection(view.id).primary().head;
33+
return doc.text().char_to_line(head);
34+
}
35+
}
36+
panic!("no view for {}", target.display());
37+
};
38+
39+
assert_eq!(
40+
2,
41+
cursor_line(&norm1),
42+
"file1 cursor should be on line 3 (0-indexed 2)"
43+
);
44+
assert_eq!(
45+
4,
46+
cursor_line(&norm2),
47+
"file2 cursor should be on line 5 (0-indexed 4)"
48+
);
49+
}),
50+
),
51+
(Some(":qa!<ret>"), None),
52+
],
53+
true,
54+
)
55+
.await
56+
}
57+
58+
#[tokio::test(flavor = "multi_thread")]
59+
async fn test_diff_open_applies_line_number_on_only_one_side() -> anyhow::Result<()> {
60+
let file1 = helpers::temp_file_with_contents("a1\na2\na3\na4\na5\na6\n")?;
61+
let file2 = helpers::temp_file_with_contents("b1\nb2\nb3\nb4\nb5\nb6\n")?;
62+
63+
let mut app = helpers::AppBuilder::new().build()?;
64+
65+
test_key_sequences(
66+
&mut app,
67+
vec![
68+
(
69+
Some(&format!(
70+
":diff-open {}:3 {}<ret>",
71+
file1.path().to_string_lossy(),
72+
file2.path().to_string_lossy()
73+
)),
74+
Some(&|app| {
75+
assert_eq!(1, app.editor.diff_sessions.len());
76+
helpers::assert_status_not_error(&app.editor);
77+
78+
let norm1 = path::normalize(file1.path());
79+
let norm2 = path::normalize(file2.path());
80+
81+
let cursor_line = |target: &std::path::Path| -> usize {
82+
for (view, _focused) in app.editor.tree.views() {
83+
let doc = &app.editor.documents[&view.doc];
84+
if doc.path() == Some(&path::normalize(target)) {
85+
let head = doc.selection(view.id).primary().head;
86+
return doc.text().char_to_line(head);
87+
}
88+
}
89+
panic!("no view for {}", target.display());
90+
};
91+
92+
assert_eq!(
93+
2,
94+
cursor_line(&norm1),
95+
"file1 with :3 suffix should be on line 3"
96+
);
97+
assert_eq!(
98+
0,
99+
cursor_line(&norm2),
100+
"file2 without suffix should stay at line 1"
101+
);
102+
}),
103+
),
104+
(Some(":qa!<ret>"), None),
105+
],
106+
true,
107+
)
108+
.await
109+
}
110+
111+
#[tokio::test(flavor = "multi_thread")]
112+
async fn test_diff_cli_flag_applies_line_numbers_to_both_sides() -> anyhow::Result<()> {
113+
let file1 = helpers::temp_file_with_contents("a1\na2\na3\na4\na5\na6\n")?;
114+
let file2 = helpers::temp_file_with_contents("b1\nb2\nb3\nb4\nb5\nb6\n")?;
115+
116+
let app = helpers::AppBuilder::new()
117+
.with_diff()
118+
.with_file(file1.path(), Some(helix_core::Position::new(2, 0)))
119+
.with_file(file2.path(), Some(helix_core::Position::new(4, 0)))
120+
.build()?;
121+
122+
let norm1 = path::normalize(file1.path());
123+
let norm2 = path::normalize(file2.path());
124+
125+
let cursor_line = |target: &std::path::Path| -> usize {
126+
for (view, _focused) in app.editor.tree.views() {
127+
let doc = &app.editor.documents[&view.doc];
128+
if doc.path() == Some(&path::normalize(target)) {
129+
let head = doc.selection(view.id).primary().head;
130+
return doc.text().char_to_line(head);
131+
}
132+
}
133+
panic!("no view for {}", target.display());
134+
};
135+
136+
assert_eq!(
137+
1,
138+
app.editor.diff_sessions.len(),
139+
"--diff should create one diff session"
140+
);
141+
assert_eq!(
142+
2,
143+
cursor_line(&norm1),
144+
"file1 cursor should be on line 3 (0-indexed 2)"
145+
);
146+
assert_eq!(
147+
4,
148+
cursor_line(&norm2),
149+
"file2 cursor should be on line 5 (0-indexed 4)"
150+
);
151+
Ok(())
152+
}
153+
154+
#[tokio::test(flavor = "multi_thread")]
155+
async fn test_diff_open_without_line_numbers_stays_at_start() -> anyhow::Result<()> {
156+
let file1 = helpers::temp_file_with_contents("a1\na2\na3\n")?;
157+
let file2 = helpers::temp_file_with_contents("b1\nb2\nb3\n")?;
158+
159+
let mut app = helpers::AppBuilder::new().build()?;
160+
161+
test_key_sequences(
162+
&mut app,
163+
vec![
164+
(
165+
Some(&format!(
166+
":diff-open {} {}<ret>",
167+
file1.path().to_string_lossy(),
168+
file2.path().to_string_lossy()
169+
)),
170+
Some(&|app| {
171+
helpers::assert_status_not_error(&app.editor);
172+
173+
for (view, _focused) in app.editor.tree.views() {
174+
let doc = &app.editor.documents[&view.doc];
175+
if doc.path().is_none() {
176+
continue;
177+
}
178+
let head = doc.selection(view.id).primary().head;
179+
assert_eq!(
180+
0,
181+
doc.text().char_to_line(head),
182+
"unqualified :diff-open must leave both cursors at line 1 (0-indexed 0)"
183+
);
184+
}
185+
}),
186+
),
187+
(Some(":qa!<ret>"), None),
188+
],
189+
true,
190+
)
191+
.await
192+
}
193+
5194
#[tokio::test(flavor = "multi_thread")]
6195
async fn test_diff_open_creates_session() -> anyhow::Result<()> {
7196
let file1 = helpers::temp_file_with_contents("one\ntwo\nthree\n")?;

helix-term/tests/test/helpers.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,13 @@ impl AppBuilder {
356356
self
357357
}
358358

359+
// Remove this attribute once `with_diff` is used outside the diff tests:
360+
#[allow(dead_code)]
361+
pub fn with_diff(mut self) -> Self {
362+
self.args.diff = true;
363+
self
364+
}
365+
359366
// Remove this attribute once `with_config` is used in a test:
360367
#[allow(dead_code)]
361368
pub fn with_config(mut self, mut config: Config) -> Self {

0 commit comments

Comments
 (0)