From d51c32a75aac0db9bff6a4cc24a186ec928f4ac3 Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Thu, 8 Oct 2026 09:46:31 +0200 Subject: [PATCH] Open external editor at the selected diff line --- CHANGELOG.md | 1 + asyncgit/src/sync/diff.rs | 5 + src/app.rs | 30 ++--- src/components/diff.rs | 181 ++++++++++++++++++++++++++++++- src/components/revision_files.rs | 5 +- src/components/status_tree.rs | 7 +- src/popups/commit.rs | 5 +- src/popups/externaleditor.rs | 47 ++++++++ src/queue.rs | 2 +- 9 files changed, 259 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1ca0eaac2..112786f00f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added * support x509 commit signing [[@kaden-l-nelson](https://github.com/kaden-l-nelson)] ([#2514](https://github.com/gitui-org/gitui/issues/2514)) +* open the external editor at the line selected in the diff view (vim, nvim, nano, emacs, helix) [[@davxy](https://github.com/davxy)] ([#1206](https://github.com/gitui-org/gitui/issues/1206)) ### Changed * use [tombi](https://github.com/tombi-toml/tombi) for all toml file formatting diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index c13fc476c7..30e6991f16 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -108,6 +108,8 @@ impl From> for HunkHeader { pub struct Hunk { /// hash of the hunk header pub header_hash: u64, + /// start line of the hunk in the new file, as in the hunk header + pub new_start: u32, /// list of `DiffLine`s pub lines: Vec, } @@ -269,6 +271,7 @@ fn raw_diff_to_file_diff( let mut res = res_cell.borrow_mut(); res.hunks.push(Hunk { header_hash: hash(header), + new_start: header.new_start, lines: lines.clone(), }); res.lines += lines.len(); @@ -553,6 +556,8 @@ mod tests { get_diff(repo_path, "bar.txt", false, None).unwrap(); assert_eq!(res.hunks.len(), 2); + assert_eq!(res.hunks[0].new_start, 1); + assert_eq!(res.hunks[1].new_start, 8); } #[test] diff --git a/src/app.rs b/src/app.rs index ddb157af3f..d484585ad4 100644 --- a/src/app.rs +++ b/src/app.rs @@ -118,7 +118,7 @@ pub struct App { // "Flags" requires_redraw: Cell, - file_to_open: Option, + file_to_open: Option<(String, Option)>, } pub struct Environment { @@ -371,17 +371,19 @@ impl App { } else if let InputEvent::State(polling_state) = ev { self.external_editor_popup.hide(); if matches!(polling_state, InputState::Paused) { - let result = - if let Some(path) = self.file_to_open.take() { - ExternalEditorPopup::open_file_in_editor( - &self.repo.borrow(), - Path::new(&path), - ) - } else { - let changes = - self.status_tab.get_files_changes()?; - self.commit_popup.show_editor(changes) - }; + let result = if let Some((path, line)) = + self.file_to_open.take() + { + ExternalEditorPopup::open_file_in_editor( + &self.repo.borrow(), + Path::new(&path), + line, + ) + } else { + let changes = + self.status_tab.get_files_changes()?; + self.commit_popup.show_editor(changes) + }; if let Err(e) = result { let msg = @@ -815,10 +817,10 @@ impl App { flags.insert(NeedsUpdate::ALL); } } - InternalEvent::OpenExternalEditor(path) => { + InternalEvent::OpenExternalEditor(path, line) => { self.input.set_polling(false); self.external_editor_popup.show()?; - self.file_to_open = path; + self.file_to_open = path.map(|path| (path, line)); flags.insert(NeedsUpdate::COMMANDS); } InternalEvent::Push(branch, push_type, force, delete) => { diff --git a/src/components/diff.rs b/src/components/diff.rs index 04779caada..717e11c8d3 100644 --- a/src/components/diff.rs +++ b/src/components/diff.rs @@ -615,6 +615,27 @@ impl DiffComponent { .unwrap_or_default() } + fn cursor_line_in_new_file(&self) -> Option { + let diff = self.diff.as_ref()?; + let mut row_in_hunk = self.selection.get_end(); + + for hunk in &diff.hunks { + if row_in_hunk < hunk.lines.len() { + let new_file_rows_before = hunk.lines[..row_in_hunk] + .iter() + .filter(|line| line.position.new_lineno.is_some()) + .count(); + let line = hunk.new_start.saturating_add( + u32::try_from(new_file_rows_before).ok()?, + ); + return Some(line.max(1)); + } + row_in_hunk -= hunk.lines.len(); + } + + None + } + fn reset_untracked(&self) { self.queue.push(InternalEvent::ConfirmAction(Action::Reset( ResetItem { @@ -889,9 +910,10 @@ impl Component for DiffComponent { && self.can_edit_file() { self.queue.push( - InternalEvent::OpenExternalEditor(Some( - self.current.path.clone(), - )), + InternalEvent::OpenExternalEditor( + Some(self.current.path.clone()), + self.cursor_line_in_new_file(), + ), ); Ok(EventState::Consumed) } else if key_match( @@ -966,6 +988,7 @@ mod tests { use crate::{ app::Environment, queue::InternalEvent, ui::style::Theme, }; + use asyncgit::sync::diff::Hunk; use crossterm::event::{Event, KeyCode, KeyEvent, KeyModifiers}; use std::io::Write; use std::rc::Rc; @@ -1055,8 +1078,158 @@ mod tests { let event = env.queue.pop(); assert!(matches!( event, - Some(InternalEvent::OpenExternalEditor(Some(path))) + Some(InternalEvent::OpenExternalEditor(Some(path), None)) if path == "src/main.rs" )); } + + fn diff_line( + line_type: DiffLineType, + old_lineno: Option, + new_lineno: Option, + ) -> DiffLine { + DiffLine { + content: "".into(), + line_type, + position: DiffLinePosition { + old_lineno, + new_lineno, + }, + } + } + + // @@ -4,3 +4,3 @@ and @@ -20,2 +20,3 @@ + fn two_hunk_diff() -> FileDiff { + let hunks = vec![ + Hunk { + header_hash: 0, + new_start: 4, + lines: vec![ + diff_line(DiffLineType::Header, None, None), + diff_line(DiffLineType::None, Some(4), Some(4)), + diff_line(DiffLineType::Delete, Some(5), None), + diff_line(DiffLineType::Add, None, Some(5)), + diff_line(DiffLineType::None, Some(6), Some(6)), + ], + }, + Hunk { + header_hash: 1, + new_start: 20, + lines: vec![ + diff_line(DiffLineType::Header, None, None), + diff_line(DiffLineType::None, Some(20), Some(20)), + diff_line(DiffLineType::Add, None, Some(21)), + diff_line(DiffLineType::None, Some(21), Some(22)), + ], + }, + ]; + + FileDiff { + lines: 9, + hunks, + ..FileDiff::default() + } + } + + fn editor_line_on_edit( + file_diff: FileDiff, + selection: Selection, + ) -> Option { + let env = Environment::test_env(); + let mut diff = DiffComponent::new(&env, false); + + diff.focus(true); + diff.current.path = String::from("src/main.rs"); + diff.diff = Some(file_diff); + diff.selection = selection; + + diff.event(&Event::Key(KeyEvent::new( + KeyCode::Char('e'), + KeyModifiers::empty(), + ))) + .unwrap(); + + let Some(InternalEvent::OpenExternalEditor(Some(_), line)) = + env.queue.pop() + else { + panic!("editor not opened"); + }; + line + } + + #[test] + fn diff_component_opens_editor_at_line_of_kept_row() { + let line_at = |row| { + editor_line_on_edit( + two_hunk_diff(), + Selection::Single(row), + ) + }; + + assert_eq!(line_at(1), Some(4)); + assert_eq!(line_at(3), Some(5)); + assert_eq!(line_at(4), Some(6)); + assert_eq!(line_at(7), Some(21)); + assert_eq!(line_at(8), Some(22)); + } + + // A deleted row is not in the file on disk. The editor must show + // the place where it was. + #[test] + fn diff_component_opens_editor_where_deleted_row_was() { + assert_eq!( + editor_line_on_edit( + two_hunk_diff(), + Selection::Single(2) + ), + Some(5) + ); + } + + #[test] + fn diff_component_opens_editor_at_hunk_start_from_header() { + let line_at = |row| { + editor_line_on_edit( + two_hunk_diff(), + Selection::Single(row), + ) + }; + + assert_eq!(line_at(0), Some(4)); + assert_eq!(line_at(5), Some(20)); + } + + #[test] + fn diff_component_opens_editor_at_cursor_end_of_range() { + let line_of = |selection| { + editor_line_on_edit(two_hunk_diff(), selection) + }; + + assert_eq!(line_of(Selection::Multiple(1, 7)), Some(21)); + assert_eq!(line_of(Selection::Multiple(7, 1)), Some(4)); + } + + // git writes `+0,0` for a file with no lines left. Editors have no + // line 0. + #[test] + fn diff_component_opens_editor_at_first_line_of_emptied_file() { + let emptied = FileDiff { + lines: 3, + hunks: vec![Hunk { + header_hash: 0, + new_start: 0, + lines: vec![ + diff_line(DiffLineType::Header, None, None), + diff_line(DiffLineType::Delete, Some(1), None), + diff_line(DiffLineType::Delete, Some(2), None), + ], + }], + ..FileDiff::default() + }; + + assert_eq!( + editor_line_on_edit(emptied, Selection::Single(2)), + Some(1) + ); + } } diff --git a/src/components/revision_files.rs b/src/components/revision_files.rs index 1e15ec086f..28ece1a0a8 100644 --- a/src/components/revision_files.rs +++ b/src/components/revision_files.rs @@ -524,7 +524,10 @@ impl Component for RevisionFilesComponent { // not altering a file inside a revision here self.queue.push(InternalEvent::TabSwitchStatus); self.queue.push( - InternalEvent::OpenExternalEditor(Some(file)), + InternalEvent::OpenExternalEditor( + Some(file), + None, + ), ); return Ok(EventState::Consumed); } diff --git a/src/components/status_tree.rs b/src/components/status_tree.rs index ac4fc9f6a8..3a6c2ab70a 100644 --- a/src/components/status_tree.rs +++ b/src/components/status_tree.rs @@ -509,9 +509,10 @@ impl Component for StatusTreeComponent { { if let Some(status_item) = self.selection_file() { self.queue.push( - InternalEvent::OpenExternalEditor(Some( - status_item.path, - )), + InternalEvent::OpenExternalEditor( + Some(status_item.path), + None, + ), ); } Ok(EventState::Consumed) diff --git a/src/popups/commit.rs b/src/popups/commit.rs index b5dff7677c..da0f8bff6e 100644 --- a/src/popups/commit.rs +++ b/src/popups/commit.rs @@ -189,6 +189,7 @@ impl CommitPopup { ExternalEditorPopup::open_file_in_editor( &self.repo.borrow(), &file_path, + None, )?; let mut message = String::new(); @@ -587,7 +588,9 @@ impl Component for CommitPopup { self.key_config.keys.open_commit_editor, ) { self.queue.push( - InternalEvent::OpenExternalEditor(None), + InternalEvent::OpenExternalEditor( + None, None, + ), ); self.hide(); true diff --git a/src/popups/externaleditor.rs b/src/popups/externaleditor.rs index 52a7327b13..367dc77631 100644 --- a/src/popups/externaleditor.rs +++ b/src/popups/externaleditor.rs @@ -27,6 +27,20 @@ use scopeguard::defer; use std::ffi::OsStr; use std::{env, io, path::Path, process::Command}; +const EDITORS_WITH_LINE_ARGUMENT: [&str; 7] = + ["vim", "nvim", "nano", "emacs", "emacsclient", "hx", "helix"]; + +fn editor_line_argument( + command: &str, + line: Option, +) -> Option { + let line = line?; + let editor = Path::new(command).file_stem()?.to_str()?; + EDITORS_WITH_LINE_ARGUMENT + .contains(&editor) + .then(|| format!("+{line}")) +} + /// pub struct ExternalEditorPopup { visible: bool, @@ -48,6 +62,7 @@ impl ExternalEditorPopup { pub fn open_file_in_editor( repo: &RepoPath, path: &Path, + line: Option, ) -> Result<()> { let work_dir = repo_work_dir(repo)?; @@ -104,9 +119,12 @@ impl ExternalEditorPopup { let remainder_str = echars.collect::(); let remainder = remainder_str.split_whitespace(); + let line_argument = editor_line_argument(&command, line); + let mut args: Vec<&OsStr> = remainder.map(OsStr::new).collect(); + args.extend(line_argument.as_deref().map(OsStr::new)); args.push(path.as_os_str()); Command::new(command.clone()) @@ -185,3 +203,32 @@ impl Component for ExternalEditorPopup { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::editor_line_argument; + + #[test] + fn line_argument_for_known_editor() { + assert_eq!( + editor_line_argument("hx", Some(12)), + Some(String::from("+12")) + ); + assert_eq!( + editor_line_argument("/usr/bin/nvim", Some(3)), + Some(String::from("+3")) + ); + } + + // busybox vi and VS Code read `+12` as a file name. + #[test] + fn no_line_argument_for_other_editor() { + assert_eq!(editor_line_argument("vi", Some(12)), None); + assert_eq!(editor_line_argument("code", Some(12)), None); + } + + #[test] + fn no_line_argument_without_line() { + assert_eq!(editor_line_argument("hx", None), None); + } +} diff --git a/src/queue.rs b/src/queue.rs index bbf873ad50..b8c8727150 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -121,7 +121,7 @@ pub enum InternalEvent { /// SelectBranch, /// - OpenExternalEditor(Option), + OpenExternalEditor(Option, Option), /// Push(String, PushType, bool, bool), ///