diff --git a/Cargo.lock b/Cargo.lock index 3ba31dc3..b0e5f307 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3343,6 +3343,7 @@ checksum = "43e734407157c3c2034e0258f5e4473ddb361b1e85f95a66690d67264d7cd1da" dependencies = [ "base64", "bytes", + "futures-channel", "futures-core", "futures-util", "http", diff --git a/Cargo.toml b/Cargo.toml index c488d02d..68877669 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ repository = "https://github.com/jnsahaj/lumen" [dependencies] clap = { version = "4.4", features = ["derive", "env"] } -reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] } +reqwest = { version = "0.12", default-features = false, features = ["blocking", "json", "rustls-tls"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" tokio = { version = "1.0", features = ["full"] } diff --git a/README.md b/README.md index 6af76de9..9ad0d04a 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ A fast terminal diff viewer and code review TUI, written in Rust. Review `git diff`, commits, branches, or GitHub PRs side-by-side without leaving your terminal. Ships as a single static Rust binary and stays snappy on multi-thousand-line diffs. - Side-by-side diff viewer with tree-sitter syntax highlighting -- Review GitHub Pull Requests with `lumen diff --pr 123` +- Review GitHub and Azure DevOps Pull Requests with `lumen diff --pr 123` - Annotate selections, hunks, or whole files - Watch mode and stacked-commit review - Optional AI commit messages and change explanations (10+ providers) @@ -52,6 +52,8 @@ Before you begin, ensure you have: 1. `git` installed on your system 2. [fzf](https://github.com/junegunn/fzf) (optional) - Required for `lumen explain --list` command 3. [mdcat](https://github.com/swsnr/mdcat) (optional) - Required for pretty output formatting +4. [GitHub CLI (`gh`)](https://cli.github.com/) (optional) - Required for reviewing GitHub Pull Requests +5. [Azure CLI (`az`)](https://learn.microsoft.com/cli/azure/) (optional) - Required for reviewing Azure DevOps Pull Requests (sign in with `az login`, or set `AZURE_DEVOPS_EXT_PAT`) ### Installation @@ -86,9 +88,10 @@ lumen diff HEAD~1 # View changes between branches lumen diff main..feature/A -# View changes in a GitHub Pull Request +# View changes in a Pull Request (GitHub or Azure DevOps) lumen diff --pr 123 # (--pr is optional) lumen diff https://github.com/owner/repo/pull/123 +lumen diff https://dev.azure.com/org/project/_git/repo/pullrequest/123 # Open the PR associated with the current branch lumen diff --detect-pr diff --git a/src/command/diff/app.rs b/src/command/diff/app.rs index 2af08187..e65dfa73 100644 --- a/src/command/diff/app.rs +++ b/src/command/diff/app.rs @@ -1,4 +1,4 @@ -use std::collections::VecDeque; +use std::collections::{HashSet, VecDeque}; use std::io::{self, IsTerminal, Write}; use std::sync::mpsc::TryRecvError; use std::time::Duration; @@ -36,11 +36,11 @@ fn open_tui_writer() -> io::Result> { } use super::annotation::{AnnotationEditor, AnnotationEditorResult}; +use super::app_mode::AppMode; use super::coordinates::{extract_selected_text, PanelLayout}; -use super::git::{ - get_current_branch, load_file_diffs, load_pr_file_diffs, load_single_commit_diffs, -}; +use super::git::{get_current_branch, load_file_diffs, load_single_commit_diffs}; use super::highlight; +use super::pr_provider::{PrError, ViewedFileSync}; use super::render::{ render_diff, render_empty_state, truncate_path, FilePickerItem, KeyBind, KeyBindSection, Modal, ModalContent, ModalFileStatus, ModalResult, @@ -52,9 +52,7 @@ use super::types::{ SelectionMode, SidebarItem, }; use super::watcher::{setup_watcher, WatchEvent}; -use super::{ - fetch_viewed_files, mark_file_as_viewed_async, unmark_file_as_viewed_async, DiffOptions, PrInfo, -}; +use super::{DiffOptions, PrInfo}; use spinoff::{spinners, Color, Spinner}; use crate::commit_reference::CommitReference; @@ -250,24 +248,32 @@ fn format_annotation_preview(annotation: &super::state::Annotation) -> String { } } -pub fn run_app_with_pr( - options: DiffOptions, - pr_info: PrInfo, - backend: &dyn VcsBackend, -) -> io::Result<()> { - match load_pr_file_diffs(&pr_info) { - Ok(file_diffs) => run_app_internal(options, Some(pr_info), file_diffs, None, backend), - Err(_) => std::process::exit(1), +pub fn run_app_with_pr(options: DiffOptions, pr_info: PrInfo) -> io::Result<()> { + let file_diffs = load_filtered_pr_diffs(&pr_info, options.file.as_deref()) + .map_err(|error| io::Error::other(format!("failed to load PR diffs: {error}")))?; + run_app_internal( + options, + file_diffs, + AppMode::PullRequest { + pr: Box::new(pr_info), + }, + ) +} + +fn load_filtered_pr_diffs( + pr: &PrInfo, + filter: Option<&[String]>, +) -> Result, PrError> { + let mut diffs = pr.load_file_diffs()?; + if let Some(filter) = filter { + diffs.retain(|diff| filter.contains(&diff.filename)); } + Ok(diffs) } -pub fn run_app( - options: DiffOptions, - pr_info: Option, - backend: &dyn VcsBackend, -) -> io::Result<()> { +pub fn run_app(options: DiffOptions, backend: &dyn VcsBackend) -> io::Result<()> { let file_diffs = load_file_diffs(&options, backend); - run_app_internal(options, pr_info, file_diffs, None, backend) + run_app_internal(options, file_diffs, AppMode::Local { backend }) } pub fn run_app_stacked( @@ -278,27 +284,80 @@ pub fn run_app_stacked( // Load the first commit's diff let first_commit = &commits[0]; let file_diffs = load_single_commit_diffs(&first_commit.commit_id, &options.file, backend); - run_app_internal(options, None, file_diffs, Some(commits), backend) + run_app_internal( + options, + file_diffs, + AppMode::Stacked { + backend, + initial_commits: commits, + }, + ) } -/// Sync viewed files from GitHub to local state -fn sync_viewed_files_from_github(pr_info: &PrInfo, state: &mut AppState) { - if let Ok(viewed_paths) = fetch_viewed_files(pr_info) { - state.viewed_files.clear(); - for (idx, diff) in state.file_diffs.iter().enumerate() { - if viewed_paths.contains(&diff.filename) { - state.viewed_files.insert(idx); - } +fn apply_viewed_paths( + state: &mut AppState, + viewed_paths: Option<&HashSet>, +) -> Option { + let viewed_paths = viewed_paths?; + state.viewed_files = state + .file_diffs + .iter() + .enumerate() + .filter_map(|(idx, diff)| viewed_paths.contains(&diff.filename).then_some(idx)) + .collect(); + Some(state.viewed_files.len()) +} + +/// Sync per-file viewed state from the hosting provider into local state. +/// `None` means the provider does not support viewed-file synchronization. +fn sync_viewed_files_from_provider( + state: &mut AppState, + viewed_sync: Option<&ViewedFileSync>, +) -> Result, String> { + let Some(sync) = viewed_sync else { + return Ok(None); + }; + let viewed_paths = sync.viewed_paths().map_err(|error| error.to_string())?; + Ok(apply_viewed_paths(state, Some(&viewed_paths))) +} + +fn sync_viewed_files_on_startup(state: &mut AppState, viewed_sync: Option<&ViewedFileSync>) { + if viewed_sync.is_none() { + return; + } + + let mut spinner = Spinner::new( + spinners::Dots, + format!("Syncing viewed status for {} files", state.file_diffs.len()), + Color::Cyan, + ); + match sync_viewed_files_from_provider(state, viewed_sync) { + Ok(Some(viewed_count)) => { + spinner.success(&format!("{} files marked as viewed", viewed_count)); + } + Ok(None) => { + spinner.fail("Viewed status sync is unavailable"); + } + Err(error) => { + spinner.fail(&format!("Failed to sync viewed status: {error}")); } } } +fn apply_pr_reload( + state: &mut AppState, + result: Result, PrError>, + changed_files: Option<&HashSet>, +) -> Result<(), PrError> { + let file_diffs = result?; + state.reload(file_diffs, changed_files); + Ok(()) +} + fn run_app_internal( options: DiffOptions, - pr_info: Option, file_diffs: Vec, - stacked_commits: Option>, - backend: &dyn VcsBackend, + mut mode: AppMode<'_>, ) -> io::Result<()> { theme::init(options.theme.as_deref()); highlight::init(); @@ -306,13 +365,15 @@ fn run_app_internal( // Initialize state before TUI so we can sync viewed files let mut state = AppState::new(file_diffs, options.focus.as_deref()); state.settings.wrap = options.wrap; - state.set_vcs_name(backend.name()); + state.set_vcs_name(mode.backend().map_or("pr", VcsBackend::name)); // Set diff reference for annotation export context - let diff_ref_str = if let Some(pr) = &pr_info { + let diff_ref_str = if let Some(pr) = mode.pr() { Some(format!( "PR #{} ({}...{})", - pr.number, pr.base_ref, pr.head_ref + pr.number(), + pr.base_ref(), + pr.head_ref() )) } else { options.reference.as_ref().map(|r| match r { @@ -325,20 +386,15 @@ fn run_app_internal( state.set_diff_reference(diff_ref_str); // Initialize stacked mode if commits were provided - if let Some(commits) = stacked_commits { + if let Some(commits) = mode.take_initial_commits() { state.init_stacked_mode(commits); } - // Load viewed files from GitHub on startup in PR mode (before TUI starts) - if let Some(ref pr) = pr_info { - let mut spinner = Spinner::new( - spinners::Dots, - format!("Syncing viewed status for {} files", state.file_diffs.len()), - Color::Cyan, - ); - sync_viewed_files_from_github(pr, &mut state); - let viewed_count = state.viewed_files.len(); - spinner.success(&format!("{} files marked as viewed", viewed_count)); + let mut viewed_sync = mode.pr().and_then(ViewedFileSync::new); + + // Load viewed files from the provider on startup in PR mode (before TUI starts). + if mode.pr().is_some() { + sync_viewed_files_on_startup(&mut state, viewed_sync.as_ref()); } // Now enter TUI mode. Use /dev/tty when stdout is captured so the @@ -357,8 +413,8 @@ fn run_app_internal( let mut terminal = Terminal::new(CrosstermBackend::new(tui_writer))?; - state.watching = options.watch && pr_info.is_none(); - let (mut _watch_handle, mut watch_rx) = if state.watching && pr_info.is_none() { + state.watching = options.watch && !mode.is_pull_request(); + let (mut _watch_handle, mut watch_rx) = if state.watching { if let Some((handle, rx)) = setup_watcher() { (Some(handle), Some(rx)) } else { @@ -374,8 +430,31 @@ fn run_app_internal( let mut pending_watch_event: Option = None; let mut pending_events: VecDeque = VecDeque::new(); let mut send_annotations_on_exit = false; + let branch_fallback = mode + .backend() + .map(get_current_branch) + .unwrap_or_else(|| "unknown".to_string()); 'main: loop { + if let Some(sync) = viewed_sync.as_mut() { + for completion in sync.drain() { + if let Err(error) = completion.result { + let reconciliation = + sync_viewed_files_from_provider(&mut state, Some(sync)).err(); + let reconciliation = reconciliation + .map(|error| format!("\n\nCould not refresh viewed status: {error}")) + .unwrap_or_default(); + active_modal = Some(Modal::info( + "Viewed status update failed", + format!( + "Could not update {}.\n\n{error}{reconciliation}", + completion.path + ), + )); + } + } + } + if let Some(ref rx) = watch_rx { match rx.try_recv() { Ok(event) => { @@ -388,26 +467,46 @@ fn run_app_internal( } if state.needs_reload { - let file_diffs = if let Some(ref pr) = pr_info { - // In PR mode, reload from GitHub - match load_pr_file_diffs(pr) { - Ok(diffs) => diffs, - Err(e) => { - eprintln!("Warning: failed to reload PR diffs: {}", e); - Vec::new() + // Clear this before loading so a provider failure does not retry every frame. + state.needs_reload = false; + match &mode { + AppMode::PullRequest { pr } => { + let changed_files = pending_watch_event + .as_ref() + .map(|event| &event.changed_files); + match apply_pr_reload( + &mut state, + load_filtered_pr_diffs(pr, options.file.as_deref()), + changed_files, + ) { + Ok(()) => { + pending_watch_event.take(); + if let Err(error) = + sync_viewed_files_from_provider(&mut state, viewed_sync.as_ref()) + { + active_modal = Some(Modal::info( + "Viewed status sync failed", + format!( + "The diff was reloaded, but viewed status could not be synced.\n\n{error}" + ), + )); + } + } + Err(error) => { + active_modal = Some(Modal::info( + "Reload failed", + format!( + "Could not reload PR diffs. The current diff and review state were preserved.\n\n{error}" + ), + )); + } } } - } else { - load_file_diffs(&options, backend) - }; - - // Pass changed files to reload so it can unmark them from viewed - let changed_files = pending_watch_event.take().map(|e| e.changed_files); - state.reload(file_diffs, changed_files.as_ref()); - - // Re-sync viewed files from GitHub in PR mode - if let Some(ref pr) = pr_info { - sync_viewed_files_from_github(pr, &mut state); + AppMode::Local { backend } | AppMode::Stacked { backend, .. } => { + let file_diffs = load_file_diffs(&options, *backend); + let changed_files = pending_watch_event.take().map(|event| event.changed_files); + state.reload(file_diffs, changed_files.as_ref()); + } } } @@ -434,12 +533,12 @@ fn run_app_internal( .viewed_hunks .get(&diff.filename) .unwrap_or(&empty_viewed_hunks); - let branch_fallback = get_current_branch(backend); let commit_ref = state.diff_reference.as_deref().unwrap_or(&branch_fallback); let row_offset = std::cell::Cell::new(0usize); let gaps_cell = std::cell::RefCell::new(Vec::new()); let rects_cell = std::cell::RefCell::new(Vec::new()); - let editor_rect_cell: std::cell::Cell> = std::cell::Cell::new(None); + let editor_rect_cell: std::cell::Cell> = + std::cell::Cell::new(None); terminal.draw(|frame| { let (offset, gaps, rects, er) = render_diff( frame, @@ -467,14 +566,14 @@ fn run_app_internal( state.diff_fullscreen, &state.search_state, commit_ref, - pr_info.as_ref(), + mode.pr(), state.focused_hunk, - &hunks, + hunks, state.stacked_mode, state.current_commit(), state.current_commit_index, state.stacked_commits.len(), - &side_by_side, + side_by_side, state.vcs_name, &state.annotations, &state.selection, @@ -948,9 +1047,11 @@ fn run_app_internal( // Left arrow click (first 4 columns to cover " < ") if mouse.column < 4 && state.current_commit_index > 0 { let new_index = state.current_commit_index - 1; - navigate_stacked_commit( - &mut state, new_index, &options, backend, - ); + if let Some(backend) = mode.stacked_backend() { + navigate_stacked_commit( + &mut state, new_index, &options, backend, + ); + } } // Right arrow click (last 4 columns to cover " > ") else if mouse.column >= term_size.width.saturating_sub(4) @@ -958,9 +1059,11 @@ fn run_app_internal( < state.stacked_commits.len().saturating_sub(1) { let new_index = state.current_commit_index + 1; - navigate_stacked_commit( - &mut state, new_index, &options, backend, - ); + if let Some(backend) = mode.stacked_backend() { + navigate_stacked_commit( + &mut state, new_index, &options, backend, + ); + } } } else if state.show_sidebar && mouse.column < sidebar_width @@ -1329,14 +1432,22 @@ fn run_app_internal( && state.current_commit_index < state.stacked_commits.len() - 1 { let new_index = state.current_commit_index + 1; - navigate_stacked_commit(&mut state, new_index, &options, backend); + if let Some(backend) = mode.stacked_backend() { + navigate_stacked_commit( + &mut state, new_index, &options, backend, + ); + } } } // Stacked mode: navigate to previous commit KeyCode::Char('h') if key.modifiers.contains(KeyModifiers::CONTROL) => { if state.stacked_mode && state.current_commit_index > 0 { let new_index = state.current_commit_index - 1; - navigate_stacked_commit(&mut state, new_index, &options, backend); + if let Some(backend) = mode.stacked_backend() { + navigate_stacked_commit( + &mut state, new_index, &options, backend, + ); + } } } KeyCode::Char('d') if key.modifiers.contains(KeyModifiers::CONTROL) => { @@ -1448,7 +1559,7 @@ fn run_app_internal( } } KeyCode::Char('w') => { - if pr_info.is_some() { + if mode.is_pull_request() { // PR mode doesn't support watching } else { state.watching = !state.watching; @@ -1538,13 +1649,8 @@ fn run_app_internal( state.viewed_files.insert(file_idx); } - // Fire off async API call if in PR mode - if let Some(ref pr) = pr_info { - if was_viewed { - unmark_file_as_viewed_async(pr, &filename); - } else { - mark_file_as_viewed_async(pr, &filename); - } + if let Some(sync) = viewed_sync.as_mut() { + sync.set(&filename, !was_viewed); } } SidebarItem::Directory { path, .. } => { @@ -1582,15 +1688,10 @@ fn run_app_internal( } } - // Fire off async API calls if in PR mode - if let Some(ref pr) = pr_info { + if let Some(sync) = viewed_sync.as_mut() { for &idx in &child_indices { let filename = &state.file_diffs[idx].filename; - if all_viewed { - unmark_file_as_viewed_async(pr, filename); - } else { - mark_file_as_viewed_async(pr, filename); - } + sync.set(filename, !all_viewed); } } } @@ -1649,13 +1750,8 @@ fn run_app_internal( } } - // Fire off async API call if in PR mode - if let Some(ref pr) = pr_info { - if was_viewed { - unmark_file_as_viewed_async(pr, &filename); - } else { - mark_file_as_viewed_async(pr, &filename); - } + if let Some(sync) = viewed_sync.as_mut() { + sync.set(&filename, !was_viewed); } } } @@ -1899,10 +1995,8 @@ fn run_app_internal( } KeyCode::Char('e') => { if !state.file_diffs.is_empty() { - let _ = execute!( - terminal.backend_mut(), - PopKeyboardEnhancementFlags - ); + let _ = + execute!(terminal.backend_mut(), PopKeyboardEnhancementFlags); execute!( terminal.backend_mut(), DisableMouseCapture, @@ -1951,23 +2045,18 @@ fn run_app_internal( )?; let _ = execute!( terminal.backend_mut(), - PushKeyboardEnhancementFlags(KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES) + PushKeyboardEnhancementFlags( + KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES + ) ); terminal.clear()?; } } KeyCode::Char('o') => { - if let Some(ref pr) = pr_info { + if let Some(pr) = mode.pr() { if !state.file_diffs.is_empty() { let filename = &state.file_diffs[state.current_file].filename; - let file_url = format!( - "https://github.com/{}/{}/pull/{}/files#diff-{}", - pr.repo_owner, - pr.repo_name, - pr.number, - generate_file_anchor(filename) - ); - let _ = open_url(&file_url); + let _ = open_url(&pr.file_web_url(filename)); } } } @@ -2105,10 +2194,10 @@ fn run_app_internal( key: "h/l or left/right", description: "Scroll horizontally", }, - KeyBind { - key: "w", - description: "Toggle watch mode", - }, + KeyBind { + key: "w", + description: "Toggle watch mode", + }, KeyBind { key: "gg / G", description: "Scroll to top / bottom", @@ -2152,7 +2241,8 @@ fn run_app_internal( }, KeyBind { key: "ctrl+f", - description: "Global fuzzy search (all files, with preview)", + description: + "Global fuzzy search (all files, with preview)", }, KeyBind { key: "n or down", @@ -2237,10 +2327,57 @@ fn open_url(url: &str) -> io::Result<()> { Ok(()) } -fn generate_file_anchor(filename: &str) -> String { - use sha2::{Digest, Sha256}; +#[cfg(test)] +mod tests { + use super::*; + use crate::command::diff::types::FileDiff; + + fn state_with_one_viewed_file() -> AppState { + let mut state = AppState::new( + vec![FileDiff { + filename: "src/lib.rs".to_string(), + old_content: String::new(), + new_content: String::new(), + status: FileStatus::Modified, + is_binary: false, + }], + None, + ); + state.viewed_files.insert(0); + state + } + + #[test] + fn apply_viewed_paths_preserves_local_state_when_unsupported() { + let mut state = state_with_one_viewed_file(); - let mut hasher = Sha256::new(); - hasher.update(filename.as_bytes()); - format!("{:x}", hasher.finalize()) + let result = apply_viewed_paths(&mut state, None); + + assert_eq!((result, state.viewed_files), (None, HashSet::from([0]))); + } + + #[test] + fn apply_viewed_paths_treats_empty_set_as_authoritative() { + let mut state = state_with_one_viewed_file(); + let viewed_paths = HashSet::new(); + + let result = apply_viewed_paths(&mut state, Some(&viewed_paths)); + + assert_eq!((result, state.viewed_files), (Some(0), HashSet::new())); + } + + #[test] + fn failed_pr_reload_preserves_current_state() { + let mut state = state_with_one_viewed_file(); + + let result = apply_pr_reload( + &mut state, + Err(PrError::Other("temporary failure".to_string())), + None, + ); + + assert!(result.is_err()); + assert_eq!(state.file_diffs[0].filename, "src/lib.rs"); + assert_eq!(state.viewed_files, HashSet::from([0])); + } } diff --git a/src/command/diff/app_mode.rs b/src/command/diff/app_mode.rs new file mode 100644 index 00000000..9d091cb3 --- /dev/null +++ b/src/command/diff/app_mode.rs @@ -0,0 +1,51 @@ +use super::PrInfo; +use crate::vcs::{StackedCommitInfo, VcsBackend}; + +pub(super) enum AppMode<'a> { + Local { + backend: &'a dyn VcsBackend, + }, + PullRequest { + pr: Box, + }, + Stacked { + backend: &'a dyn VcsBackend, + initial_commits: Vec, + }, +} + +impl<'a> AppMode<'a> { + pub fn pr(&self) -> Option<&PrInfo> { + match self { + Self::PullRequest { pr } => Some(pr.as_ref()), + Self::Local { .. } | Self::Stacked { .. } => None, + } + } + + pub fn backend(&self) -> Option<&'a dyn VcsBackend> { + match self { + Self::Local { backend } | Self::Stacked { backend, .. } => Some(*backend), + Self::PullRequest { .. } => None, + } + } + + pub fn stacked_backend(&self) -> Option<&'a dyn VcsBackend> { + match self { + Self::Stacked { backend, .. } => Some(*backend), + Self::Local { .. } | Self::PullRequest { .. } => None, + } + } + + pub fn is_pull_request(&self) -> bool { + matches!(self, Self::PullRequest { .. }) + } + + pub fn take_initial_commits(&mut self) -> Option> { + match self { + Self::Stacked { + initial_commits, .. + } => Some(std::mem::take(initial_commits)), + Self::Local { .. } | Self::PullRequest { .. } => None, + } + } +} diff --git a/src/command/diff/git.rs b/src/command/diff/git.rs index 44ec6286..8a957bcf 100644 --- a/src/command/diff/git.rs +++ b/src/command/diff/git.rs @@ -1,23 +1,11 @@ use std::fs; use std::path::Path; -use std::process::Command; -use std::sync::mpsc; -use std::sync::{Arc, Mutex}; -use std::thread; - -use spinoff::{spinners, Color, Spinner}; use super::types::{is_binary_content, FileDiff, FileStatus}; -use super::{DiffOptions, PrInfo}; +use super::DiffOptions; use crate::commit_reference::CommitReference; use crate::vcs::VcsBackend; -/// Max concurrent `gh api` requests when fetching PR file contents. -/// GitHub's documented secondary rate limit caps concurrent requests at 100 -/// (shared across REST+GraphQL); 8 keeps us comfortably under that while -/// still giving a large speedup over serial fetching. -const PR_FETCH_CONCURRENCY: usize = 8; - pub fn get_current_branch(backend: &dyn VcsBackend) -> String { backend .get_current_branch() @@ -87,7 +75,7 @@ pub fn get_changed_files(options: &DiffOptions, backend: &dyn VcsBackend) -> Vec let wt_files = backend.get_working_tree_changed_files().unwrap_or_default(); let mut seen: std::collections::HashSet = std::collections::HashSet::new(); let mut combined: Vec = Vec::new(); - for f in range_files.into_iter().chain(wt_files.into_iter()) { + for f in range_files.into_iter().chain(wt_files) { if seen.insert(f.clone()) { combined.push(f); } @@ -141,6 +129,26 @@ pub fn get_new_content(filename: &str, refs: &DiffRefs, backend: &dyn VcsBackend } } +/// Assemble a `FileDiff` from a filename and its two sides, deriving the file +/// status and binary flag from the contents. +pub fn build_file_diff(filename: String, old_content: String, new_content: String) -> FileDiff { + let status = if old_content.is_empty() && !new_content.is_empty() { + FileStatus::Added + } else if !old_content.is_empty() && new_content.is_empty() { + FileStatus::Deleted + } else { + FileStatus::Modified + }; + let is_binary = is_binary_content(&old_content) || is_binary_content(&new_content); + FileDiff { + filename, + old_content, + new_content, + status, + is_binary, + } +} + pub fn load_file_diffs(options: &DiffOptions, backend: &dyn VcsBackend) -> Vec { let refs = DiffRefs::from_options(options, backend); get_changed_files(options, backend) @@ -148,291 +156,11 @@ pub fn load_file_diffs(options: &DiffOptions, backend: &dyn VcsBackend) -> Vec Result, String> { - let repo_arg = format!("{}/{}", pr_info.repo_owner, pr_info.repo_name); - - let mut spinner = Spinner::new( - spinners::Dots, - format!( - "Fetching file list for {}/{}#{}", - pr_info.repo_owner, pr_info.repo_name, pr_info.number - ), - Color::Cyan, - ); - - // Get PR diff to find changed files - let output = Command::new("gh") - .args([ - "pr", - "diff", - &pr_info.number.to_string(), - "--repo", - &repo_arg, - ]) - .output(); - - let output = match output { - Ok(o) => o, - Err(e) => { - let msg = format!("Failed to run gh pr diff: {}", e); - spinner.fail(&msg); - return Err(msg); - } - }; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - let msg = format!("gh pr diff failed: {}", stderr.trim()); - spinner.fail(&msg); - return Err(msg); - } - - let diff_output = String::from_utf8_lossy(&output.stdout); - let changed_files = parse_changed_files_from_diff(&diff_output); - let n = changed_files.len(); - - if n == 0 { - spinner.success("PR has no changed files"); - return Ok(Vec::new()); - } - - let base_repo = format!("{}/{}", pr_info.base_repo_owner, pr_info.repo_name); - let head_repo = pr_info - .head_repo_owner - .as_ref() - .map(|owner| format!("{}/{}", owner, pr_info.repo_name)) - .unwrap_or_else(|| base_repo.clone()); - - let contents = fetch_pr_file_contents_parallel( - &changed_files, - &base_repo, - &pr_info.base_ref, - &head_repo, - &pr_info.head_ref, - &mut spinner, - ); - - let file_diffs: Vec = changed_files - .into_iter() - .zip(contents.into_iter()) - .map(|(filename, (old_content, new_content))| { - let status = if old_content.is_empty() && !new_content.is_empty() { - FileStatus::Added - } else if !old_content.is_empty() && new_content.is_empty() { - FileStatus::Deleted - } else { - FileStatus::Modified - }; - - let is_binary = - is_binary_content(&old_content) || is_binary_content(&new_content); - FileDiff { - filename, - old_content, - new_content, - status, - is_binary, - } - }) - .collect(); - - spinner.success(&format!("Fetched {} files", n)); - Ok(file_diffs) -} - -#[derive(Clone, Copy)] -enum Side { - Old, - New, -} - -struct FetchTask { - idx: usize, - filename: String, - repo: String, - git_ref: String, - side: Side, -} - -enum FetchEvent { - Started(String), - Finished { - idx: usize, - side: Side, - filename: String, - content: String, - }, -} - -/// Fetch (old, new) contents for every changed file using a bounded worker -/// pool, updating `spinner` with live progress. -fn fetch_pr_file_contents_parallel( - files: &[String], - base_repo: &str, - base_ref: &str, - head_repo: &str, - head_ref: &str, - spinner: &mut Spinner, -) -> Vec<(String, String)> { - let n = files.len(); - let mut tasks: Vec = Vec::with_capacity(2 * n); - for (idx, filename) in files.iter().enumerate() { - tasks.push(FetchTask { - idx, - filename: filename.clone(), - repo: base_repo.to_string(), - git_ref: base_ref.to_string(), - side: Side::Old, - }); - tasks.push(FetchTask { - idx, - filename: filename.clone(), - repo: head_repo.to_string(), - git_ref: head_ref.to_string(), - side: Side::New, - }); - } - // Pop from the back, so process files in listed order. - tasks.reverse(); - - let total = tasks.len(); - let queue = Arc::new(Mutex::new(tasks)); - let (tx, rx) = mpsc::channel::(); - - let worker_count = PR_FETCH_CONCURRENCY.min(total); - let mut handles = Vec::with_capacity(worker_count); - for _ in 0..worker_count { - let queue = Arc::clone(&queue); - let tx = tx.clone(); - handles.push(thread::spawn(move || loop { - let task = { queue.lock().unwrap().pop() }; - let Some(task) = task else { break }; - let _ = tx.send(FetchEvent::Started(task.filename.clone())); - let content = fetch_file_content_from_github(&task.repo, &task.git_ref, &task.filename); - let _ = tx.send(FetchEvent::Finished { - idx: task.idx, - side: task.side, - filename: task.filename, - content, - }); - })); - } - drop(tx); - - let mut contents: Vec<(String, String)> = vec![(String::new(), String::new()); n]; - let mut done = 0usize; - let mut in_flight: Vec = Vec::new(); - let mut last_finished: Option = None; - - while let Ok(ev) = rx.recv() { - match ev { - FetchEvent::Started(name) => { - in_flight.push(name); - } - FetchEvent::Finished { - idx, - side, - filename, - content, - } => { - if let Some(pos) = in_flight.iter().position(|f| f == &filename) { - in_flight.swap_remove(pos); - } - match side { - Side::Old => contents[idx].0 = content, - Side::New => contents[idx].1 = content, - } - done += 1; - last_finished = Some(filename); - } - } - spinner.update_text(format_fetch_progress(done, total, &in_flight, last_finished.as_deref())); - } - - for h in handles { - let _ = h.join(); - } - - contents -} - -fn format_fetch_progress( - done: usize, - total: usize, - in_flight: &[String], - last_finished: Option<&str>, -) -> String { - let current = if let Some(name) = in_flight.last() { - name.as_str() - } else if let Some(name) = last_finished { - name - } else { - "" - }; - if current.is_empty() { - format!("Fetching files [{}/{}]", done, total) - } else { - format!("Fetching files [{}/{}] · {}", done, total, current) - } -} - -fn fetch_file_content_from_github(repo: &str, git_ref: &str, path: &str) -> String { - let api_path = format!("repos/{}/contents/{}?ref={}", repo, path, git_ref); - let output = Command::new("gh") - .args([ - "api", - &api_path, - "-H", - "Accept: application/vnd.github.raw+json", - ]) - .output(); - - match output { - Ok(o) if o.status.success() => String::from_utf8_lossy(&o.stdout).to_string(), - _ => String::new(), - } -} - -fn parse_changed_files_from_diff(diff: &str) -> Vec { - let mut files = Vec::new(); - - for line in diff.lines() { - if line.starts_with("diff --git") { - let parts: Vec<&str> = line.split_whitespace().collect(); - if parts.len() >= 4 { - let b_path = parts[3]; - if let Some(filename) = b_path.strip_prefix("b/") { - files.push(filename.to_string()); - } else { - files.push(b_path.to_string()); - } - } - } - } - - files -} - /// Load file diffs for a single commit (comparing commit to its parent). /// Uses VcsBackend for backend-agnostic file content retrieval. pub fn load_single_commit_diffs( @@ -473,23 +201,7 @@ pub fn load_single_commit_diffs( .get_file_content_at_ref(commit_id, path) .unwrap_or_default(); - let status = if old_content.is_empty() && !new_content.is_empty() { - FileStatus::Added - } else if !old_content.is_empty() && new_content.is_empty() { - FileStatus::Deleted - } else { - FileStatus::Modified - }; - - let is_binary = - is_binary_content(&old_content) || is_binary_content(&new_content); - FileDiff { - filename, - old_content, - new_content, - status, - is_binary, - } + build_file_diff(filename, old_content, new_content) }) .collect() } @@ -597,9 +309,11 @@ mod tests { let backend = crate::vcs::GitBackend::from_cwd().expect("should open repo"); let options = super::super::DiffOptions { - reference: Some(crate::commit_reference::CommitReference::RangeToWorkingTree { - from: "HEAD~1".to_string(), - }), + reference: Some( + crate::commit_reference::CommitReference::RangeToWorkingTree { + from: "HEAD~1".to_string(), + }, + ), pr: None, detect_pr: false, file: None, @@ -637,7 +351,10 @@ mod tests { assert_eq!(base.new_content, "base modified\n"); // committed.txt: old=empty (not in HEAD~1), new=fs content - let committed = diffs.iter().find(|d| d.filename == "committed.txt").unwrap(); + let committed = diffs + .iter() + .find(|d| d.filename == "committed.txt") + .unwrap(); assert_eq!(committed.old_content, ""); assert_eq!(committed.new_content, "committed\n"); diff --git a/src/command/diff/mod.rs b/src/command/diff/mod.rs index c1ce8199..d7b7b108 100644 --- a/src/command/diff/mod.rs +++ b/src/command/diff/mod.rs @@ -1,11 +1,13 @@ mod annotation; mod app; +mod app_mode; mod context; mod coordinates; mod diff_algo; pub mod git; mod global_search; pub mod highlight; +pub mod pr_provider; mod render; mod search; mod state; @@ -15,16 +17,16 @@ pub mod theme; mod types; mod watcher; -use std::collections::HashSet; use std::io; -use std::process::{self, Command}; -use std::thread; +use std::process; use spinoff::{spinners, Color, Spinner}; use crate::commit_reference::CommitReference; use crate::vcs::VcsBackend; +pub use pr_provider::PrInfo; + pub struct DiffOptions { pub reference: Option, pub pr: Option, @@ -38,307 +40,10 @@ pub struct DiffOptions { pub wrap: bool, } -#[derive(Clone)] -pub struct PrInfo { - pub number: u64, - pub node_id: String, - pub repo_owner: String, - pub repo_name: String, - pub base_ref: String, - pub head_ref: String, - pub base_repo_owner: String, - pub head_repo_owner: Option, // None if head repo was deleted (fork deleted) -} - -fn parse_pr_input(input: &str) -> Option<(Option, Option, u64)> { - // Try to parse as a URL first - if input.starts_with("http://") || input.starts_with("https://") { - // Extract PR number and repo info from URL - // Format: https://github.com/owner/repo/pull/123 - let parts: Vec<&str> = input.trim_end_matches('/').split('/').collect(); - if parts.len() >= 2 { - if let Some(pos) = parts.iter().position(|&p| p == "pull") { - if pos + 1 < parts.len() { - if let Ok(num) = parts[pos + 1].parse::() { - // Extract owner and repo - if pos >= 2 { - let owner = parts[pos - 2].to_string(); - let repo = parts[pos - 1].to_string(); - return Some((Some(owner), Some(repo), num)); - } - return Some((None, None, num)); - } - } - } - } - None - } else { - // Try to parse as a PR number - input.parse::().ok().map(|num| (None, None, num)) - } -} - -fn resolve_origin_repo() -> Result { - let output = Command::new("git") - .args(["remote", "get-url", "origin"]) - .output() - .map_err(|e| format!("Failed to run git: {}", e))?; - if !output.status.success() { - return Err( - "Could not determine repository. Set origin remote or use --origin owner/repo" - .to_string(), - ); - } - let url = String::from_utf8_lossy(&output.stdout).trim().to_string(); - let url = url.strip_suffix(".git").unwrap_or(&url); - let path = url - .split("github.com") - .nth(1) - .ok_or_else(|| format!("Origin URL is not a GitHub URL: {}", url))?; - let path = path.trim_start_matches(':').trim_start_matches('/'); - let parts: Vec<&str> = path.split('/').collect(); - if parts.len() >= 2 { - Ok(format!("{}/{}", parts[0], parts[1])) - } else { - Err(format!("Could not parse owner/repo from origin URL: {}", url)) - } -} - -fn fetch_pr_info(pr_input: &str, repo_override: Option<&str>) -> Result { - let (owner, repo, number) = parse_pr_input(pr_input).ok_or_else(|| { - format!( - "Invalid PR reference: {}. Use a PR number or URL.", - pr_input - ) - })?; - - let repo_full = match (&owner, &repo, repo_override) { - (Some(o), Some(r), _) => format!("{}/{}", o, r), - (_, _, Some(r)) => r.to_string(), - _ => resolve_origin_repo()?, - }; - - let (repo_owner, repo_name) = { - let parts: Vec<&str> = repo_full.split('/').collect(); - if parts.len() != 2 { - return Err(format!("Invalid repo format: {}", repo_full)); - } - ( - owner.unwrap_or_else(|| parts[0].to_string()), - repo.unwrap_or_else(|| parts[1].to_string()), - ) - }; - - // Use GraphQL to get the PR node ID, branch refs, and repo owners - let query = format!( - r#"query {{ repository(owner: "{}", name: "{}") {{ pullRequest(number: {}) {{ id url baseRefName headRefName baseRepository {{ owner {{ login }} }} headRepository {{ owner {{ login }} }} }} }} }}"#, - repo_owner, repo_name, number - ); - - let output = Command::new("gh") - .args(["api", "graphql", "-f", &format!("query={}", query)]) - .output() - .map_err(|e| format!("Failed to run gh api graphql: {}", e))?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(format!("gh api graphql failed: {}", stderr.trim())); - } - - let json_str = String::from_utf8_lossy(&output.stdout); - - // Parse the GraphQL response - let node_id = extract_json_string(&json_str, "id") - .ok_or_else(|| "Could not parse PR node ID from GraphQL response".to_string())?; - let base_ref = - extract_json_string(&json_str, "baseRefName").unwrap_or_else(|| "base".to_string()); - let head_ref = - extract_json_string(&json_str, "headRefName").unwrap_or_else(|| "head".to_string()); - - // Extract repo owners from nested structure - let base_repo_owner = - extract_nested_login(&json_str, "baseRepository").unwrap_or_else(|| repo_owner.clone()); - let head_repo_owner = extract_nested_login(&json_str, "headRepository"); - - Ok(PrInfo { - number, - node_id, - repo_owner, - repo_name, - base_ref, - head_ref, - base_repo_owner, - head_repo_owner, - }) -} - -fn extract_json_string(json: &str, key: &str) -> Option { - let pattern = format!("\"{}\":\"", key); - if let Some(start) = json.find(&pattern) { - let value_start = start + pattern.len(); - if let Some(end) = json[value_start..].find('"') { - return Some(json[value_start..value_start + end].to_string()); - } - } - None -} - -fn extract_nested_login(json: &str, parent_key: &str) -> Option { - // Look for pattern like "baseRepository":{"owner":{"login":"username"}} - // or handle null case like "headRepository":null - let pattern = format!("\"{}\":", parent_key); - if let Some(start) = json.find(&pattern) { - let after_key = &json[start + pattern.len()..]; - // Check if it's null - if after_key.trim_start().starts_with("null") { - return None; - } - // Look for login within this section - if let Some(login_start) = after_key.find("\"login\":\"") { - let value_start = login_start + 9; - let after_login = &after_key[value_start..]; - if let Some(end) = after_login.find('"') { - return Some(after_login[..end].to_string()); - } - } - } - None -} - -/// Fetch the list of files that are marked as viewed on GitHub -pub fn fetch_viewed_files(pr_info: &PrInfo) -> Result, String> { - let query = format!( - r#"query {{ repository(owner: "{}", name: "{}") {{ pullRequest(number: {}) {{ files(first: 100) {{ nodes {{ path viewerViewedState }} }} }} }} }}"#, - pr_info.repo_owner, pr_info.repo_name, pr_info.number - ); - - let output = Command::new("gh") - .args(["api", "graphql", "-f", &format!("query={}", query)]) - .output() - .map_err(|e| format!("Failed to run gh api graphql: {}", e))?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(format!("gh api graphql failed: {}", stderr.trim())); - } - - let json_str = String::from_utf8_lossy(&output.stdout); - - // Parse the response to find viewed files - // Look for patterns like: "path":"filename","viewerViewedState":"VIEWED" - let mut viewed_files = HashSet::new(); +pub fn run_diff_ui(options: DiffOptions, backend: Option<&dyn VcsBackend>) -> io::Result<()> { + let repository_context = + pr_provider::RepositoryContext::resolve(backend, options.origin.as_deref()); - // Simple parsing: find all path/viewerViewedState pairs - let mut remaining = json_str.as_ref(); - while let Some(path_start) = remaining.find("\"path\":\"") { - let path_value_start = path_start + 8; - let after_path = &remaining[path_value_start..]; - if let Some(path_end) = after_path.find('"') { - let path = &after_path[..path_end]; - - // Look for viewerViewedState after this path - let after_path_str = &after_path[path_end..]; - if let Some(state_start) = after_path_str.find("\"viewerViewedState\":\"") { - let state_value_start = state_start + 21; - let after_state = &after_path_str[state_value_start..]; - if let Some(state_end) = after_state.find('"') { - let state = &after_state[..state_end]; - if state == "VIEWED" { - viewed_files.insert(path.to_string()); - } - } - } - - remaining = &remaining[path_value_start + path_end..]; - } else { - break; - } - } - - Ok(viewed_files) -} - -/// Mark a file as viewed on GitHub PR (non-blocking, spawns a thread) -pub fn mark_file_as_viewed_async(pr_info: &PrInfo, file_path: &str) { - let node_id = pr_info.node_id.clone(); - let path = file_path.to_string(); - - thread::spawn(move || { - let _ = mark_file_as_viewed_sync(&node_id, &path); - }); -} - -/// Unmark a file as viewed on GitHub PR (non-blocking, spawns a thread) -pub fn unmark_file_as_viewed_async(pr_info: &PrInfo, file_path: &str) { - let node_id = pr_info.node_id.clone(); - let path = file_path.to_string(); - - thread::spawn(move || { - let _ = unmark_file_as_viewed_sync(&node_id, &path); - }); -} - -/// Mark a file as viewed on GitHub PR (blocking) -fn mark_file_as_viewed_sync(node_id: &str, file_path: &str) -> Result<(), String> { - let mutation = format!( - r#"mutation {{ markFileAsViewed(input: {{ pullRequestId: "{}", path: "{}" }}) {{ clientMutationId }} }}"#, - node_id, file_path - ); - - let output = Command::new("gh") - .args(["api", "graphql", "-f", &format!("query={}", mutation)]) - .output() - .map_err(|e| format!("Failed to run gh api graphql: {}", e))?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(stderr.trim().to_string()); - } - - Ok(()) -} - -/// Unmark a file as viewed on GitHub PR (blocking) -fn unmark_file_as_viewed_sync(node_id: &str, file_path: &str) -> Result<(), String> { - let mutation = format!( - r#"mutation {{ unmarkFileAsViewed(input: {{ pullRequestId: "{}", path: "{}" }}) {{ clientMutationId }} }}"#, - node_id, file_path - ); - - let output = Command::new("gh") - .args(["api", "graphql", "-f", &format!("query={}", mutation)]) - .output() - .map_err(|e| format!("Failed to run gh api graphql: {}", e))?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(stderr.trim().to_string()); - } - - Ok(()) -} - -fn detect_current_branch_pr() -> Result { - let output = Command::new("gh") - .args(["pr", "view", "--json", "number", "-q", ".number"]) - .output() - .map_err(|e| format!("Failed to run gh: {}", e))?; - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - let msg = stderr.trim(); - if msg.is_empty() { - return Err("No PR found for the current branch".to_string()); - } - return Err(msg.to_string()); - } - let number = String::from_utf8_lossy(&output.stdout).trim().to_string(); - if number.is_empty() { - return Err("No PR found for the current branch".to_string()); - } - Ok(number) -} - -pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Result<()> { // Resolve --detect-pr into options.pr if options.detect_pr && options.pr.is_none() { let mut spinner = Spinner::new( @@ -346,13 +51,13 @@ pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Re "Detecting PR for current branch", Color::Cyan, ); - match detect_current_branch_pr() { - Ok(number) => { - spinner.success(&format!("Detected PR #{}", number)); - options.pr = Some(number); + match pr_provider::detect_current_branch_pr(&repository_context, backend) { + Ok(pr_info) => { + spinner.success(&format!("Detected PR #{}", pr_info.number())); + return app::run_app_with_pr(options, pr_info); } Err(e) => { - spinner.fail(&e); + spinner.fail(&e.to_string()); process::exit(1); } } @@ -360,23 +65,14 @@ pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Re // Handle PR mode if let Some(ref pr_input) = options.pr { - let spinner_msg = match parse_pr_input(pr_input) { - Some((Some(owner), Some(repo), number)) => { - format!("Fetching PR {}/{}#{}", owner, repo, number) - } - Some((_, _, number)) => { - format!("Fetching PR #{}", number) - } - None => "Fetching PR".to_string(), - }; - let mut spinner = Spinner::new(spinners::Dots, spinner_msg, Color::Cyan); - match fetch_pr_info(pr_input, options.origin.as_deref()) { + let mut spinner = Spinner::new(spinners::Dots, "Fetching PR metadata", Color::Cyan); + match pr_provider::fetch_pr_info(pr_input, &repository_context) { Ok(pr_info) => { spinner.success("Fetched PR metadata"); - return app::run_app_with_pr(options, pr_info, backend); + return app::run_app_with_pr(options, pr_info); } Err(e) => { - spinner.fail(&e); + spinner.fail(&e.to_string()); process::exit(1); } } @@ -384,24 +80,15 @@ pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Re // Also check if the reference looks like a PR (number or URL) if let Some(CommitReference::Single(ref input)) = options.reference { - if input.contains("/pull/") || input.parse::().is_ok() { - let spinner_msg = match parse_pr_input(input) { - Some((Some(owner), Some(repo), number)) => { - format!("Fetching PR {}/{}#{}", owner, repo, number) - } - Some((_, _, number)) => { - format!("Fetching PR #{}", number) - } - None => "Fetching PR".to_string(), - }; - let mut spinner = Spinner::new(spinners::Dots, spinner_msg, Color::Cyan); - match fetch_pr_info(input, options.origin.as_deref()) { + if pr_provider::is_pr_reference(input) { + let mut spinner = Spinner::new(spinners::Dots, "Fetching PR metadata", Color::Cyan); + match pr_provider::fetch_pr_info(input, &repository_context) { Ok(pr_info) => { spinner.success("Fetched PR metadata"); - return app::run_app_with_pr(options, pr_info, backend); + return app::run_app_with_pr(options, pr_info); } Err(e) => { - spinner.fail(&e); + spinner.fail(&e.to_string()); process::exit(1); } } @@ -410,6 +97,7 @@ pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Re // Handle stacked mode for range references if options.stacked { + let backend = require_backend(backend)?; if let Some(ref reference) = options.reference { let (from, to) = match reference { CommitReference::Range { from, to } => (from.clone(), to.clone()), @@ -450,5 +138,9 @@ pub fn run_diff_ui(mut options: DiffOptions, backend: &dyn VcsBackend) -> io::Re } } - app::run_app(options, None, backend) + app::run_app(options, require_backend(backend)?) +} + +fn require_backend(backend: Option<&dyn VcsBackend>) -> io::Result<&dyn VcsBackend> { + backend.ok_or_else(|| io::Error::new(io::ErrorKind::NotFound, "not a repository")) } diff --git a/src/command/diff/pr_provider/azure/client.rs b/src/command/diff/pr_provider/azure/client.rs new file mode 100644 index 00000000..0ba945f6 --- /dev/null +++ b/src/command/diff/pr_provider/azure/client.rs @@ -0,0 +1,887 @@ +//! Azure DevOps REST client for pull-request review. +//! +//! The `az` CLI has no first-class command to list a PR's changed files or +//! fetch file content, and the generic `az devops invoke` passthrough spawns a +//! Python process per call — unusable for an interactive diff. So we talk to the +//! Azure DevOps REST API directly. +//! +//! Diffs use the iteration-changes endpoint with `$compareTo=0`, i.e. the diff +//! against the PR's merge base — the same three-dot view the Azure web UI shows, +//! rather than a raw tip-to-tip comparison. +//! +//! Auth: `az login` alone is enough — we mint a bearer token via `az account +//! get-access-token`. Alternatively set a PAT in `AZURE_DEVOPS_EXT_PAT` (the +//! conventional var; `AZURE_DEVOPS_PAT` / `ADO_PAT` also work), sent via HTTP +//! Basic. Only core `az` (or a PAT) is required — not the `azure-devops` +//! extension. + +use std::env; +use std::process::Command; +use std::sync::Arc; +use std::thread; + +use reqwest::header::ACCEPT; +use serde::Deserialize; + +use crate::command::diff::pr_provider::{percent_encode, PrError}; +use crate::command::diff::types::{is_binary_content, FileDiff, FileStatus}; + +const API_VERSION: &str = "7.1"; +/// Azure DevOps OAuth resource id, used with `az account get-access-token`. +const ADO_RESOURCE: &str = "499b84ac-1321-427f-aa17-267ca6975798"; +/// Max concurrent blob fetches — keeps us well under Azure's rate limits while +/// still parallelising content retrieval. +const BLOB_CONCURRENCY: usize = 8; +/// Page size for the paginated iteration-changes endpoint. +const CHANGES_PAGE: usize = 1000; + +/// PR metadata needed to drive the diff UI. +pub struct AzurePrMeta { + pub source_ref: String, + pub target_ref: String, + pub repo_name: String, +} + +enum AdoAuth { + /// Personal access token, sent via HTTP Basic with an empty username. + Pat(String), + /// OAuth bearer token (from `az account get-access-token`). + Bearer(String), +} + +#[derive(Clone)] +pub(super) struct AdoClient { + http: reqwest::blocking::Client, + /// Organisation base URL, e.g. `https://dev.azure.com/org`. + base: String, + project: String, + repo: String, + auth: Arc, +} + +impl AdoClient { + fn new(org_url: &str, project: &str, repo: &str) -> Result { + Ok(Self { + http: reqwest::blocking::Client::builder() + .build() + .map_err(|e| format!("could not create Azure HTTP client: {}", e))?, + base: org_url.trim_end_matches('/').to_string(), + project: project.to_string(), + repo: repo.to_string(), + auth: Arc::new(resolve_auth()?), + }) + } + + fn authed(&self, rb: reqwest::blocking::RequestBuilder) -> reqwest::blocking::RequestBuilder { + match self.auth.as_ref() { + AdoAuth::Pat(pat) => rb.basic_auth("", Some(pat)), + AdoAuth::Bearer(token) => rb.bearer_auth(token), + } + } + + fn with_repo(mut self, repo: &str) -> Self { + self.repo = repo.to_string(); + self + } + + fn refreshed(&self) -> Result { + Self::new(&self.base, &self.project, &self.repo) + } + + /// `{base}/{project}/_apis/git/...` with project URL-encoded. + fn git_url(&self, suffix: &str) -> String { + format!( + "{}/{}/_apis/git/{}", + self.base, + percent_encode(&self.project), + suffix + ) + } + + /// GET `url` and deserialize the JSON body into `T`. Unknown fields are + /// ignored, so each caller's struct declares only the fields it needs. + fn get(&self, url: &str) -> Result { + let resp = self + .authed(self.http.get(url)) + .header(ACCEPT, "application/json") + .send() + .map_err(|e| format!("request failed: {}", e))?; + let status = resp.status(); + let body = resp + .text() + .map_err(|e| format!("response body read failed: {}", e))?; + if !status.is_success() { + return Err(auth_hint(status, &body)); + } + serde_json::from_str(&body) + .map_err(|e| PrError::Other(format!("invalid JSON from Azure: {}", e))) + } + + /// Fetch a blob's text content while retaining whether the side exists. + fn blob_text(&self, blob_id: Option<&str>) -> Result, PrError> { + let Some(blob_id) = blob_id else { + return Ok(None); + }; + let url = format!( + "{}?$format=text&api-version={}", + self.git_url(&format!( + "repositories/{}/blobs/{}", + percent_encode(&self.repo), + blob_id + )), + API_VERSION + ); + let resp = self + .authed(self.http.get(&url)) + .header(ACCEPT, "text/plain") + .send() + .map_err(|e| format!("blob {} request failed: {}", blob_id, e))?; + let status = resp.status(); + if !status.is_success() { + let body = resp + .text() + .map_err(|e| format!("blob {} error body read failed: {}", blob_id, e))?; + return Err(auth_hint(status, &body)); + } + // Read as bytes then lossy-decode so binary blobs degrade gracefully + // (build_file_diff flags them as binary downstream). + let bytes = resp + .bytes() + .map_err(|e| format!("blob {} read failed: {}", blob_id, e))?; + Ok(Some(String::from_utf8_lossy(&bytes).into_owned())) + } + + fn latest_iteration(&self, pr_id: u64) -> Result { + let url = format!( + "{}?api-version={}", + self.git_url(&format!( + "repositories/{}/pullRequests/{}/iterations", + percent_encode(&self.repo), + pr_id + )), + API_VERSION + ); + let list: IterationList = self.get(&url)?; + list.value + .iter() + .map(|i| i.id) + .max() + .ok_or_else(|| PrError::Other("PR has no iterations".to_string())) + } + + /// All change entries for `iteration`, compared against the merge base + /// (`$compareTo=0`), following `$skip`/`$top` pagination. + fn changes(&self, pr_id: u64, iteration: u64) -> Result, PrError> { + let mut entries = Vec::new(); + let mut skip = 0_u64; + loop { + let url = format!( + "{}?$compareTo=0&$top={}&$skip={}&api-version={}", + self.git_url(&format!( + "repositories/{}/pullRequests/{}/iterations/{}/changes", + percent_encode(&self.repo), + pr_id, + iteration + )), + CHANGES_PAGE, + skip, + API_VERSION + ); + let page: ChangesPage = self.get(&url)?; + for raw_change in page.change_entries { + if let Some(change) = raw_change.into_change()? { + entries.push(change); + } + } + match page.next_skip { + None | Some(0) => break, + Some(next_skip) if next_skip > skip => skip = next_skip, + Some(next_skip) => { + return Err(PrError::Other(format!( + "Azure returned non-advancing nextSkip cursor {} after {}", + next_skip, skip + ))) + } + } + } + Ok(entries) + } + + fn fetch_file_diff(&self, change: &ChangeEntry) -> Result { + let old = self.blob_text(change.old_blob.as_deref()); + let new = self.blob_text(change.new_blob.as_deref()); + match (old, new) { + (Ok(old), Ok(new)) => Ok(build_file_diff(change, old, new)), + (Err(old), Err(new)) => Err(PrError::Other(format!( + "both blob fetches failed for {}: {}; {}", + change.path, old, new + ))), + (Err(error), _) | (_, Err(error)) => Err(error), + } + } + + fn load_file_diffs(&self, pr_id: u64) -> Result, PrError> { + let iteration = self.latest_iteration(pr_id)?; + let changes = self.changes(pr_id, iteration)?; + if changes.is_empty() { + return Ok(Vec::new()); + } + + let worker_count = changes.len().min(BLOB_CONCURRENCY); + let base_chunk_size = changes.len() / worker_count; + let larger_chunks = changes.len() % worker_count; + let batches = thread::scope(|scope| { + let mut workers = Vec::with_capacity(worker_count); + let mut chunk_start = 0; + for worker_index in 0..worker_count { + let chunk_len = base_chunk_size + usize::from(worker_index < larger_chunks); + let chunk = &changes[chunk_start..chunk_start + chunk_len]; + let result_start = chunk_start; + chunk_start += chunk_len; + let client = self.clone(); + workers.push(scope.spawn(move || { + chunk + .iter() + .enumerate() + .map(|(offset, change)| { + (result_start + offset, client.fetch_file_diff(change)) + }) + .collect::>() + })); + } + + workers + .into_iter() + .map(|worker| worker.join()) + .collect::>() + }); + + let mut out = std::iter::repeat_with(|| None) + .take(changes.len()) + .collect::>>(); + let mut failures = Vec::new(); + for batch in batches { + match batch { + Ok(results) => { + for (index, result) in results { + match result { + Ok(diff) => out[index] = Some(diff), + Err(error) => failures.push(error), + } + } + } + Err(_) => failures.push(PrError::Other( + "Azure blob fetch worker panicked".to_string(), + )), + } + } + + if failures.len() == 1 { + return Err(failures.remove(0)); + } + if !failures.is_empty() { + return Err(PrError::Other(format!( + "multiple Azure blob fetch failures: {}", + failures + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("; ") + ))); + } + + out.into_iter() + .enumerate() + .map(|(index, diff)| { + diff.ok_or_else(|| { + PrError::Other(format!( + "Azure blob worker returned no result for change {}", + index + )) + }) + }) + .collect() + } + + fn fetch_pr_metadata(&self, pr_id: u64) -> Result { + let url = format!( + "{}?api-version={}", + self.git_url(&format!("pullrequests/{}", pr_id)), + API_VERSION + ); + let detail: PrDetail = self.get(&url)?; + Ok(AzurePrMeta { + source_ref: detail.source_ref_name, + target_ref: detail.target_ref_name, + repo_name: detail + .repository + .and_then(|repository| repository.name) + .unwrap_or_else(|| self.repo.clone()), + }) + } + + fn detect_active_pr(&self, branch: &str) -> Result { + let url = format!( + "{}?searchCriteria.status=active&searchCriteria.sourceRefName={}&api-version={}", + self.git_url(&format!( + "repositories/{}/pullrequests", + percent_encode(&self.repo) + )), + percent_encode(&format!("refs/heads/{}", branch)), + API_VERSION + ); + let list: PrList = self.get(&url)?; + unique_active_pr(branch, &list.value) + } +} + +/// The PR iterations list; we only need each iteration's numeric id. +#[derive(Deserialize)] +struct IterationList { + value: Vec, +} + +#[derive(Deserialize)] +struct Iteration { + id: u64, +} + +/// A page of the iteration-changes endpoint. +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct ChangesPage { + change_entries: Vec, + /// Canonical "more pages" cursor; `0` or absent means we're done. + next_skip: Option, +} + +/// A `changeEntries[]` entry exactly as Azure returns it. +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct RawChange { + change_type: AzureChangeType, + item: Option, + original_path: Option, +} + +#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +enum AzureChangeType { + None, + Add, + Edit, + Encoding, + Rename, + Delete, + Undelete, + Branch, + Merge, + Lock, + Rollback, + SourceRename, + TargetRename, + Property, + All, +} + +impl AzureChangeType { + fn file_status(self) -> FileStatus { + match self { + Self::Add | Self::Undelete => FileStatus::Added, + Self::Delete => FileStatus::Deleted, + Self::None + | Self::Edit + | Self::Encoding + | Self::Rename + | Self::Branch + | Self::Merge + | Self::Lock + | Self::Rollback + | Self::SourceRename + | Self::TargetRename + | Self::Property + | Self::All => FileStatus::Modified, + } + } +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct RawItem { + path: Option, + object_id: Option, + original_object_id: Option, + #[serde(default)] + is_folder: bool, +} + +/// One PR file change reduced to what the diff UI needs. +#[derive(Debug)] +struct ChangeEntry { + /// Repo-relative path without a leading slash. + path: String, + /// Blob id of the new (head) side, if any. + new_blob: Option, + /// Blob id of the old (base) side, if any. + old_blob: Option, + status: FileStatus, +} + +impl RawChange { + /// Reduce a wire entry to a [`ChangeEntry`], skipping folders explicitly. + fn into_change(self) -> Result, PrError> { + let item = self.item.ok_or_else(|| { + PrError::Other("Azure returned a file change without an item".to_string()) + })?; + if item.is_folder { + return Ok(None); + } + let raw_path = item + .path + .or(self.original_path) + .filter(|path| !path.trim_matches('/').is_empty()) + .ok_or_else(|| { + PrError::Other("Azure returned a file change without a path".to_string()) + })?; + let path = raw_path.trim_start_matches('/').to_string(); + let new_blob = item.object_id.filter(|id| !id.is_empty()); + let old_blob = item.original_object_id.filter(|id| !id.is_empty()); + let status = self.change_type.file_status(); + + match status { + FileStatus::Added if new_blob.is_none() => { + return Err(missing_blob_error(&path, "new")); + } + FileStatus::Deleted if old_blob.is_none() => { + return Err(missing_blob_error(&path, "old")); + } + FileStatus::Modified if old_blob.is_none() || new_blob.is_none() => { + let side = if old_blob.is_none() { "old" } else { "new" }; + return Err(missing_blob_error(&path, side)); + } + _ => {} + } + + Ok(Some(ChangeEntry { + path, + new_blob, + old_blob, + status, + })) + } +} + +fn missing_blob_error(path: &str, side: &str) -> PrError { + PrError::Other(format!( + "Azure returned file change {} without {} blob id", + path, side + )) +} + +fn build_file_diff( + change: &ChangeEntry, + old_content: Option, + new_content: Option, +) -> FileDiff { + let old_content = old_content.unwrap_or_default(); + let new_content = new_content.unwrap_or_default(); + let is_binary = is_binary_content(&old_content) || is_binary_content(&new_content); + FileDiff { + filename: change.path.clone(), + old_content, + new_content, + status: change.status, + is_binary, + } +} + +/// The PR detail endpoint, reduced to the refs and repo name we display. +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct PrDetail { + #[serde(default)] + source_ref_name: String, + #[serde(default)] + target_ref_name: String, + repository: Option, +} + +#[derive(Deserialize)] +struct RepoRef { + name: Option, +} + +/// Active PRs matching a source branch. +#[derive(Deserialize)] +struct PrList { + value: Vec, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct PrId { + pull_request_id: u64, + target_ref_name: Option, +} + +fn unique_active_pr(branch: &str, prs: &[PrId]) -> Result { + match prs { + [] => Err(PrError::NotFound(format!( + "No active PR found for branch {}", + branch + ))), + [pr] => Ok(pr.pull_request_id), + prs => Err(PrError::Other(format!( + "multiple active PRs found for branch {}: {}. Specify the PR explicitly", + branch, + prs.iter() + .map(|pr| match pr.target_ref_name.as_deref() { + Some(target) => format!("#{} ({})", pr.pull_request_id, target), + None => format!("#{}", pr.pull_request_id), + }) + .collect::>() + .join(", ") + ))), + } +} + +pub fn resolve_pr( + org_url: &str, + project: &str, + repo: &str, + pr_id: u64, +) -> Result<(AdoClient, AzurePrMeta), PrError> { + on_http_thread(|| { + let client = AdoClient::new(org_url, project, repo)?; + let metadata = client.fetch_pr_metadata(pr_id)?; + let client = if metadata.repo_name.is_empty() || metadata.repo_name == repo { + client + } else { + client.with_repo(&metadata.repo_name) + }; + Ok((client, metadata)) + }) +} + +pub fn detect_active_pr( + org_url: &str, + project: &str, + repo: &str, + branch: &str, +) -> Result<(AdoClient, u64, AzurePrMeta), PrError> { + on_http_thread(|| { + let client = AdoClient::new(org_url, project, repo)?; + let pr_id = client.detect_active_pr(branch)?; + let metadata = client.fetch_pr_metadata(pr_id)?; + let client = if metadata.repo_name.is_empty() || metadata.repo_name == repo { + client + } else { + client.with_repo(&metadata.repo_name) + }; + Ok((client, pr_id, metadata)) + }) +} + +pub fn load_pr_file_diffs(client: &AdoClient, pr_id: u64) -> Result, PrError> { + let client = client.clone(); + on_http_thread(move || match client.load_file_diffs(pr_id) { + Err(PrError::Auth(_)) => client.refreshed()?.load_file_diffs(pr_id), + result => result, + }) +} + +/// Keep reqwest's blocking client outside any transitive async runtime. +fn on_http_thread(operation: F) -> Result +where + T: Send, + F: FnOnce() -> Result + Send, +{ + thread::scope(|scope| { + scope + .spawn(operation) + .join() + .map_err(|_| PrError::Other("Azure HTTP worker panicked".to_string()))? + }) +} + +fn resolve_auth() -> Result { + // `AZURE_DEVOPS_EXT_PAT` is the conventional Azure DevOps PAT var (read by + // the `az devops` CLI extension); the others are accepted as aliases. + for var in ["AZURE_DEVOPS_EXT_PAT", "AZURE_DEVOPS_PAT", "ADO_PAT"] { + if let Ok(pat) = env::var(var) { + if !pat.trim().is_empty() { + return Ok(AdoAuth::Pat(pat)); + } + } + } + // Fall back to an OAuth token from the Azure CLI (`az login`). + let output = Command::new("az") + .args([ + "account", + "get-access-token", + "--resource", + ADO_RESOURCE, + "-o", + "json", + ]) + .output() + .map_err(|e| { + PrError::Auth(format!( + "No Azure DevOps credentials: run `az login`, or set AZURE_DEVOPS_EXT_PAT ({})", + e + )) + })?; + if !output.status.success() { + return Err(PrError::Auth( + "No Azure DevOps credentials: run `az login`, or set AZURE_DEVOPS_EXT_PAT.".to_string(), + )); + } + let token: TokenResponse = serde_json::from_slice(&output.stdout) + .map_err(|e| PrError::Other(format!("Could not parse az token output: {}", e)))?; + token + .access_token + .map(AdoAuth::Bearer) + .ok_or_else(|| PrError::Auth("az returned no access token".to_string())) +} + +/// The `az account get-access-token --output json` response. +#[derive(Deserialize)] +struct TokenResponse { + #[serde(rename = "accessToken")] + access_token: Option, +} + +fn auth_hint(status: reqwest::StatusCode, body: &str) -> PrError { + use reqwest::StatusCode; + match status { + StatusCode::UNAUTHORIZED => PrError::Auth( + "Azure DevOps auth failed (401). Check your PAT scopes (Code: Read) or run `az login`." + .to_string(), + ), + StatusCode::FORBIDDEN => PrError::Auth( + "Azure DevOps returned 403. The token lacks access to this repository.".to_string(), + ), + StatusCode::NOT_FOUND => { + PrError::NotFound("Azure DevOps returned 404 (PR or repository not found).".to_string()) + } + _ => { + let snippet: String = body.chars().take(200).collect(); + PrError::Other(format!( + "Azure DevOps request failed ({}): {}", + status, + snippet.trim() + )) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Deserialize a wire change entry and reduce it the way `changes()` does. + fn change(v: serde_json::Value) -> Result, PrError> { + serde_json::from_value::(v) + .unwrap() + .into_change() + } + + #[test] + fn change_type_parses_add() { + let add = change(serde_json::json!({ + "changeType": "add", + "item": { "path": "/src/new.rs", "objectId": "newsha" } + })) + .unwrap() + .unwrap(); + + assert_eq!(add.status, FileStatus::Added); + } + + #[test] + fn change_type_parses_edit() { + let edit = change(serde_json::json!({ + "changeType": "edit", + "item": { "path": "/a.txt", "objectId": "n", "originalObjectId": "o" } + })) + .unwrap() + .unwrap(); + + assert_eq!(edit.status, FileStatus::Modified); + } + + #[test] + fn change_type_parses_delete() { + let del = change(serde_json::json!({ + "changeType": "delete", + "item": { "path": "/gone.rs", "originalObjectId": "o" } + })) + .unwrap() + .unwrap(); + + assert_eq!(del.status, FileStatus::Deleted); + } + + #[test] + fn change_type_parses_rename_as_modified() { + let rename = change(serde_json::json!({ + "changeType": "rename", + "item": { + "path": "/new.rs", + "objectId": "n", + "originalObjectId": "o" + }, + "originalPath": "/old.rs" + })) + .unwrap() + .unwrap(); + + assert_eq!(rename.status, FileStatus::Modified); + } + + #[test] + fn unknown_change_type_is_rejected() { + let result = serde_json::from_value::(serde_json::json!({ + "changeType": "unexpected", + "item": { "path": "/a.txt", "objectId": "n" } + })); + + assert!(result.is_err()); + } + + #[test] + fn skips_folders() { + assert!(change(serde_json::json!({ + "changeType": "add", + "item": { "path": "/dir", "isFolder": true } + })) + .unwrap() + .is_none()); + } + + #[test] + fn falls_back_to_original_path_on_rename() { + let renamed = change(serde_json::json!({ + "changeType": "rename", + "item": { "objectId": "n", "originalObjectId": "o" }, + "originalPath": "/old/name.rs" + })) + .unwrap() + .unwrap(); + assert_eq!(renamed.path, "old/name.rs"); + } + + #[test] + fn malformed_change_without_item_is_rejected() { + let error = change(serde_json::json!({ + "changeType": "add" + })) + .unwrap_err(); + + assert!(error.to_string().contains("without an item")); + } + + #[test] + fn malformed_change_without_path_is_rejected() { + let error = change(serde_json::json!({ + "changeType": "add", + "item": { "objectId": "n" } + })) + .unwrap_err(); + + assert!(error.to_string().contains("without a path")); + } + + #[test] + fn malformed_change_without_required_blob_is_rejected() { + let error = change(serde_json::json!({ + "changeType": "delete", + "item": { "path": "/empty.txt" } + })) + .unwrap_err(); + + assert!(error.to_string().contains("without old blob id")); + } + + #[test] + fn empty_added_file_keeps_added_status() { + let change = change(serde_json::json!({ + "changeType": "add", + "item": { "path": "/empty.txt", "objectId": "n" } + })) + .unwrap() + .unwrap(); + let diff = build_file_diff(&change, None, Some(String::new())); + + assert_eq!(diff.status, FileStatus::Added); + } + + #[test] + fn edit_to_empty_content_keeps_modified_status() { + let change = change(serde_json::json!({ + "changeType": "edit", + "item": { + "path": "/empty.txt", + "objectId": "n", + "originalObjectId": "o" + } + })) + .unwrap() + .unwrap(); + let diff = build_file_diff(&change, Some("content".to_string()), Some(String::new())); + + assert_eq!(diff.status, FileStatus::Modified); + } + + #[test] + fn empty_deleted_file_keeps_deleted_status() { + let change = change(serde_json::json!({ + "changeType": "delete", + "item": { "path": "/empty.txt", "originalObjectId": "o" } + })) + .unwrap() + .unwrap(); + let diff = build_file_diff(&change, Some(String::new()), None); + + assert_eq!(diff.status, FileStatus::Deleted); + } + + #[test] + fn active_pr_detection_rejects_ambiguous_matches() { + let prs = [ + PrId { + pull_request_id: 12, + target_ref_name: Some("refs/heads/main".to_string()), + }, + PrId { + pull_request_id: 18, + target_ref_name: Some("refs/heads/release".to_string()), + }, + ]; + + let error = unique_active_pr("feature", &prs).expect_err("ambiguous PRs"); + + assert!(error.to_string().contains("#12 (refs/heads/main)")); + assert!(error.to_string().contains("#18 (refs/heads/release)")); + } + + #[test] + fn active_pr_detection_accepts_one_match() { + let prs = [PrId { + pull_request_id: 12, + target_ref_name: None, + }]; + + assert_eq!(unique_active_pr("feature", &prs).unwrap(), 12); + } + + #[test] + fn encodes_segments() { + assert_eq!(percent_encode("My Project"), "My%20Project"); + assert_eq!( + percent_encode("refs/heads/feature/x"), + "refs%2Fheads%2Ffeature%2Fx" + ); + assert_eq!(percent_encode("simple-repo.git"), "simple-repo.git"); + } +} diff --git a/src/command/diff/pr_provider/azure/mod.rs b/src/command/diff/pr_provider/azure/mod.rs new file mode 100644 index 00000000..aa5b4fba --- /dev/null +++ b/src/command/diff/pr_provider/azure/mod.rs @@ -0,0 +1,311 @@ +//! Azure DevOps provider routing and URL parsing. + +mod client; + +use crate::command::diff::types::FileDiff; + +use super::{ + decoded_path_segments, parse_http_url, percent_encode, strip_http_userinfo, HttpUrl, PrError, +}; + +#[derive(Clone, Debug)] +pub(super) struct AzureRepository { + org_url: String, + org: String, + project: String, + repo: String, +} + +impl AzureRepository { + pub(super) fn with_number(&self, id: u64) -> AzurePrReference { + AzurePrReference { + repository: self.clone(), + id, + } + } +} + +#[derive(Clone, Debug)] +pub(super) struct AzurePrReference { + repository: AzureRepository, + id: u64, +} + +#[derive(Clone)] +pub(crate) struct AzurePr { + client: client::AdoClient, + pub(super) number: u64, + pub(super) org: String, + pub(super) repo_name: String, + pub(super) base_ref: String, + pub(super) head_ref: String, + org_url: String, + project: String, +} + +impl AzurePr { + fn resolved( + repository: &AzureRepository, + client: client::AdoClient, + number: u64, + meta: client::AzurePrMeta, + ) -> Self { + Self { + client, + number, + org: repository.org.clone(), + repo_name: if meta.repo_name.is_empty() { + repository.repo.clone() + } else { + meta.repo_name + }, + base_ref: strip_ref_prefix(&meta.target_ref), + head_ref: strip_ref_prefix(&meta.source_ref), + org_url: repository.org_url.clone(), + project: repository.project.clone(), + } + } +} + +pub(super) fn parse_pr_url(url: HttpUrl<'_>) -> Option { + let parts = decoded_path_segments(url.path)?; + let (repository, pullrequest_index) = if url.host.eq_ignore_ascii_case("dev.azure.com") { + if parts.len() != 6 || parts[2] != "_git" { + return None; + } + ( + AzureRepository { + org_url: format!("https://dev.azure.com/{}", parts[0]), + org: parts[0].clone(), + project: parts[1].clone(), + repo: parts[3].clone(), + }, + 4, + ) + } else { + let org = visualstudio_org(url.host)?; + let (project_index, git_index, pr_index) = match parts.len() { + 5 => (0, 1, 3), + 6 => (1, 2, 4), + _ => return None, + }; + if parts[git_index] != "_git" { + return None; + } + ( + AzureRepository { + org_url: format!("https://{}", url.host), + org, + project: parts[project_index].clone(), + repo: parts[git_index + 1].clone(), + }, + pr_index, + ) + }; + + if !parts[pullrequest_index].eq_ignore_ascii_case("pullrequest") { + return None; + } + let id = parts.get(pullrequest_index + 1)?.parse().ok()?; + Some(AzurePrReference { repository, id }) +} + +pub(super) fn parse_repository(input: &str) -> Option { + let input = input.trim().trim_end_matches('/'); + if let Some(repository) = parse_ssh_repository(input) { + return Some(repository); + } + + let normalized = strip_http_userinfo(input); + let url = parse_http_url(normalized.as_ref())?; + let mut parts = decoded_path_segments(url.path)?; + let repo = parts.last_mut()?; + *repo = repo.trim_end_matches(".git").to_string(); + if repo.is_empty() { + return None; + } + + if url.host.eq_ignore_ascii_case("dev.azure.com") { + if parts.len() != 4 || parts[2] != "_git" { + return None; + } + Some(AzureRepository { + org_url: format!("https://dev.azure.com/{}", parts[0]), + org: parts[0].clone(), + project: parts[1].clone(), + repo: parts[3].clone(), + }) + } else { + let org = visualstudio_org(url.host)?; + let (project_index, git_index) = match parts.len() { + 3 => (0, 1), + 4 => (1, 2), + _ => return None, + }; + if parts[git_index] != "_git" { + return None; + } + Some(AzureRepository { + org_url: format!("https://{}", url.host), + org, + project: parts[project_index].clone(), + repo: parts[git_index + 1].clone(), + }) + } +} + +fn visualstudio_org(host: &str) -> Option { + let lowercase = host.to_ascii_lowercase(); + let org = lowercase.strip_suffix(".visualstudio.com")?; + if org.is_empty() || org.contains('.') { + return None; + } + Some(org.to_string()) +} + +fn parse_ssh_repository(input: &str) -> Option { + let path = input + .strip_prefix("git@ssh.dev.azure.com:") + .or_else(|| input.strip_prefix("ssh://git@ssh.dev.azure.com/"))?; + let parts = decoded_path_segments(&format!("/{}", path))?; + let (org_index, project_index, repo_index) = if parts.len() == 4 && parts[0] == "v3" { + (1, 2, 3) + } else if parts.len() == 3 { + (0, 1, 2) + } else { + return None; + }; + let repo = parts[repo_index].trim_end_matches(".git").to_string(); + if repo.is_empty() { + return None; + } + Some(AzureRepository { + org_url: format!("https://dev.azure.com/{}", parts[org_index]), + org: parts[org_index].clone(), + project: parts[project_index].clone(), + repo, + }) +} + +pub(super) fn fetch_pr_info(reference: &AzurePrReference) -> Result { + let az = &reference.repository; + let id = reference.id; + let (client, meta) = client::resolve_pr(&az.org_url, &az.project, &az.repo, id)?; + + Ok(AzurePr::resolved(az, client, id, meta)) +} + +pub(super) fn detect_current_branch_pr( + repository: &AzureRepository, + branch: &str, +) -> Result { + let (client, id, meta) = client::detect_active_pr( + &repository.org_url, + &repository.project, + &repository.repo, + branch, + )?; + Ok(AzurePr::resolved(repository, client, id, meta)) +} + +pub(super) fn load_pr_file_diffs(pr: &AzurePr) -> Result, PrError> { + client::load_pr_file_diffs(&pr.client, pr.number) +} + +pub(super) fn file_web_url(pr: &AzurePr, filename: &str) -> String { + format!( + "{}/{}/_git/{}/pullrequest/{}?path={}", + pr.org_url, + pr.project, + pr.repo_name, + pr.number, + percent_encode(&format!("/{}", filename)) + ) +} + +fn strip_ref_prefix(ref_name: &str) -> String { + ref_name + .strip_prefix("refs/heads/") + .or_else(|| ref_name.strip_prefix("refs/")) + .unwrap_or(ref_name) + .to_string() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_exact_azure_pr_urls() { + let reference = + parse_http_url("https://dev.azure.com/myorg/MyProject/_git/myrepo/pullrequest/55") + .and_then(parse_pr_url) + .expect("should parse"); + assert_eq!(reference.repository.org_url, "https://dev.azure.com/myorg"); + assert_eq!(reference.repository.project, "MyProject"); + assert_eq!(reference.repository.repo, "myrepo"); + assert_eq!(reference.id, 55); + + assert!(parse_http_url( + "https://dev.azure.com.evil.test/myorg/MyProject/_git/myrepo/pullrequest/55", + ) + .and_then(parse_pr_url) + .is_none()); + assert!(parse_http_url( + "https://dev.azure.com/myorg/MyProject/_git/myrepo/pullrequest/55/files", + ) + .and_then(parse_pr_url) + .is_none()); + } + + #[test] + fn azure_repository_url_is_not_a_pr_reference() { + let url = "https://dev.azure.com/org/project/_git/repo"; + assert!(parse_http_url(url).and_then(parse_pr_url).is_none()); + assert!(parse_repository(url).is_some()); + } + + #[test] + fn fully_decodes_azure_path_segments() { + let reference = + parse_http_url("https://dev.azure.com/org/My%2BProject/_git/caf%C3%A9/pullrequest/1") + .and_then(parse_pr_url) + .expect("should parse"); + assert_eq!(reference.repository.project, "My+Project"); + assert_eq!(reference.repository.repo, "café"); + } + + #[test] + fn parses_visualstudio_pr_url() { + let reference = + parse_http_url("https://myorg.visualstudio.com/MyProject/_git/myrepo/pullrequest/9") + .and_then(parse_pr_url) + .expect("should parse"); + assert_eq!( + reference.repository.org_url, + "https://myorg.visualstudio.com" + ); + assert_eq!(reference.repository.project, "MyProject"); + assert_eq!(reference.id, 9); + } + + #[test] + fn parses_azure_https_and_ssh_repositories() { + let https = + parse_repository("https://myorg@dev.azure.com/myorg/My%20Project/_git/myrepo.git") + .expect("https"); + let ssh = + parse_repository("git@ssh.dev.azure.com:v3/myorg/My%20Project/myrepo").expect("ssh"); + assert_eq!(https.project, "My Project"); + assert_eq!(https.repo, "myrepo"); + assert_eq!(ssh.project, "My Project"); + assert_eq!(ssh.repo, "myrepo"); + } + + #[test] + fn strip_ref_prefixes() { + assert_eq!(strip_ref_prefix("refs/heads/main"), "main"); + assert_eq!(strip_ref_prefix("refs/tags/v1"), "tags/v1"); + assert_eq!(strip_ref_prefix("feature/x"), "feature/x"); + } +} diff --git a/src/command/diff/pr_provider/github.rs b/src/command/diff/pr_provider/github.rs new file mode 100644 index 00000000..1e3e3363 --- /dev/null +++ b/src/command/diff/pr_provider/github.rs @@ -0,0 +1,946 @@ +//! GitHub provider: everything is driven through the `gh` CLI (GraphQL for PR +//! metadata and viewed-file state, the contents API for file blobs). + +use std::collections::HashSet; +use std::process::Command; +use std::sync::{mpsc, Arc, Mutex}; +use std::thread; + +use serde::Deserialize; +use spinoff::{spinners, Color, Spinner}; + +use super::{ + decoded_path_segments, parse_http_url, percent_encode, strip_http_userinfo, HttpUrl, PrError, +}; +use crate::command::diff::types::{is_binary_content, FileDiff, FileStatus}; + +/// Max concurrent `gh api` requests when fetching PR file contents. +/// GitHub's documented secondary rate limit caps concurrent requests at 100 +/// (shared across REST+GraphQL); 8 keeps us comfortably under that while +/// still giving a large speedup over serial fetching. +const PR_FETCH_CONCURRENCY: usize = 8; + +const VIEWED_FILES_QUERY: &str = r#" +query($owner: String!, $name: String!, $number: Int!, $after: String) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + files(first: 100, after: $after) { + nodes { path viewerViewedState } + pageInfo { hasNextPage endCursor } + } + } + } +} +"#; + +const MARK_FILE_VIEWED_MUTATION: &str = r#" +mutation($pullRequestId: ID!, $path: String!) { + markFileAsViewed(input: { pullRequestId: $pullRequestId, path: $path }) { + clientMutationId + } +} +"#; + +const UNMARK_FILE_VIEWED_MUTATION: &str = r#" +mutation($pullRequestId: ID!, $path: String!) { + unmarkFileAsViewed(input: { pullRequestId: $pullRequestId, path: $path }) { + clientMutationId + } +} +"#; + +#[derive(Clone, Debug)] +pub(super) struct GitHubRepository { + owner: String, + repo: String, +} + +impl GitHubRepository { + pub(super) fn with_number(&self, number: u64) -> GitHubPrReference { + GitHubPrReference { + repository: self.clone(), + number, + } + } +} + +#[derive(Clone, Debug)] +pub(super) struct GitHubPrReference { + repository: GitHubRepository, + number: u64, +} + +#[derive(Clone)] +pub(crate) struct GitHubPr { + pub(super) node_id: String, + pub(super) number: u64, + pub(super) repo_owner: String, + pub(super) repo_name: String, + pub(super) base_ref: String, + pub(super) head_ref: String, + pub(super) base_repo_owner: String, + pub(super) head_repo_owner: Option, +} + +pub(super) fn parse_pr_url(url: HttpUrl<'_>) -> Option { + if !url.host.eq_ignore_ascii_case("github.com") { + return None; + } + let parts = decoded_path_segments(url.path)?; + if parts.len() < 4 || parts[2] != "pull" { + return None; + } + Some(GitHubPrReference { + repository: GitHubRepository { + owner: parts[0].clone(), + repo: parts[1].clone(), + }, + number: parts[3].parse().ok()?, + }) +} + +pub(super) fn parse_repository(input: &str) -> Option { + let input = input.trim().trim_end_matches('/'); + let normalized = strip_http_userinfo(input); + if let Some(url) = parse_http_url(normalized.as_ref()) { + if !url.host.eq_ignore_ascii_case("github.com") { + return None; + } + return repository_from_path(url.path); + } + + if let Some(path) = input.strip_prefix("git@github.com:") { + return repository_from_path(&format!("/{}", path)); + } + if let Some(path) = input.strip_prefix("ssh://git@github.com/") { + return repository_from_path(&format!("/{}", path)); + } + + repository_from_path(&format!("/{}", input)) +} + +fn repository_from_path(path: &str) -> Option { + let mut parts = decoded_path_segments(path)?; + if parts.len() != 2 { + return None; + } + let repo = parts.pop()?.trim_end_matches(".git").to_string(); + let owner = parts.pop()?; + if owner.is_empty() || repo.is_empty() { + return None; + } + Some(GitHubRepository { owner, repo }) +} + +/// The `gh api graphql` response envelope: `{ "data": { ... } }`. +#[derive(Deserialize)] +struct GraphQl { + data: Option, +} + +#[derive(Deserialize)] +struct RepoNode { + repository: Option>, +} + +#[derive(Deserialize)] +struct PullRequestNode { + #[serde(rename = "pullRequest")] + pull_request: Option, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct PrNode { + id: String, + base_ref_name: Option, + head_ref_name: Option, + base_repository: Option, + head_repository: Option, +} + +#[derive(Deserialize)] +struct RepoOwner { + owner: Owner, +} + +#[derive(Deserialize)] +struct Owner { + login: String, +} + +pub(super) fn fetch_pr_info(reference: &GitHubPrReference) -> Result { + let number = reference.number; + let repo_owner = reference.repository.owner.clone(); + let repo_name = reference.repository.repo.clone(); + + // Use GraphQL to get the PR node ID, branch refs, and repo owners + let query = format!( + r#"query {{ repository(owner: "{}", name: "{}") {{ pullRequest(number: {}) {{ id url baseRefName headRefName baseRepository {{ owner {{ login }} }} headRepository {{ owner {{ login }} }} }} }} }}"#, + repo_owner, repo_name, number + ); + + let output = Command::new("gh") + .args(["api", "graphql", "-f", &format!("query={}", query)]) + .output() + .map_err(|e| format!("Failed to run gh api graphql: {}", e))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(PrError::Other(format!( + "gh api graphql failed: {}", + stderr.trim() + ))); + } + + let resp: GraphQl> = serde_json::from_slice(&output.stdout) + .map_err(|e| PrError::Other(format!("could not parse gh graphql response: {}", e)))?; + let pr = resp + .data + .and_then(|d| d.repository) + .and_then(|r| r.pull_request) + .ok_or_else(|| PrError::NotFound(format!("PR #{} not found", number)))?; + + Ok(GitHubPr { + node_id: pr.id, + number, + repo_owner: repo_owner.clone(), + repo_name, + base_ref: pr.base_ref_name.unwrap_or_else(|| "base".to_string()), + head_ref: pr.head_ref_name.unwrap_or_else(|| "head".to_string()), + base_repo_owner: pr + .base_repository + .map(|r| r.owner.login) + .unwrap_or(repo_owner), + head_repo_owner: pr.head_repository.map(|r| r.owner.login), + }) +} + +pub(super) fn detect_current_branch_pr( + repository: &GitHubRepository, + branch: &str, +) -> Result { + let repo = format!("{}/{}", repository.owner, repository.repo); + let output = Command::new("gh") + .args([ + "pr", "view", branch, "--repo", &repo, "--json", "number", "-q", ".number", + ]) + .output() + .map_err(|e| format!("Failed to run gh: {}", e))?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + let msg = stderr.trim(); + if msg.is_empty() { + return Err(PrError::NotFound( + "No PR found for the current branch".to_string(), + )); + } + return Err(PrError::Other(msg.to_string())); + } + let number = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if number.is_empty() { + return Err(PrError::NotFound( + "No PR found for the current branch".to_string(), + )); + } + let number = number + .parse() + .map_err(|error| PrError::Other(format!("invalid PR number from gh: {error}")))?; + Ok(repository.with_number(number)) +} + +pub(super) fn file_web_url(pr: &GitHubPr, filename: &str) -> String { + format!( + "https://github.com/{}/{}/pull/{}/files#diff-{}", + pr.repo_owner, + pr.repo_name, + pr.number, + file_anchor(filename) + ) +} + +/// SHA-256 file anchor used by GitHub's PR "Files changed" deep links +/// (`#diff-`). +fn file_anchor(filename: &str) -> String { + use sha2::{Digest, Sha256}; + let mut hasher = Sha256::new(); + hasher.update(filename.as_bytes()); + format!("{:x}", hasher.finalize()) +} + +#[derive(Deserialize)] +struct PrFiles { + files: FileConnection, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct FileConnection { + nodes: Vec, + page_info: PageInfo, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct PageInfo { + has_next_page: bool, + end_cursor: Option, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct FileNode { + path: String, + viewer_viewed_state: String, +} + +/// Fetch the list of files that are marked as viewed on GitHub +pub(super) fn fetch_viewed_files(pr_info: &GitHubPr) -> Result, PrError> { + fetch_all_viewed_files(|after| fetch_viewed_files_page(pr_info, after)) +} + +fn fetch_viewed_files_page(pr_info: &GitHubPr, after: Option<&str>) -> Result, PrError> { + let mut command = Command::new("gh"); + command + .args(["api", "graphql"]) + .arg("-f") + .arg(format!("query={VIEWED_FILES_QUERY}")) + .arg("-f") + .arg(format!("owner={}", pr_info.repo_owner)) + .arg("-f") + .arg(format!("name={}", pr_info.repo_name)) + .arg("-F") + .arg(format!("number={}", pr_info.number)); + if let Some(cursor) = after { + command.arg("-f").arg(format!("after={cursor}")); + } + + let output = command + .output() + .map_err(|e| format!("Failed to run gh api graphql: {}", e))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(PrError::Other(format!( + "gh api graphql failed: {}", + stderr.trim() + ))); + } + + Ok(output.stdout) +} + +fn fetch_all_viewed_files(mut fetch_page: F) -> Result, PrError> +where + F: FnMut(Option<&str>) -> Result, PrError>, +{ + let mut viewed_paths = HashSet::new(); + let mut cursor = None; + + loop { + let body = fetch_page(cursor.as_deref())?; + let next_cursor = accumulate_viewed_files_page(&body, &mut viewed_paths)?; + if next_cursor.is_none() { + return Ok(viewed_paths); + } + if next_cursor == cursor { + return Err(PrError::Other( + "github graphql returned a repeated file-page cursor".to_string(), + )); + } + cursor = next_cursor; + } +} + +fn accumulate_viewed_files_page( + body: &[u8], + viewed_paths: &mut HashSet, +) -> Result, PrError> { + let resp: GraphQl> = serde_json::from_slice(body) + .map_err(|e| PrError::Other(format!("could not parse gh graphql response: {}", e)))?; + let files = resp + .data + .and_then(|d| d.repository) + .and_then(|r| r.pull_request) + .map(|p| p.files) + .ok_or_else(|| { + PrError::NotFound( + "github graphql response did not include pull request files".to_string(), + ) + })?; + + viewed_paths.extend( + files + .nodes + .into_iter() + .filter(|node| node.viewer_viewed_state == "VIEWED") + .map(|node| node.path), + ); + + if files.page_info.has_next_page { + files.page_info.end_cursor.map(Some).ok_or_else(|| { + PrError::Other("github graphql file page is missing its end cursor".to_string()) + }) + } else { + Ok(None) + } +} + +pub(super) fn set_file_viewed(node_id: &str, file_path: &str, viewed: bool) -> Result<(), PrError> { + let mutation = if viewed { + MARK_FILE_VIEWED_MUTATION + } else { + UNMARK_FILE_VIEWED_MUTATION + }; + let output = Command::new("gh") + .args(["api", "graphql"]) + .arg("-f") + .arg(format!("query={mutation}")) + .arg("-f") + .arg(format!("pullRequestId={node_id}")) + .arg("-f") + .arg(format!("path={file_path}")) + .output() + .map_err(|e| format!("Failed to run gh api graphql: {}", e))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(PrError::Other(stderr.trim().to_string())); + } + + Ok(()) +} + +#[derive(Clone, Deserialize)] +struct ChangedFile { + filename: String, + status: String, + previous_filename: Option, +} + +impl ChangedFile { + fn file_status(&self) -> Result { + match self.status.as_str() { + "added" => Ok(FileStatus::Added), + "removed" => Ok(FileStatus::Deleted), + "modified" | "renamed" | "copied" | "changed" | "unchanged" => Ok(FileStatus::Modified), + status => Err(PrError::Other(format!( + "github returned unsupported file status {status:?} for {}", + self.filename + ))), + } + } + + fn old_path(&self) -> Result, PrError> { + match self.status.as_str() { + "added" => Ok(None), + "renamed" => self.previous_filename.as_deref().map(Some).ok_or_else(|| { + PrError::Other(format!( + "github returned renamed file {} without previous_filename", + self.filename + )) + }), + "copied" => Ok(self.previous_filename.as_deref()), + _ => Ok(Some(&self.filename)), + } + } + + fn new_path(&self) -> Option<&str> { + (self.status != "removed").then_some(self.filename.as_str()) + } +} + +fn fetch_changed_files(pr: &GitHubPr) -> Result, PrError> { + let endpoint = format!( + "repos/{}/{}/pulls/{}/files?per_page=100", + pr.repo_owner, pr.repo_name, pr.number + ); + let output = Command::new("gh") + .args(["api", &endpoint, "--paginate", "--slurp"]) + .output() + .map_err(|error| PrError::Other(format!("failed to list GitHub PR files: {error}")))?; + if !output.status.success() { + return Err(PrError::Other(format!( + "failed to list GitHub PR files: {}", + String::from_utf8_lossy(&output.stderr).trim() + ))); + } + let pages: Vec> = serde_json::from_slice(&output.stdout) + .map_err(|error| PrError::Other(format!("invalid GitHub PR files response: {error}")))?; + Ok(pages.into_iter().flatten().collect()) +} + +fn build_file_diff( + file: ChangedFile, + old_content: Option, + new_content: Option, +) -> Result { + let status = file.file_status()?; + let old_content = old_content.unwrap_or_default(); + let new_content = new_content.unwrap_or_default(); + let is_binary = is_binary_content(&old_content) || is_binary_content(&new_content); + Ok(FileDiff { + filename: file.filename, + old_content, + new_content, + status, + is_binary, + }) +} + +pub(super) fn load_pr_file_diffs(pr_info: &GitHubPr) -> Result, PrError> { + let mut spinner = Spinner::new( + spinners::Dots, + format!( + "Fetching file list for {}/{}#{}", + pr_info.repo_owner, pr_info.repo_name, pr_info.number + ), + Color::Cyan, + ); + + let changed_files = match fetch_changed_files(pr_info) { + Ok(files) => files, + Err(error) => { + let msg = error.to_string(); + spinner.fail(&msg); + return Err(error); + } + }; + let n = changed_files.len(); + if n == 0 { + spinner.success("PR has no changed files"); + return Ok(Vec::new()); + } + + let base_repo = format!("{}/{}", pr_info.base_repo_owner, pr_info.repo_name); + let head_repo = pr_info + .head_repo_owner + .as_ref() + .map(|owner| format!("{}/{}", owner, pr_info.repo_name)) + .unwrap_or_else(|| base_repo.clone()); + let contents = match fetch_pr_file_contents_parallel( + &changed_files, + &base_repo, + &pr_info.base_ref, + &head_repo, + &pr_info.head_ref, + &mut spinner, + ) { + Ok(contents) => contents, + Err(error) => { + let msg = error.to_string(); + spinner.fail(&msg); + return Err(error); + } + }; + + let file_diffs = changed_files + .into_iter() + .zip(contents) + .map(|(file, contents)| build_file_diff(file, contents.old, contents.new)) + .collect::, PrError>>()?; + + spinner.success(&format!("Fetched {} files", n)); + Ok(file_diffs) +} + +#[derive(Clone, Copy)] +enum Side { + Old, + New, +} + +struct FetchTask { + idx: usize, + filename: String, + repo: String, + git_ref: String, + side: Side, +} + +struct FileContents { + old: Option, + new: Option, +} + +enum FetchEvent { + Started(String), + Finished { + idx: usize, + side: Side, + filename: String, + content: Result, + }, +} + +fn fetch_pr_file_contents_parallel( + files: &[ChangedFile], + base_repo: &str, + base_ref: &str, + head_repo: &str, + head_ref: &str, + spinner: &mut Spinner, +) -> Result, PrError> { + let mut tasks = Vec::with_capacity(2 * files.len()); + for (idx, file) in files.iter().enumerate() { + if let Some(path) = file.old_path()? { + tasks.push(FetchTask { + idx, + filename: path.to_string(), + repo: base_repo.to_string(), + git_ref: base_ref.to_string(), + side: Side::Old, + }); + } + if let Some(path) = file.new_path() { + tasks.push(FetchTask { + idx, + filename: path.to_string(), + repo: head_repo.to_string(), + git_ref: head_ref.to_string(), + side: Side::New, + }); + } + } + tasks.reverse(); + + let total = tasks.len(); + let queue = Arc::new(Mutex::new(tasks)); + let (tx, rx) = mpsc::channel::(); + let worker_count = PR_FETCH_CONCURRENCY.min(total); + let mut handles = Vec::with_capacity(worker_count); + for _ in 0..worker_count { + let queue = Arc::clone(&queue); + let tx = tx.clone(); + handles.push(thread::spawn(move || loop { + let task = { queue.lock().expect("GitHub fetch queue poisoned").pop() }; + let Some(task) = task else { break }; + let _ = tx.send(FetchEvent::Started(task.filename.clone())); + let content = fetch_file_content_from_github(&task.repo, &task.git_ref, &task.filename); + let _ = tx.send(FetchEvent::Finished { + idx: task.idx, + side: task.side, + filename: task.filename, + content, + }); + })); + } + drop(tx); + + let mut contents = std::iter::repeat_with(|| FileContents { + old: None, + new: None, + }) + .take(files.len()) + .collect::>(); + let mut failures = Vec::new(); + let mut done = 0usize; + let mut in_flight = Vec::new(); + let mut last_finished = None; + + while let Ok(event) = rx.recv() { + match event { + FetchEvent::Started(name) => in_flight.push(name), + FetchEvent::Finished { + idx, + side, + filename, + content, + } => { + if let Some(position) = in_flight.iter().position(|path| path == &filename) { + in_flight.swap_remove(position); + } + match content { + Ok(content) => match side { + Side::Old => contents[idx].old = Some(content), + Side::New => contents[idx].new = Some(content), + }, + Err(error) => failures.push(error), + } + done += 1; + last_finished = Some(filename); + } + } + spinner.update_text(format_fetch_progress( + done, + total, + &in_flight, + last_finished.as_deref(), + )); + } + + for handle in handles { + if handle.join().is_err() { + failures.push(PrError::Other( + "GitHub file-content worker panicked".to_string(), + )); + } + } + if !failures.is_empty() { + return Err(PrError::Other( + failures + .into_iter() + .map(|error| error.to_string()) + .collect::>() + .join("; "), + )); + } + + Ok(contents) +} + +fn format_fetch_progress( + done: usize, + total: usize, + in_flight: &[String], + last_finished: Option<&str>, +) -> String { + let current = in_flight + .last() + .map(String::as_str) + .or(last_finished) + .unwrap_or_default(); + if current.is_empty() { + format!("Fetching files [{}/{}]", done, total) + } else { + format!("Fetching files [{}/{}] · {}", done, total, current) + } +} + +fn fetch_file_content_from_github( + repo: &str, + git_ref: &str, + path: &str, +) -> Result { + let encoded_path = path + .split('/') + .map(percent_encode) + .collect::>() + .join("/"); + let api_path = format!( + "repos/{}/contents/{}?ref={}", + repo, + encoded_path, + percent_encode(git_ref) + ); + let output = Command::new("gh") + .args([ + "api", + &api_path, + "-H", + "Accept: application/vnd.github.raw+json", + ]) + .output() + .map_err(|error| PrError::Other(format!("failed to fetch GitHub file {path}: {error}")))?; + if !output.status.success() { + return Err(PrError::Other(format!( + "failed to fetch GitHub file {path}: {}", + String::from_utf8_lossy(&output.stderr).trim() + ))); + } + Ok(String::from_utf8_lossy(&output.stdout).into_owned()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_only_exact_github_pull_urls() { + let reference = parse_http_url("https://github.com/owner/repo/pull/123") + .and_then(parse_pr_url) + .expect("should parse"); + assert_eq!(reference.repository.owner, "owner"); + assert_eq!(reference.repository.repo, "repo"); + assert_eq!(reference.number, 123); + assert!(parse_http_url("https://notgithub.com/owner/repo/pull/123") + .and_then(parse_pr_url) + .is_none()); + assert!(parse_http_url("https://github.com/owner/repo/issues/123") + .and_then(parse_pr_url) + .is_none()); + assert!( + parse_http_url("https://github.com/owner/repo/pull/123/checks") + .and_then(parse_pr_url) + .is_some() + ); + assert!( + parse_http_url("https://github.com/owner/repo/pull/123/commits/abc") + .and_then(parse_pr_url) + .is_some() + ); + } + + #[test] + fn parses_github_pull_subpage_urls() { + for url in [ + "https://github.com/owner/repo/pull/123/files", + "https://github.com/owner/repo/pull/123/commits", + ] { + let reference = parse_http_url(url) + .and_then(parse_pr_url) + .expect("supported PR subpage"); + assert_eq!(reference.number, 123); + } + } + + #[test] + fn parses_github_https_and_ssh_repositories() { + let https = parse_repository("https://github.com/owner/repo.git").expect("https"); + let credentialed = + parse_repository("https://user:TOKEN@github.com/owner/repo.git").expect("credentials"); + let ssh = parse_repository("git@github.com:owner/repo.git").expect("ssh"); + assert_eq!(https.owner, "owner"); + assert_eq!(https.repo, "repo"); + assert_eq!(credentialed.owner, "owner"); + assert_eq!(credentialed.repo, "repo"); + assert_eq!(ssh.owner, "owner"); + assert_eq!(ssh.repo, "repo"); + } + + #[test] + fn parses_pr_info_graphql_with_deleted_head_fork() { + let body = serde_json::json!({ + "data": { "repository": { "pullRequest": { + "id": "PR_node1", "url": "https://github.com/o/r/pull/1", + "baseRefName": "main", "headRefName": "feature", + "baseRepository": { "owner": { "login": "base-owner" } }, + "headRepository": null + }}} + }); + let resp: GraphQl> = serde_json::from_value(body).unwrap(); + let pr = resp.data.unwrap().repository.unwrap().pull_request.unwrap(); + assert_eq!(pr.id, "PR_node1"); + assert_eq!(pr.base_ref_name.as_deref(), Some("main")); + assert_eq!(pr.base_repository.unwrap().owner.login, "base-owner"); + assert!(pr.head_repository.is_none()); + } + + #[test] + fn parses_viewed_state_graphql() { + let body = serde_json::json!({ + "data": { "repository": { "pullRequest": { "files": { "nodes": [ + { "path": "a.rs", "viewerViewedState": "VIEWED" }, + { "path": "b.rs", "viewerViewedState": "UNVIEWED" } + ], "pageInfo": { "hasNextPage": false, "endCursor": null }}}}} + }); + let resp: GraphQl> = serde_json::from_value(body).unwrap(); + let nodes = resp + .data + .unwrap() + .repository + .unwrap() + .pull_request + .unwrap() + .files + .nodes; + assert_eq!(nodes[0].viewer_viewed_state, "VIEWED"); + } + + #[test] + fn accumulates_viewed_files_across_graphql_pages() { + let pages = [ + serde_json::json!({ + "data": { "repository": { "pullRequest": { "files": { + "nodes": [ + { "path": "a.rs", "viewerViewedState": "VIEWED" }, + { "path": "b.rs", "viewerViewedState": "UNVIEWED" } + ], + "pageInfo": { "hasNextPage": true, "endCursor": "cursor-1" } + }}}} + }) + .to_string() + .into_bytes(), + serde_json::json!({ + "data": { "repository": { "pullRequest": { "files": { + "nodes": [ + { "path": "c.rs", "viewerViewedState": "VIEWED" } + ], + "pageInfo": { "hasNextPage": false, "endCursor": "cursor-2" } + }}}} + }) + .to_string() + .into_bytes(), + ]; + let mut pages = pages.into_iter(); + let mut cursors = Vec::new(); + + let viewed = fetch_all_viewed_files(|cursor| { + cursors.push(cursor.map(str::to_owned)); + Ok(pages.next().expect("requested page")) + }) + .expect("all pages"); + + assert_eq!(cursors, vec![None, Some("cursor-1".to_string())]); + assert_eq!( + viewed, + HashSet::from(["a.rs".to_string(), "c.rs".to_string()]) + ); + } + + #[test] + fn rejects_partial_graphql_page_without_next_cursor() { + let page = serde_json::json!({ + "data": { "repository": { "pullRequest": { "files": { + "nodes": [{ "path": "a.rs", "viewerViewedState": "VIEWED" }], + "pageInfo": { "hasNextPage": true, "endCursor": null } + }}}} + }) + .to_string() + .into_bytes(); + + let error = fetch_all_viewed_files(|_| Ok(page.clone())).expect_err("missing cursor"); + + assert!(error.to_string().contains("missing its end cursor")); + } + + #[test] + fn empty_added_file_keeps_added_status() { + let file = ChangedFile { + filename: "empty.txt".to_string(), + status: "added".to_string(), + previous_filename: None, + }; + + let diff = build_file_diff(file, None, Some(String::new())).unwrap(); + + assert_eq!(diff.status, FileStatus::Added); + } + + #[test] + fn empty_removed_file_keeps_deleted_status() { + let file = ChangedFile { + filename: "empty.txt".to_string(), + status: "removed".to_string(), + previous_filename: None, + }; + + let diff = build_file_diff(file, Some(String::new()), None).unwrap(); + + assert_eq!(diff.status, FileStatus::Deleted); + } + + #[test] + fn renamed_file_uses_previous_path_for_old_side() { + let file = ChangedFile { + filename: "new.rs".to_string(), + status: "renamed".to_string(), + previous_filename: Some("old.rs".to_string()), + }; + + assert_eq!(file.old_path().unwrap(), Some("old.rs")); + assert_eq!(file.new_path(), Some("new.rs")); + } + + #[test] + fn copied_file_without_previous_path_has_no_old_side() { + let file = ChangedFile { + filename: "copy.rs".to_string(), + status: "copied".to_string(), + previous_filename: None, + }; + + assert_eq!(file.old_path().unwrap(), None); + assert_eq!(file.new_path(), Some("copy.rs")); + } +} diff --git a/src/command/diff/pr_provider/mod.rs b/src/command/diff/pr_provider/mod.rs new file mode 100644 index 00000000..f4bbb7f1 --- /dev/null +++ b/src/command/diff/pr_provider/mod.rs @@ -0,0 +1,437 @@ +//! Pull-request hosting provider routing. + +mod azure; +mod github; +mod viewed; + +use std::borrow::Cow; +use std::collections::HashSet; +use std::fmt; + +use super::types::FileDiff; +use crate::vcs::VcsBackend; + +use azure::{AzurePrReference, AzureRepository}; +use github::{GitHubPrReference, GitHubRepository}; +pub(crate) use viewed::ViewedFileSync; + +#[derive(Debug, thiserror::Error)] +pub enum PrError { + #[error("authentication failed: {0}")] + Auth(String), + #[error("not found: {0}")] + NotFound(String), + #[error("invalid PR reference: {0}")] + InvalidRef(String), + #[error("{0}")] + Other(String), +} + +impl From for PrError { + fn from(s: String) -> Self { + PrError::Other(s) + } +} + +impl From<&str> for PrError { + fn from(s: &str) -> Self { + PrError::Other(s.to_string()) + } +} + +#[derive(Clone)] +pub enum PrInfo { + GitHub(github::GitHubPr), + Azure(azure::AzurePr), +} + +impl PrInfo { + pub fn number(&self) -> u64 { + match self { + Self::GitHub(pr) => pr.number, + Self::Azure(pr) => pr.number, + } + } + + pub fn base_ref(&self) -> &str { + match self { + Self::GitHub(pr) => &pr.base_ref, + Self::Azure(pr) => &pr.base_ref, + } + } + + pub fn head_ref(&self) -> &str { + match self { + Self::GitHub(pr) => &pr.head_ref, + Self::Azure(pr) => &pr.head_ref, + } + } + + pub fn base_repo_owner(&self) -> &str { + match self { + Self::GitHub(pr) => &pr.base_repo_owner, + Self::Azure(pr) => &pr.org, + } + } + + pub fn head_repo_owner(&self) -> Option<&str> { + match self { + Self::GitHub(pr) => pr.head_repo_owner.as_deref(), + Self::Azure(pr) => Some(&pr.org), + } + } + + pub fn load_file_diffs(&self) -> Result, PrError> { + match self { + Self::GitHub(pr) => github::load_pr_file_diffs(pr), + Self::Azure(pr) => azure::load_pr_file_diffs(pr), + } + } + + fn viewed_file_provider(&self) -> Option { + match self { + Self::GitHub(pr) => Some(ViewedFileProvider { pr: pr.clone() }), + Self::Azure(_) => None, + } + } + + pub fn file_web_url(&self, filename: &str) -> String { + match self { + Self::GitHub(pr) => github::file_web_url(pr, filename), + Self::Azure(pr) => azure::file_web_url(pr, filename), + } + } +} + +#[derive(Clone)] +struct ViewedFileProvider { + pr: github::GitHubPr, +} + +impl ViewedFileProvider { + fn fetch(&self) -> Result, PrError> { + github::fetch_viewed_files(&self.pr) + } + + fn set(&self, path: &str, viewed: bool) -> Result<(), PrError> { + github::set_file_viewed(&self.pr.node_id, path, viewed) + } +} + +#[derive(Clone, Debug)] +enum Repository { + GitHub(GitHubRepository), + Azure(AzureRepository), +} + +/// Repository information read once from the selected VCS backend. +#[derive(Clone, Debug)] +pub struct RepositoryContext { + origin: Option, + repository: Option, + origin_error: Option, +} + +impl RepositoryContext { + pub fn resolve(backend: Option<&dyn VcsBackend>, repository_override: Option<&str>) -> Self { + let (origin, origin_error) = match repository_override { + Some(origin) => (Some(origin.to_owned()), None), + None => match backend.map(VcsBackend::origin_url).transpose() { + Ok(origin) => (origin.flatten(), None), + Err(error) => (None, Some(error.to_string())), + }, + }; + let repository = origin.as_deref().and_then(parse_repository); + + Self { + origin, + repository, + origin_error, + } + } + + #[cfg(test)] + fn from_sources(repository_override: Option<&str>, backend_origin: Option<&str>) -> Self { + let origin = repository_override + .map(str::to_owned) + .or_else(|| backend_origin.map(str::to_owned)); + let repository = origin.as_deref().and_then(parse_repository); + Self { + origin, + repository, + origin_error: None, + } + } +} + +#[derive(Clone, Debug)] +enum PrReference { + GitHub(GitHubPrReference), + Azure(AzurePrReference), + Number(u64), +} + +fn parse_pr_reference(input: &str) -> Option { + if let Ok(number) = input.parse::() { + return Some(PrReference::Number(number)); + } + let url = parse_http_url(input)?; + github::parse_pr_url(url) + .map(PrReference::GitHub) + .or_else(|| azure::parse_pr_url(url).map(PrReference::Azure)) +} + +fn parse_repository(input: &str) -> Option { + azure::parse_repository(input) + .map(Repository::Azure) + .or_else(|| github::parse_repository(input).map(Repository::GitHub)) +} + +fn resolve_pr_reference(input: &str, context: &RepositoryContext) -> Result { + match parse_pr_reference(input) { + Some(PrReference::Number(number)) => match &context.repository { + Some(Repository::GitHub(repo)) => Ok(PrReference::GitHub(repo.with_number(number))), + Some(Repository::Azure(repo)) => Ok(PrReference::Azure(repo.with_number(number))), + None => Err(PrError::InvalidRef(repository_context_error(context))), + }, + Some(reference) => Ok(reference), + None => Err(PrError::InvalidRef(format!( + "{}. Use a PR number or an exact GitHub/Azure DevOps PR URL.", + safe_diagnostic(input) + ))), + } +} + +pub fn is_pr_reference(input: &str) -> bool { + parse_pr_reference(input).is_some() +} + +pub fn fetch_pr_info(input: &str, context: &RepositoryContext) -> Result { + match resolve_pr_reference(input, context)? { + PrReference::GitHub(reference) => github::fetch_pr_info(&reference).map(PrInfo::GitHub), + PrReference::Azure(reference) => azure::fetch_pr_info(&reference).map(PrInfo::Azure), + PrReference::Number(_) => { + unreachable!("bare PR numbers are resolved using repository context") + } + } +} + +pub fn detect_current_branch_pr( + context: &RepositoryContext, + backend: Option<&dyn VcsBackend>, +) -> Result { + let backend = backend.ok_or_else(|| { + PrError::NotFound("could not determine the current branch or bookmark".to_string()) + })?; + let branch = backend + .get_pr_source_branch() + .map_err(|error| PrError::Other(error.to_string()))? + .ok_or_else(|| { + PrError::NotFound("could not determine the current branch or bookmark".to_string()) + })?; + match &context.repository { + Some(Repository::GitHub(repo)) => { + let reference = github::detect_current_branch_pr(repo, &branch)?; + github::fetch_pr_info(&reference).map(PrInfo::GitHub) + } + Some(Repository::Azure(repo)) => { + azure::detect_current_branch_pr(repo, &branch).map(PrInfo::Azure) + } + None => Err(PrError::InvalidRef(repository_context_error(context))), + } +} + +fn repository_context_error(context: &RepositoryContext) -> String { + match (&context.origin, &context.origin_error) { + (Some(origin), _) => format!("unsupported repository origin: {}", safe_diagnostic(origin)), + (None, Some(error)) => format!("could not read repository origin: {error}"), + (None, None) => { + "could not determine repository; configure origin or pass --origin".to_string() + } + } +} + +#[derive(Clone, Copy)] +pub(super) struct HttpUrl<'a> { + pub host: &'a str, + pub path: &'a str, +} + +pub(super) fn strip_http_userinfo(input: &str) -> Cow<'_, str> { + let Some((scheme, rest)) = input.split_once("://") else { + return input.into(); + }; + if !scheme.eq_ignore_ascii_case("http") && !scheme.eq_ignore_ascii_case("https") { + return input.into(); + } + let authority_end = rest.find(['/', '?', '#']).unwrap_or(rest.len()); + let authority = &rest[..authority_end]; + let Some((_, host)) = authority.rsplit_once('@') else { + return input.into(); + }; + format!("{}://{}{}", scheme, host, &rest[authority_end..]).into() +} + +struct SafeDiagnostic<'a>(&'a str); + +impl fmt::Display for SafeDiagnostic<'_> { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(strip_http_userinfo(self.0).as_ref()) + } +} + +fn safe_diagnostic(input: &str) -> SafeDiagnostic<'_> { + SafeDiagnostic(input) +} + +pub(super) fn parse_http_url(input: &str) -> Option> { + let (scheme, rest) = input.split_once("://")?; + if !scheme.eq_ignore_ascii_case("http") && !scheme.eq_ignore_ascii_case("https") { + return None; + } + let authority_end = rest.find(['/', '?', '#']).unwrap_or(rest.len()); + let authority = &rest[..authority_end]; + if authority.is_empty() || authority.contains('@') || authority.contains(':') { + return None; + } + let suffix = &rest[authority_end..]; + let path_end = suffix.find(['?', '#']).unwrap_or(suffix.len()); + let path = &suffix[..path_end]; + Some(HttpUrl { + host: authority, + path, + }) +} + +pub(super) fn decoded_path_segments(path: &str) -> Option> { + let path = path.strip_prefix('/')?; + let path = path.strip_suffix('/').unwrap_or(path); + if path.is_empty() { + return Some(Vec::new()); + } + path.split('/').map(decode_path_segment).collect() +} + +pub(super) fn percent_encode(segment: &str) -> String { + use std::fmt::Write; + + let mut encoded = String::with_capacity(segment.len()); + for byte in segment.bytes() { + match byte { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => { + encoded.push(byte as char) + } + _ => { + let _ = write!(encoded, "%{byte:02X}"); + } + } + } + encoded +} + +fn decode_path_segment(segment: &str) -> Option { + if segment.is_empty() { + return None; + } + let bytes = segment.as_bytes(); + let mut decoded = Vec::with_capacity(bytes.len()); + let mut index = 0; + while index < bytes.len() { + if bytes[index] == b'%' { + let high = hex_value(*bytes.get(index + 1)?)?; + let low = hex_value(*bytes.get(index + 2)?)?; + decoded.push((high << 4) | low); + index += 3; + } else { + decoded.push(bytes[index]); + index += 1; + } + } + String::from_utf8(decoded).ok() +} + +fn hex_value(byte: u8) -> Option { + match byte { + b'0'..=b'9' => Some(byte - b'0'), + b'a'..=b'f' => Some(byte - b'a' + 10), + b'A'..=b'F' => Some(byte - b'A' + 10), + _ => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn routes_only_exact_provider_urls() { + assert!(is_pr_reference("123")); + assert!(is_pr_reference("https://github.com/o/r/pull/1")); + assert!(is_pr_reference( + "https://dev.azure.com/o/p/_git/r/pullrequest/1" + )); + assert!(!is_pr_reference("https://github.com.evil.test/o/r/pull/1")); + assert!(!is_pr_reference( + "https://evil.test/dev.azure.com/o/p/_git/r/pullrequest/1" + )); + assert!(!is_pr_reference("https://github.com/o/r/issues/1")); + } + + #[test] + fn repository_override_replaces_backend_origin() { + let context = RepositoryContext::from_sources( + Some("https://dev.azure.com/org/project/_git/repo"), + Some("git@github.com:owner/repo.git"), + ); + + assert!(matches!(context.repository, Some(Repository::Azure(_)))); + } + + #[test] + fn github_repository_origin_accepts_https_credentials() { + let context = RepositoryContext::from_sources( + None, + Some("https://user:TOKEN@github.com/owner/repo.git"), + ); + + assert!(matches!(context.repository, Some(Repository::GitHub(_)))); + } + + #[test] + fn unsupported_origin_error_redacts_https_credentials() { + let context = RepositoryContext::from_sources( + None, + Some("https://user:SECRET@example.com/owner/repo.git"), + ); + + let error = resolve_pr_reference("12", &context).expect_err("unsupported origin"); + + assert_eq!( + error.to_string(), + "invalid PR reference: unsupported repository origin: https://example.com/owner/repo.git" + ); + } + + #[test] + fn invalid_reference_error_redacts_https_credentials() { + let context = RepositoryContext::from_sources(None, None); + + let error = resolve_pr_reference( + "https://user:SECRET@example.com/owner/repo/pull/1", + &context, + ) + .expect_err("unknown provider"); + + assert!(!error.to_string().contains("SECRET")); + } + + #[test] + fn decodes_all_percent_encoded_utf8_bytes() { + assert_eq!( + decode_path_segment("My%2BProject").as_deref(), + Some("My+Project") + ); + assert_eq!(decode_path_segment("caf%C3%A9").as_deref(), Some("café")); + assert!(decode_path_segment("bad%2").is_none()); + } +} diff --git a/src/command/diff/pr_provider/viewed.rs b/src/command/diff/pr_provider/viewed.rs new file mode 100644 index 00000000..f46c47b0 --- /dev/null +++ b/src/command/diff/pr_provider/viewed.rs @@ -0,0 +1,162 @@ +use std::collections::{HashMap, HashSet, VecDeque}; +use std::sync::mpsc::{self, Receiver, Sender}; +use std::thread::{self, JoinHandle}; + +use super::{PrError, PrInfo, ViewedFileProvider}; + +struct ViewedUpdate { + sequence: u64, + path: String, + viewed: bool, +} + +fn coalesce_updates( + first: ViewedUpdate, + queued: impl Iterator, +) -> Vec { + let mut batch = vec![first]; + for queued in queued { + if let Some(update) = batch.iter_mut().find(|update| update.path == queued.path) { + *update = queued; + } else { + batch.push(queued); + } + } + batch +} + +pub(crate) struct ViewedCompletion { + pub path: String, + pub viewed: bool, + pub result: Result<(), PrError>, +} + +pub(crate) struct ViewedFileSync { + provider: ViewedFileProvider, + updates: Option>, + completions: Receiver<(u64, ViewedCompletion)>, + pending: HashMap, + next_sequence: u64, + local_completions: VecDeque, + worker: Option>, +} + +impl ViewedFileSync { + pub fn new(pr: &PrInfo) -> Option { + let provider = pr.viewed_file_provider()?; + let worker_provider = provider.clone(); + let (update_tx, update_rx) = mpsc::channel::(); + let (completion_tx, completion_rx) = mpsc::channel(); + let worker = thread::spawn(move || { + while let Ok(first) = update_rx.recv() { + let batch = coalesce_updates(first, update_rx.try_iter()); + for update in batch { + let result = worker_provider.set(&update.path, update.viewed); + let completion = ViewedCompletion { + path: update.path, + viewed: update.viewed, + result, + }; + let _ = completion_tx.send((update.sequence, completion)); + } + } + }); + Some(Self { + provider, + updates: Some(update_tx), + completions: completion_rx, + pending: HashMap::new(), + next_sequence: 0, + local_completions: VecDeque::new(), + worker: Some(worker), + }) + } + + pub fn set(&mut self, path: &str, viewed: bool) { + let path = path.to_string(); + let sequence = self.next_sequence; + self.next_sequence = self.next_sequence.wrapping_add(1); + self.pending.insert(path.clone(), (sequence, viewed)); + let send_failed = self.updates.as_ref().is_none_or(|updates| { + updates + .send(ViewedUpdate { + sequence, + path: path.clone(), + viewed, + }) + .is_err() + }); + if send_failed { + self.pending.remove(&path); + self.local_completions.push_back(ViewedCompletion { + path, + viewed, + result: Err(PrError::Other( + "viewed status synchronization worker stopped".to_string(), + )), + }); + } + } + + pub fn viewed_paths(&self) -> Result, PrError> { + let mut paths = self.provider.fetch()?; + for (path, (_, viewed)) in &self.pending { + if *viewed { + paths.insert(path.clone()); + } else { + paths.remove(path); + } + } + Ok(paths) + } + + pub fn drain(&mut self) -> Vec { + let mut current = self.local_completions.drain(..).collect::>(); + while let Ok((sequence, completion)) = self.completions.try_recv() { + if self.pending.get(&completion.path) == Some(&(sequence, completion.viewed)) { + self.pending.remove(&completion.path); + current.push(completion); + } + } + current + } +} + +impl Drop for ViewedFileSync { + fn drop(&mut self) { + self.updates.take(); + if let Some(worker) = self.worker.take() { + let _ = worker.join(); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn update(sequence: u64, path: &str, viewed: bool) -> ViewedUpdate { + ViewedUpdate { + sequence, + path: path.to_string(), + viewed, + } + } + + #[test] + fn coalesces_queued_updates_to_latest_value_per_path() { + let batch = coalesce_updates( + update(1, "a.rs", true), + [ + update(2, "b.rs", true), + update(3, "a.rs", false), + update(4, "a.rs", true), + ] + .into_iter(), + ); + + assert_eq!(batch.len(), 2); + assert_eq!((batch[0].sequence, batch[0].viewed), (4, true)); + assert_eq!((batch[1].sequence, batch[1].path.as_str()), (2, "b.rs")); + } +} diff --git a/src/command/diff/render/footer.rs b/src/command/diff/render/footer.rs index 47610cbb..af3e58ce 100644 --- a/src/command/diff/render/footer.rs +++ b/src/command/diff/render/footer.rs @@ -144,21 +144,21 @@ pub fn render_footer(frame: &mut Frame, footer_area: Rect, data: FooterData) { let left_spans = if let Some(pr) = data.pr_info { // PR mode: show "base <- head #123" or "owner:base <- owner:head #123" for forks - let is_fork = pr.head_repo_owner.as_ref() != Some(&pr.base_repo_owner); + let is_fork = pr.head_repo_owner() != Some(pr.base_repo_owner()); let base_label = if is_fork { - format!(" {}:{} ", pr.base_repo_owner, pr.base_ref) + format!(" {}:{} ", pr.base_repo_owner(), pr.base_ref()) } else { - format!(" {} ", pr.base_ref) + format!(" {} ", pr.base_ref()) }; let head_label = if is_fork { - match &pr.head_repo_owner { - Some(owner) => format!(" {}:{} ", owner, pr.head_ref), - None => format!(" {} ", pr.head_ref), // Fork was deleted + match pr.head_repo_owner() { + Some(owner) => format!(" {}:{} ", owner, pr.head_ref()), + None => format!(" {} ", pr.head_ref()), // Fork was deleted } } else { - format!(" {} ", pr.head_ref) + format!(" {} ", pr.head_ref()) }; let mut spans = vec![ diff --git a/src/config/cli.rs b/src/config/cli.rs index b34bd815..e7b90026 100644 --- a/src/config/cli.rs +++ b/src/config/cli.rs @@ -108,11 +108,13 @@ pub enum Commands { /// Launch interactive side-by-side diff viewer Diff { /// Commit reference: SHA, HEAD, HEAD~3..HEAD, main..feature, main...feature - /// Can also be a PR number or URL (e.g., 123 or https://github.com/owner/repo/pull/123) + /// Can also be a PR number or URL (GitHub: 123 or + /// https://github.com/owner/repo/pull/123; Azure DevOps: + /// https://dev.azure.com/org/project/_git/repo/pullrequest/123) #[arg(value_parser = clap::value_parser!(CommitReference))] reference: Option, - /// View a GitHub pull request (number or URL) + /// View a pull request (number or URL; GitHub via `gh`, Azure DevOps via its REST API) #[arg(long)] pr: Option, @@ -140,7 +142,7 @@ pub enum Commands { #[arg(long)] focus: Option, - /// Origin repository in owner/repo format (default: origin git remote) + /// Repository override: GitHub owner/repo, or a full Azure DevOps repository remote URL #[arg(long)] origin: Option, diff --git a/src/main.rs b/src/main.rs index b335bf5e..e8a2fa2d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -40,7 +40,7 @@ async fn run() -> Result<(), LumenError> { // Get VCS backend based on CLI override or auto-detection let cwd = std::env::current_dir()?; let vcs_override = cli.vcs.map(VcsBackendType::from); - let backend = vcs::get_backend(&cwd, vcs_override)?; + let backend = vcs::get_backend(&cwd, vcs_override); match cli.command { Commands::Explain { @@ -49,6 +49,7 @@ async fn run() -> Result<(), LumenError> { query, list, } => { + let backend = backend?; let git_entity = if list { let sha = LumenCommand::get_sha_from_fzf(backend.as_ref())?; let info = backend.get_commit(&sha)?; @@ -98,6 +99,7 @@ async fn run() -> Result<(), LumenError> { .await?; } Commands::List => { + let backend = backend?; eprintln!("Warning: 'lumen list' is deprecated. Use 'lumen explain --list' instead."); command .execute(command::CommandType::List { @@ -106,6 +108,7 @@ async fn run() -> Result<(), LumenError> { .await? } Commands::Draft { context } => { + let backend = backend?; // Draft always uses staged diff (git convention) let diff = backend.get_working_tree_diff(true)?; let git_entity = GitEntity::Diff(Diff::from_working_tree_diff(diff, true)?); @@ -134,6 +137,11 @@ async fn run() -> Result<(), LumenError> { origin, wrap, } => { + let backend = match backend { + Ok(backend) => Some(backend), + Err(vcs::VcsError::NotARepository) => None, + Err(error) => return Err(error.into()), + }; let options = command::diff::DiffOptions { reference, pr, @@ -146,7 +154,7 @@ async fn run() -> Result<(), LumenError> { origin, wrap: wrap || config.wrap.unwrap_or(false), }; - command::diff::run_diff_ui(options, backend.as_ref())?; + command::diff::run_diff_ui(options, backend.as_deref())?; } Commands::Configure => { command::configure::ConfigureCommand::execute()?; diff --git a/src/vcs/backend.rs b/src/vcs/backend.rs index 43743a90..6fa7aa84 100644 --- a/src/vcs/backend.rs +++ b/src/vcs/backend.rs @@ -83,6 +83,17 @@ pub trait VcsBackend { /// Get current branch name (or bookmark for jj). fn get_current_branch(&self) -> Result, VcsError>; + /// Resolve the source branch used to detect a pull request. + /// Backends may infer a nearby branch when the working copy is detached + /// from the branch ref itself. + fn get_pr_source_branch(&self) -> Result, VcsError> { + self.get_current_branch() + } + + /// Get the canonical URL of the `origin` remote, when the backend exposes + /// remotes. Backends without a remote concept return `None`. + fn origin_url(&self) -> Result, VcsError>; + /// Get commit log formatted for fzf selection. fn get_commit_log_for_fzf(&self) -> Result; diff --git a/src/vcs/git.rs b/src/vcs/git.rs index 4c9a6997..78a1815f 100644 --- a/src/vcs/git.rs +++ b/src/vcs/git.rs @@ -137,16 +137,15 @@ impl GitBackend { /// Open a git repository at the given path. /// Uses git2::Repository::discover to find the repo from any subdirectory. pub fn new(path: &Path) -> Result { - let discover_result = Repository::discover(path);//map_err(|_| VcsError::NotARepository)?; + let discover_result = Repository::discover(path); //map_err(|_| VcsError::NotARepository)?; return match discover_result { - Ok(repo) => Ok(GitBackend { repo }), + Ok(repo) => Ok(GitBackend { repo }), Err(error) => { // Print libgit2 so there is a chance to diagnose any errors with git println!("Error on repository discovery: {error:?}"); - return Err(VcsError::NotARepository) - }, + return Err(VcsError::NotARepository); + } }; - } /// Open a git repository from the current working directory. @@ -537,6 +536,17 @@ impl VcsBackend for GitBackend { } } + fn origin_url(&self) -> Result, VcsError> { + match self.repo.find_remote("origin") { + Ok(remote) => Ok(remote.url().map(str::to_owned)), + Err(error) if error.code() == git2::ErrorCode::NotFound => Ok(None), + Err(error) => Err(VcsError::Other(format!( + "failed to read origin remote: {}", + error + ))), + } + } + fn get_commit_log_for_fzf(&self) -> Result { let mut revwalk = self .repo @@ -843,6 +853,21 @@ mod tests { assert!(branch.is_some()); } + #[test] + fn test_origin_url_reads_origin_remote() { + let repo = RepoGuard::new(); + let raw_repo = Repository::open(&repo.dir).expect("should open repo"); + raw_repo + .remote("origin", "git@github.com:owner/repo.git") + .expect("should add origin"); + let backend = GitBackend::from_cwd().expect("should open repo"); + + assert_eq!( + backend.origin_url().expect("should read origin").as_deref(), + Some("git@github.com:owner/repo.git") + ); + } + #[test] fn test_get_file_content_at_ref() { let _repo = RepoGuard::new(); diff --git a/src/vcs/jj.rs b/src/vcs/jj.rs index 3b95af7b..4b68db93 100644 --- a/src/vcs/jj.rs +++ b/src/vcs/jj.rs @@ -23,8 +23,8 @@ use jj_lib::repo::{ReadonlyRepo, Repo, StoreFactories}; use jj_lib::repo_path::RepoPath; use jj_lib::repo_path::RepoPathUiConverter; use jj_lib::revset::{ - RevsetAliasesMap, RevsetDiagnostics, RevsetExtensions, RevsetParseContext, - RevsetWorkspaceContext, SymbolResolver, SymbolResolverExtension, + ResolvedRevsetExpression, RevsetAliasesMap, RevsetDiagnostics, RevsetExtensions, + RevsetParseContext, RevsetWorkspaceContext, SymbolResolver, SymbolResolverExtension, }; use jj_lib::settings::UserSettings; use jj_lib::time_util::DatePatternContext; @@ -715,22 +715,107 @@ impl VcsBackend for JjBackend { } fn get_current_branch(&self) -> Result, VcsError> { - // jj uses "bookmarks" instead of "branches" - // Find a bookmark that points to the working copy commit (@) let wc_commit = self.resolve_single_commit("@")?; let wc_commit_id = wc_commit.id(); - - // Iterate local bookmarks to find one pointing to @ - for (name, target) in self.repo.view().local_bookmarks() { - if let Some(commit_id) = target.as_normal() { - if commit_id == wc_commit_id { - return Ok(Some(name.as_str().to_string())); - } + Ok(self + .repo + .view() + .local_bookmarks() + .find_map(|(name, target)| { + target + .as_normal() + .filter(|commit_id| *commit_id == wc_commit_id) + .map(|_| name.as_str().to_string()) + })) + } + + fn get_pr_source_branch(&self) -> Result, VcsError> { + let wc_commit = self.resolve_single_commit("@")?; + let wc_commit_id = wc_commit.id(); + let local_bookmarks: Vec<_> = self + .repo + .view() + .local_bookmarks() + .filter_map(|(name, target)| { + target + .as_normal() + .map(|commit_id| (name.as_str().to_string(), commit_id.clone())) + }) + .collect(); + + let select_unique = |mut names: Vec, location: &str| { + names.sort(); + match names.as_slice() { + [] => Ok(None), + [name] => Ok(Some(name.clone())), + _ => Err(VcsError::Other(format!( + "ambiguous current jj bookmark: {} {}. Move or delete bookmarks so only one candidate remains, or specify the PR explicitly", + location, + names.join(", ") + ))), } + }; + + let exact_names = local_bookmarks + .iter() + .filter(|(_, commit_id)| commit_id == wc_commit_id) + .map(|(name, _)| name.clone()) + .collect::>(); + if !exact_names.is_empty() { + return select_unique(exact_names, "local bookmarks pointing exactly at @:"); + } + + if local_bookmarks.is_empty() { + return Ok(None); } - // No bookmark points to @ - Ok(None) + let bookmark_commit_ids = local_bookmarks + .iter() + .map(|(_, commit_id)| commit_id.clone()) + .collect(); + let nearest_bookmarks = ResolvedRevsetExpression::commit(wc_commit_id.clone()) + .ancestors() + .intersection(&ResolvedRevsetExpression::commits(bookmark_commit_ids)) + .heads() + .evaluate(self.repo.as_ref()) + .map_err(|error| { + VcsError::Other(format!( + "failed to find the nearest local jj bookmark: {}", + error + )) + })?; + let nearest_commit_ids = nearest_bookmarks + .iter() + .collect::, _>>() + .map_err(|error| { + VcsError::Other(format!( + "failed to inspect the nearest local jj bookmark: {}", + error + )) + })?; + let nearest_names = local_bookmarks + .into_iter() + .filter(|(_, commit_id)| nearest_commit_ids.contains(commit_id)) + .map(|(name, _)| name) + .collect(); + + select_unique(nearest_names, "nearest local bookmarks reachable from @:") + } + + fn origin_url(&self) -> Result, VcsError> { + let backend = jj_lib::git::get_git_backend(self.repo.store()) + .map_err(|error| VcsError::Other(error.to_string()))?; + let repository = git2::Repository::open(backend.git_repo_path()) + .map_err(|error| VcsError::Other(format!("failed to open jj Git store: {}", error)))?; + let origin = match repository.find_remote("origin") { + Ok(remote) => Ok(remote.url().map(str::to_owned)), + Err(error) if error.code() == git2::ErrorCode::NotFound => Ok(None), + Err(error) => Err(VcsError::Other(format!( + "failed to read jj origin remote: {}", + error + ))), + }; + origin } fn resolve_ref(&self, reference: &str) -> Result { @@ -1307,10 +1392,114 @@ mod tests { // Default jj repo has no bookmarks assert!( branch.unwrap().is_none(), - "should return None when no bookmark points to @" + "should return None when no bookmark is reachable from @" ); } + #[test] + fn test_get_current_branch_prefers_unique_bookmark_at_working_copy() { + let Some(repo) = JjRepoGuard::new() else { + eprintln!("Skipping test: jj not available"); + return; + }; + + assert!(crate::vcs::test_utils::jj( + &repo.dir, + &["bookmark", "create", "parent"] + )); + assert!(crate::vcs::test_utils::jj(&repo.dir, &["new"])); + assert!(crate::vcs::test_utils::jj( + &repo.dir, + &["bookmark", "create", "current"] + )); + + let backend = JjBackend::new(&repo.dir).expect("should load backend"); + assert_eq!( + backend.get_current_branch().expect("should get bookmark"), + Some("current".to_string()) + ); + } + + #[test] + fn test_get_pr_source_branch_finds_nearest_unique_ancestor_bookmark() { + let Some(repo) = JjRepoGuard::new() else { + eprintln!("Skipping test: jj not available"); + return; + }; + + assert!(crate::vcs::test_utils::jj( + &repo.dir, + &["bookmark", "create", "older"] + )); + assert!(crate::vcs::test_utils::jj(&repo.dir, &["new"])); + assert!(crate::vcs::test_utils::jj( + &repo.dir, + &["bookmark", "create", "feature"] + )); + assert!(crate::vcs::test_utils::jj(&repo.dir, &["new"])); + + let backend = JjBackend::new(&repo.dir).expect("should load backend"); + assert_eq!( + backend.get_pr_source_branch().expect("should get bookmark"), + Some("feature".to_string()) + ); + } + + #[test] + fn test_get_pr_source_branch_rejects_ambiguous_exact_bookmarks() { + let Some(repo) = JjRepoGuard::new() else { + eprintln!("Skipping test: jj not available"); + return; + }; + + assert!(crate::vcs::test_utils::jj( + &repo.dir, + &["bookmark", "create", "feature-b"] + )); + assert!(crate::vcs::test_utils::jj( + &repo.dir, + &["bookmark", "create", "feature-a"] + )); + + let backend = JjBackend::new(&repo.dir).expect("should load backend"); + let error = backend + .get_pr_source_branch() + .expect_err("ambiguous bookmarks should fail"); + let VcsError::Other(message) = error else { + panic!("expected VcsError::Other, got {error:?}"); + }; + assert!(message.contains("feature-a, feature-b"), "{message}"); + assert!(message.contains("pointing exactly at @"), "{message}"); + } + + #[test] + fn test_get_pr_source_branch_rejects_ambiguous_nearest_bookmarks() { + let Some(repo) = JjRepoGuard::new() else { + eprintln!("Skipping test: jj not available"); + return; + }; + + assert!(crate::vcs::test_utils::jj( + &repo.dir, + &["bookmark", "create", "feature-b"] + )); + assert!(crate::vcs::test_utils::jj( + &repo.dir, + &["bookmark", "create", "feature-a"] + )); + assert!(crate::vcs::test_utils::jj(&repo.dir, &["new"])); + + let backend = JjBackend::new(&repo.dir).expect("should load backend"); + let error = backend + .get_pr_source_branch() + .expect_err("ambiguous bookmarks should fail"); + let VcsError::Other(message) = error else { + panic!("expected VcsError::Other, got {error:?}"); + }; + assert!(message.contains("feature-a, feature-b"), "{message}"); + assert!(message.contains("nearest local bookmarks"), "{message}"); + } + #[test] fn test_diff_excludes_lock_files() { use std::fs;