feat: add gf keymap and unstaged/staged diff support#1
Conversation
- Add :Difft with no args to show unstaged changes (working tree vs index) - Add :Difft --staged to show staged changes (index vs HEAD) - For jj: :Difft with no args shows uncommitted changes (working copy vs @) - Update documentation with new usage examples
- Add 'gf' keymap (configurable) to jump from diff view to editable buffer - Opens file in previous tabpage if exists, otherwise creates new tab - Preserves cursor position (line and column) - Only works from right pane (new/working version of the file) - For filler lines, jumps to nearest non-filler line - Expose aligned_lines from Rust to Lua for accurate line number mapping
- Document :Difft (unstaged), :Difft --staged commands with examples - Document gf keymap for jumping to file from diff view - Add goto_file to keymaps configuration example - Add tests for aligned_lines in created, deleted, and changed files
clabby
left a comment
There was a problem hiding this comment.
Awesome feature! Some consolidation to do, but glad to merge once addressed.
lua/difftastic-nvim/init.lua
Outdated
| -- Only works from right pane (new version) | ||
| if current_win ~= state.right_win then | ||
| return | ||
| end |
There was a problem hiding this comment.
We should also exclude the file tree window, I think.
src/lib.rs
Outdated
| .map(|file| { | ||
| let file_stats = stats.get(&file.path).copied(); | ||
| let old_lines = into_lines(jj_file_content("@", &file.path)); | ||
| let new_lines = into_lines(working_tree_content(&file.path)); |
There was a problem hiding this comment.
We probably want a separate function for jj that uses jj root. Most people colocate their jj repos nowadays, but this is a bit fragile.
src/lib.rs
Outdated
| /// Gets diff stats for unstaged changes (working tree vs index). | ||
| fn git_diff_stats_unstaged() -> FileStats { | ||
| let output = Command::new("git") | ||
| .args(["diff", "--numstat"]) | ||
| .output() | ||
| .ok(); | ||
|
|
||
| let Some(output) = output.filter(|o| o.status.success()) else { | ||
| return HashMap::new(); | ||
| }; | ||
|
|
||
| String::from_utf8_lossy(&output.stdout) | ||
| .lines() | ||
| .filter_map(|line| { | ||
| let mut parts = line.split('\t'); | ||
| let add = parts.next()?.parse().ok()?; | ||
| let del = parts.next()?.parse().ok()?; | ||
| let path = parts.next()?; | ||
| Some((PathBuf::from(path), (add, del))) | ||
| }) | ||
| .collect() | ||
| } | ||
|
|
||
| /// Gets diff stats for staged changes (index vs HEAD). | ||
| fn git_diff_stats_staged() -> FileStats { | ||
| let output = Command::new("git") | ||
| .args(["diff", "--cached", "--numstat"]) | ||
| .output() | ||
| .ok(); | ||
|
|
||
| let Some(output) = output.filter(|o| o.status.success()) else { | ||
| return HashMap::new(); | ||
| }; | ||
|
|
||
| String::from_utf8_lossy(&output.stdout) | ||
| .lines() | ||
| .filter_map(|line| { | ||
| let mut parts = line.split('\t'); | ||
| let add = parts.next()?.parse().ok()?; | ||
| let del = parts.next()?.parse().ok()?; | ||
| let path = parts.next()?; | ||
| Some((PathBuf::from(path), (add, del))) | ||
| }) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
We can deduplicate these functions, given that they're all parsing --numstat. If we squash these into git_diff_stats and change that function to accept an optional slice of additional arguments, that'd be great.
src/lib.rs
Outdated
| /// Runs difftastic for unstaged changes (working tree vs index). | ||
| /// For git: compares working tree to index | ||
| /// For jj: compares working copy to current commit | ||
| fn run_diff_unstaged(lua: &Lua, vcs: String) -> LuaResult<LuaTable> { | ||
| let files = match vcs.as_str() { | ||
| "git" => run_git_diff_unstaged(), | ||
| _ => run_jj_diff_uncommitted(), | ||
| } | ||
| .map_err(LuaError::RuntimeError)?; | ||
|
|
||
| let stats = if vcs == "git" { | ||
| git_diff_stats_unstaged() | ||
| } else { | ||
| jj_diff_stats_uncommitted() | ||
| }; | ||
|
|
||
| let display_files: Vec<_> = if vcs == "git" { | ||
| files | ||
| .into_par_iter() | ||
| .map(|file| { | ||
| let file_stats = stats.get(&file.path).copied(); | ||
| // For unstaged: old = index, new = working tree | ||
| let old_lines = into_lines(git_index_content(&file.path)); | ||
| let new_lines = into_lines(working_tree_content(&file.path)); | ||
| processor::process_file(file, old_lines, new_lines, file_stats) | ||
| }) | ||
| .collect() | ||
| } else { | ||
| // For jj, use the current revision as old and working copy content as new | ||
| files | ||
| .into_par_iter() | ||
| .map(|file| { | ||
| let file_stats = stats.get(&file.path).copied(); | ||
| let old_lines = into_lines(jj_file_content("@", &file.path)); | ||
| let new_lines = into_lines(working_tree_content(&file.path)); | ||
| processor::process_file(file, old_lines, new_lines, file_stats) | ||
| }) | ||
| .collect() | ||
| }; | ||
|
|
||
| let files_table = lua.create_table()?; | ||
| for (i, file) in display_files.into_iter().enumerate() { | ||
| files_table.set(i + 1, file.into_lua(lua)?)?; | ||
| } | ||
|
|
||
| let result = lua.create_table()?; | ||
| result.set("files", files_table)?; | ||
| Ok(result) | ||
| } | ||
|
|
||
| /// Runs difftastic for staged changes (index vs HEAD). | ||
| /// Only supported for git. For jj, this falls back to showing @ changes. | ||
| fn run_diff_staged(lua: &Lua, vcs: String) -> LuaResult<LuaTable> { | ||
| let files = match vcs.as_str() { | ||
| "git" => run_git_diff_staged(), | ||
| _ => { | ||
| // jj doesn't have a staging area concept, so show current revision | ||
| run_jj_diff("@") | ||
| } | ||
| } | ||
| .map_err(LuaError::RuntimeError)?; | ||
|
|
||
| let stats = if vcs == "git" { | ||
| git_diff_stats_staged() | ||
| } else { | ||
| jj_diff_stats("@") | ||
| }; | ||
|
|
||
| let display_files: Vec<_> = if vcs == "git" { | ||
| files | ||
| .into_par_iter() | ||
| .map(|file| { | ||
| let file_stats = stats.get(&file.path).copied(); | ||
| // For staged: old = HEAD, new = index | ||
| let old_lines = into_lines(git_file_content("HEAD", &file.path)); | ||
| let new_lines = into_lines(git_index_content(&file.path)); | ||
| processor::process_file(file, old_lines, new_lines, file_stats) | ||
| }) | ||
| .collect() | ||
| } else { | ||
| // For jj, show @ revision | ||
| files | ||
| .into_par_iter() | ||
| .map(|file| { | ||
| let file_stats = stats.get(&file.path).copied(); | ||
| let old_lines = into_lines(jj_file_content("@-", &file.path)); | ||
| let new_lines = into_lines(jj_file_content("@", &file.path)); | ||
| processor::process_file(file, old_lines, new_lines, file_stats) | ||
| }) | ||
| .collect() | ||
| }; | ||
|
|
||
| let files_table = lua.create_table()?; | ||
| for (i, file) in display_files.into_iter().enumerate() { | ||
| files_table.set(i + 1, file.into_lua(lua)?)?; | ||
| } | ||
|
|
||
| let result = lua.create_table()?; | ||
| result.set("files", files_table)?; | ||
| Ok(result) | ||
| } |
There was a problem hiding this comment.
Similarly, there's a lot of duplication here. We can expose run_diff, run_diff_unstaged, and run_diff_staged to the Lua module, though we should be able to cover everything with a single run_diff function that takes a DiffMode enum?
src/lib.rs
Outdated
| /// Runs difftastic via git for unstaged changes (working tree vs index). | ||
| /// Executes `git diff` with no arguments. | ||
| fn run_git_diff_unstaged() -> Result<Vec<difftastic::DifftFile>, String> { | ||
| let output = Command::new("git") | ||
| .args(["-c", "diff.external=difft", "diff"]) | ||
| .env("DFT_DISPLAY", "json") | ||
| .env("DFT_UNSTABLE", "yes") | ||
| .output() | ||
| .map_err(|e| format!("Failed to run git: {e}"))?; | ||
|
|
||
| if !output.status.success() { | ||
| let stderr = String::from_utf8_lossy(&output.stderr); | ||
| return Err(format!("git command failed: {stderr}")); | ||
| } | ||
|
|
||
| difftastic::parse(&String::from_utf8_lossy(&output.stdout)) | ||
| .map_err(|e| format!("Failed to parse difftastic JSON: {e}")) | ||
| } | ||
|
|
||
| /// Runs difftastic via git for staged changes (index vs HEAD). | ||
| /// Executes `git diff --cached`. | ||
| fn run_git_diff_staged() -> Result<Vec<difftastic::DifftFile>, String> { | ||
| let output = Command::new("git") | ||
| .args(["-c", "diff.external=difft", "diff", "--cached"]) | ||
| .env("DFT_DISPLAY", "json") | ||
| .env("DFT_UNSTABLE", "yes") | ||
| .output() | ||
| .map_err(|e| format!("Failed to run git: {e}"))?; | ||
|
|
||
| if !output.status.success() { | ||
| let stderr = String::from_utf8_lossy(&output.stderr); | ||
| return Err(format!("git command failed: {stderr}")); | ||
| } | ||
|
|
||
| difftastic::parse(&String::from_utf8_lossy(&output.stdout)) | ||
| .map_err(|e| format!("Failed to parse difftastic JSON: {e}")) | ||
| } |
There was a problem hiding this comment.
Likewise. We should be able to have a single run_git_diff function that takes a DiffMode enum rather than duplicating the logic.
lua/difftastic-nvim/init.lua
Outdated
| local current_tab = vim.api.nvim_get_current_tabpage() | ||
| local tabs = vim.api.nvim_list_tabpages() | ||
| local target_tab = nil | ||
|
|
||
| for i, tab in ipairs(tabs) do | ||
| if tab == current_tab and i > 1 then | ||
| target_tab = tabs[i - 1] | ||
| break | ||
| end | ||
| end |
There was a problem hiding this comment.
We should be able to simplify this a bit w/ tabpagenr:
| local current_tab = vim.api.nvim_get_current_tabpage() | |
| local tabs = vim.api.nvim_list_tabpages() | |
| local target_tab = nil | |
| for i, tab in ipairs(tabs) do | |
| if tab == current_tab and i > 1 then | |
| target_tab = tabs[i - 1] | |
| break | |
| end | |
| end | |
| local tabs = vim.api.nvim_list_tabpages() | |
| local current_idx = vim.fn.tabpagenr() | |
| local target_tab = current_idx > 1 and tabs[current_idx - 1] or nil |
- Add jj_root() for non-colocated jj repos - Consolidate git_diff_stats* into single function with extra_args - Consolidate run_git_diff* into single function with extra_args - Introduce DiffMode enum to unify run_diff implementations - Replace working_tree_content with working_tree_content_for_vcs
|
Cut a new release for the binary @ https://github.com/clabby/difftastic.nvim/releases/tag/v0.0.6 |
I was missing
gffrom diffview too much - I use it heavily to review my changes and quickly jump to edit something. This adds that support.Changes
gfkeymap: Jump from diff view to the file at cursor position (line + column). Opens in previous tab or new tab.:Difft(no args): Show unstaged changes (git) or uncommitted changes (jj):Difft --staged: Show staged changes (git)Tests and docs included.