diff --git a/src/app.rs b/src/app.rs index 352933f0..a6bff690 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,5 +1,7 @@ use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::sync::mpsc::TryRecvError; use std::time::{Duration, Instant, SystemTime}; use chrono::Utc; @@ -17,14 +19,15 @@ use crate::model::{ }; use crate::persistence::load_latest_session_for_context; use crate::review_store::{AddCommentRequest, CommentTarget, add_comment_to_session}; -use crate::syntax::SyntaxHighlighter; +use crate::syntax::streaming::{self, HighlightJob, HighlightSession}; use crate::theme::Theme; use crate::update::UpdateInfo; use crate::vcs::git::calculate_gap; use crate::vcs::traits::VcsType; use crate::vcs::{ - ChangeKind, CommitInfo, DiffWhitespaceMode, FileBackend, GitBackendPreference, PrNoopVcs, - ResolvedRevisionRange, RevisionDiffTarget, VcsBackend, VcsChangeStatus, VcsInfo, detect_vcs, + ChangeKind, CommitInfo, DiffWhitespaceMode, DiffWithJobs, FileBackend, GitBackendPreference, + PrNoopVcs, ResolvedRevisionRange, RevisionDiffTarget, VcsBackend, VcsChangeStatus, VcsInfo, + detect_vcs, }; const VISIBLE_COMMIT_COUNT: usize = 10; @@ -91,9 +94,9 @@ fn gap_annotation_line_count( } } -fn profile_diff_result(result: &Result>) -> String { +fn profile_diff_result(result: &Result) -> String { match result { - Ok(files) => format!("files={}", files.len()), + Ok((files, _)) => format!("files={}", files.len()), Err(e) => format!("error={e}"), } } @@ -924,12 +927,31 @@ pub struct App { pub diff_source: DiffSource, pub pending_editor_target: Option, + /// Background syntax-highlight worker. `None` once the current diff is + /// fully highlighted (or had nothing to highlight). Replacing the diff + /// cancels the old session so its in-flight work is discarded. + pub(crate) highlight_session: Option, + + /// Jobs handed to `install_diff` but not yet given to a worker. Holding + /// them here lets follow-up `diff_files` reorderings (commit-message + /// insert, directory sort) settle before the worker starts emitting + /// updates keyed by `file_idx`; the first `drain_highlight_updates` call + /// resolves indices by `syntax_path` and spawns the worker. + pub(crate) pending_highlight_jobs: Vec, + pub input_mode: InputMode, pub focused_panel: FocusedPanel, pub diff_view_mode: DiffViewMode, pub file_list_state: FileListState, pub comment_navigator_state: CommentNavigatorState, + /// True while the user has manually scrolled the file list independently + /// of the diff cursor. Suppresses the render-time "sync selection to + /// diff's current file" path so wheel scrolls aren't snapped back by + /// ratatui's auto-adjust-on-selection-change. Cleared when something + /// else changes the diff's current file (cursor moving into a new file, + /// click jump, } / { navigation). + pub manual_file_list_scroll: bool, pub diff_state: DiffState, pub help_state: HelpState, pub command_buffer: String, @@ -1384,8 +1406,7 @@ impl App { if let Some(file_path) = options.file_path { let vcs = Box::new(FileBackend::new(file_path)?); let vcs_info = vcs.info().clone(); - let highlighter = theme.syntax_highlighter(); - let diff_files = vcs.get_working_tree_diff(highlighter)?; + let diff = vcs.get_working_tree_diff()?; let session = Self::load_or_create_session(&vcs_info, SessionDiffSource::WorkingTree); let mut app = Self::build( @@ -1394,7 +1415,7 @@ impl App { theme, comment_type_configs, output_to_stdout, - diff_files, + diff, session, DiffSource::WorkingTree, InputMode::Normal, @@ -1435,14 +1456,13 @@ impl App { let vcs = Box::new(FileBackend::new_pristine(paths, cwd.clone())?); let mut vcs_info = vcs.info().clone(); vcs_info.head_commit = base_commit; - let highlighter = theme.syntax_highlighter(); - let diff_files = vcs.get_working_tree_diff(highlighter)?; + let diff = vcs.get_working_tree_diff()?; // `git ls-files` already honors `.gitignore`, but `.tuicrignore` // is tuicr-specific and not known to git. Run the same post-VCS // filter every other mode uses so users can elide tracked-but- // boring files (lockfiles, generated docs) from the review surface. - let diff_files = Self::filter_ignored_diff_files(&cwd, diff_files); - if diff_files.is_empty() { + let diff = Self::filter_ignored_diff(&cwd, diff); + if diff.0.is_empty() { return Err(TuicrError::NoChanges); } let session = Self::load_or_create_session(&vcs_info, SessionDiffSource::Pristine); @@ -1453,7 +1473,7 @@ impl App { theme, comment_type_configs, output_to_stdout, - diff_files, + diff, session, DiffSource::WorkingTree, InputMode::Normal, @@ -1485,8 +1505,6 @@ impl App { detect_vcs(options.git_backend_preference, options.diff_whitespace_mode) })?; let vcs_info = vcs.info().clone(); - let highlighter = - crate::profile::time("startup.syntax_highlighter", || theme.syntax_highlighter()); // Determine the diff source, files, and session based on input. // Four paths: // 1. -r + -w: combined commit range and uncommitted changes @@ -1506,11 +1524,10 @@ impl App { if options.working_tree { // Combined: commit range + staged/unstaged changes - let diff_files = Self::get_working_tree_with_commits_diff_with_ignore( + let diff = Self::get_working_tree_with_commits_diff_with_ignore( vcs.as_ref(), &vcs_info.root_path, &commit_ids, - highlighter, options.path_filter, )?; let session = Self::load_or_create_staged_unstaged_and_commits_session( @@ -1529,7 +1546,6 @@ impl App { let change_status = Self::get_change_status_with_ignore( vcs.as_ref(), &vcs_info.root_path, - highlighter, options.path_filter, )?; let mut all_commits = Vec::new(); @@ -1547,7 +1563,7 @@ impl App { theme, comment_type_configs.clone(), output_to_stdout, - diff_files, + diff, session, DiffSource::StagedUnstagedAndCommits(commit_ids), InputMode::Normal, @@ -1579,11 +1595,10 @@ impl App { } // Resolve the revisions to commits and diff as a commit range - let diff_files = Self::get_commit_range_diff_with_ignore( + let diff = Self::get_commit_range_diff_with_ignore( vcs.as_ref(), &vcs_info.root_path, &revision_range, - highlighter, options.path_filter, )?; let session = Self::load_or_create_commit_range_session(&vcs_info, &commit_ids); @@ -1602,7 +1617,7 @@ impl App { theme, comment_type_configs.clone(), output_to_stdout, - diff_files, + diff, session, DiffSource::CommitRange(commit_ids), InputMode::Normal, @@ -1632,10 +1647,9 @@ impl App { Ok(app) } else if options.working_tree { // Skip commit selector, go straight to working tree diff - let diff_files = Self::get_working_tree_diff_with_ignore( + let diff = Self::get_working_tree_diff_with_ignore( vcs.as_ref(), &vcs_info.root_path, - highlighter, options.path_filter, )?; let session = @@ -1647,7 +1661,7 @@ impl App { theme, comment_type_configs, output_to_stdout, - diff_files, + diff, session, DiffSource::StagedAndUnstaged, InputMode::Normal, @@ -1661,7 +1675,6 @@ impl App { let change_status = Self::get_change_status_with_ignore( vcs.as_ref(), &vcs_info.root_path, - highlighter, options.path_filter, )?; let has_staged_changes = change_status.staged; @@ -1717,7 +1730,7 @@ impl App { theme, comment_type_configs, output_to_stdout, - Vec::new(), + (Vec::new(), Vec::new()), session, diff_source, InputMode::CommitSelect, @@ -1744,7 +1757,7 @@ impl App { theme: Theme, comment_type_configs: Option>, output_to_stdout: bool, - diff_files: Vec, + diff: DiffWithJobs, mut session: ReviewSession, diff_source: DiffSource, input_mode: InputMode, @@ -1757,7 +1770,7 @@ impl App { // hunk keys alive until the selected diff is loaded. let preserve_hunks = matches!(diff_source, DiffSource::PullRequest(_)) && session.commit_selection_range.is_some(); - Self::register_diff_files(&mut session, &diff_files, preserve_hunks); + Self::register_diff_files(&mut session, &diff.0, preserve_hunks); let has_more_commit = commit_list.len() >= VISIBLE_COMMIT_COUNT; let visible_commit_count = if commit_list.is_empty() { @@ -1787,14 +1800,17 @@ impl App { next_review_watch_at: Instant::now() + Duration::from_millis(DEFAULT_REVIEW_WATCH_INTERVAL_MS), ephemeral_session_paths: HashSet::new(), - diff_files, + diff_files: Vec::new(), diff_source, pending_editor_target: None, + highlight_session: None, + pending_highlight_jobs: Vec::new(), input_mode, focused_panel: FocusedPanel::Diff, diff_view_mode: DiffViewMode::Unified, file_list_state: FileListState::default(), comment_navigator_state: CommentNavigatorState::default(), + manual_file_list_scroll: false, diff_state: DiffState::default(), help_state: HelpState::default(), command_buffer: String::new(), @@ -1892,6 +1908,7 @@ impl App { export_legend: true, }; // Auto-hide file list when path filter matches exactly one file + app.install_diff(diff); if app.path_filter.is_some() && app.diff_files.len() == 1 { app.show_file_list = false; app.focused_panel = FocusedPanel::Diff; @@ -2555,7 +2572,7 @@ impl App { persisted.commit_selection_range, opened.commits.len(), ); - Self::register_diff_files(&mut persisted, &opened.diff_files, preserve_hunks); + Self::register_diff_files(&mut persisted, &opened.diff.0, preserve_hunks); persisted.pr_session_key = Some(key); persisted.diff_source = SessionDiffSource::PullRequest; persisted.updated_at = chrono::Utc::now(); @@ -2655,12 +2672,10 @@ impl App { }); let backend = create_forge_backend(&target_repo, local_checkout_for_target.clone()); - let highlighter = theme.syntax_highlighter(); let mut opened = open_pull_request( backend.as_ref(), parsed, local_checkout_for_target.as_deref(), - highlighter, )?; Self::load_or_apply_pr_session(&mut opened); @@ -2686,7 +2701,7 @@ impl App { theme, comment_type_configs, output_to_stdout, - opened.diff_files, + opened.diff, opened.session, diff_source, InputMode::Normal, @@ -2724,7 +2739,7 @@ impl App { ) -> Result<()> { let crate::forge::pr_open::OpenedPullRequest { details, - diff_files, + diff, session, key, commits, @@ -2746,7 +2761,7 @@ impl App { }; self.vcs = Box::new(PrNoopVcs::new(self.vcs_info.clone())); self.session = session; - self.diff_files = diff_files; + self.install_diff(diff); self.reset_persisted_session_tracking(); self.diff_source = DiffSource::PullRequest(Box::new(pr_source)); self.forge_backend = Some(backend); @@ -3135,10 +3150,9 @@ impl App { ) -> Result<()> { use crate::vcs::diff_parser::{DiffFormat, parse_unified_diff}; - let highlighter = self.theme.syntax_highlighter(); - let parsed = match parse_unified_diff(patch, DiffFormat::GitStyle, highlighter) { - Ok(files) => files, - Err(TuicrError::NoChanges) => Vec::new(), + let parsed = match parse_unified_diff(patch, DiffFormat::GitStyle) { + Ok(diff) => diff, + Err(TuicrError::NoChanges) => (Vec::new(), Vec::new()), Err(e) => return Err(e), }; @@ -3146,12 +3160,12 @@ impl App { .forge_backend .as_deref() .and_then(|b| b.local_checkout_path()); - let files = match local_checkout.as_deref() { + let diff = match local_checkout.as_deref() { Some(root) => crate::tuicrignore::filter_diff_files(root, parsed), None => parsed, }; - self.diff_files = files; + self.install_diff(diff); self.clear_expanded_gaps(); // Range diffs can hide hunks that are still reviewed in the broader // PR session, so registration must not prune them. @@ -3261,14 +3275,7 @@ impl App { .forge_backend .as_deref() .and_then(|backend| backend.local_checkout_path()); - let highlighter = self.theme.syntax_highlighter(); - let mut opened = prepare_open_pr( - details, - &patch, - commits, - local_checkout.as_deref(), - highlighter, - )?; + let mut opened = prepare_open_pr(details, &patch, commits, local_checkout.as_deref())?; let head_changed = opened.details.head_sha != request.head_sha; if head_changed { @@ -3280,7 +3287,7 @@ impl App { self.spawn_pr_threads_fetch(&details_for_threads, local_checkout); self.set_message("Reloaded PR at new head — switched to fresh session".to_string()); } else { - self.diff_files = opened.diff_files; + self.install_diff(opened.diff); self.clear_expanded_gaps(); for file in &self.diff_files { self.session.add_diff_file(file); @@ -3338,13 +3345,7 @@ impl App { current.key.number, current.key.number.to_string(), ); - let highlighter = self.theme.syntax_highlighter(); - let mut opened = open_pull_request( - backend.as_ref(), - target, - local_checkout.as_deref(), - highlighter, - )?; + let mut opened = open_pull_request(backend.as_ref(), target, local_checkout.as_deref())?; let head_changed = opened.details.head_sha != current.key.head_sha; if head_changed { @@ -3359,7 +3360,7 @@ impl App { } else { // Same head: re-parse the diff to pick up any side-channel // changes (rare), but keep the session intact. - self.diff_files = opened.diff_files; + self.install_diff(opened.diff); self.clear_expanded_gaps(); for file in &self.diff_files { self.session.add_diff_file(file); @@ -3484,136 +3485,163 @@ impl App { .saturating_sub(self.special_commit_count()) } - fn filter_ignored_diff_files(repo_root: &Path, diff_files: Vec) -> Vec { - crate::tuicrignore::filter_diff_files(repo_root, diff_files) + fn filter_ignored_diff(repo_root: &Path, diff: DiffWithJobs) -> DiffWithJobs { + crate::tuicrignore::filter_diff_files(repo_root, diff) } - fn filter_by_path(diff_files: Vec, path: &str) -> Vec { - let path = path.trim_end_matches('/'); - diff_files - .into_iter() - .filter(|f| { - let display = f.display_path().to_string_lossy(); - display == path || display.starts_with(&format!("{path}/")) - }) - .collect() + fn filter_by_path(diff: DiffWithJobs, path: &str) -> DiffWithJobs { + let path = path.trim_end_matches('/').to_string(); + crate::vcs::filter_diff_with_jobs(diff, |f| { + let display = f.display_path().to_string_lossy(); + display == path || display.starts_with(&format!("{path}/")) + }) } - fn require_non_empty_diff_files(diff_files: Vec) -> Result> { - if diff_files.is_empty() { + fn finalize_diff( + diff: DiffWithJobs, + repo_root: &Path, + path_filter: Option<&str>, + ) -> Result { + let diff = Self::filter_ignored_diff(repo_root, diff); + let diff = match path_filter { + Some(path) => Self::filter_by_path(diff, path), + None => diff, + }; + if diff.0.is_empty() { return Err(TuicrError::NoChanges); } - Ok(diff_files) + Ok(diff) + } + + /// Replace the current diff in place. Cancels any prior highlight session + /// and stages the new jobs; the worker is spawned on the first + /// `drain_highlight_updates` call so any `diff_files` mutations queued for + /// after this call settle before updates start flowing. + fn install_diff(&mut self, diff: DiffWithJobs) { + if let Some(session) = self.highlight_session.take() { + session.cancel(); + } + let (files, jobs) = diff; + self.diff_files = files; + self.pending_highlight_jobs = jobs; + } + + fn start_pending_highlight_session(&mut self) { + if self.highlight_session.is_some() || self.pending_highlight_jobs.is_empty() { + return; + } + let jobs = std::mem::take(&mut self.pending_highlight_jobs); + let highlighter_arc = Arc::clone(self.theme.syntax_highlighter_arc()); + self.highlight_session = + HighlightSession::start_resolved(jobs, &self.diff_files, highlighter_arc); + } + + /// Drain pending highlight results and patch them into `diff_files`. + /// Spawns the worker lazily on first call after `install_diff`, so all + /// synchronous `diff_files` reorderings have completed before any update + /// is produced. + pub fn drain_highlight_updates(&mut self) { + self.start_pending_highlight_session(); + let Some(session) = self.highlight_session.as_ref() else { + return; + }; + loop { + match session.try_recv() { + Ok(update) => streaming::apply_update( + &mut self.diff_files, + self.theme.syntax_highlighter(), + update, + ), + Err(TryRecvError::Empty) => break, + Err(TryRecvError::Disconnected) => { + self.highlight_session = None; + break; + } + } + } + } + + /// True while the background highlight worker still has pending results, + /// or while jobs are staged awaiting their first drain. Used by the main + /// loop to shorten its event-poll timeout so streamed results land on + /// screen with low latency. + pub fn highlight_streaming(&self) -> bool { + self.highlight_session.is_some() || !self.pending_highlight_jobs.is_empty() } fn get_working_tree_diff_with_ignore( vcs: &dyn VcsBackend, repo_root: &Path, - highlighter: &SyntaxHighlighter, path_filter: Option<&str>, - ) -> Result> { - let diff_files = crate::profile::time_with( + ) -> Result { + let diff = crate::profile::time_with( "diff.load_working_tree", - || vcs.get_working_tree_diff(highlighter), + || vcs.get_working_tree_diff(), profile_diff_result, )?; - let diff_files = Self::filter_ignored_diff_files(repo_root, diff_files); - let diff_files = if let Some(path) = path_filter { - Self::filter_by_path(diff_files, path) - } else { - diff_files - }; - Self::require_non_empty_diff_files(diff_files) + Self::finalize_diff(diff, repo_root, path_filter) } fn get_staged_diff_with_ignore( vcs: &dyn VcsBackend, repo_root: &Path, - highlighter: &SyntaxHighlighter, path_filter: Option<&str>, - ) -> Result> { - let diff_files = crate::profile::time_with( + ) -> Result { + let diff = crate::profile::time_with( "diff.load_staged", - || vcs.get_staged_diff(highlighter), + || vcs.get_staged_diff(), profile_diff_result, )?; - let diff_files = Self::filter_ignored_diff_files(repo_root, diff_files); - let diff_files = if let Some(path) = path_filter { - Self::filter_by_path(diff_files, path) - } else { - diff_files - }; - Self::require_non_empty_diff_files(diff_files) + Self::finalize_diff(diff, repo_root, path_filter) } fn get_unstaged_diff_with_ignore( vcs: &dyn VcsBackend, repo_root: &Path, - highlighter: &SyntaxHighlighter, path_filter: Option<&str>, - ) -> Result> { - let diff_files = match crate::profile::time_with( + ) -> Result { + let diff = match crate::profile::time_with( "diff.load_unstaged", - || vcs.get_unstaged_diff(highlighter), + || vcs.get_unstaged_diff(), profile_diff_result, ) { - Ok(diff_files) => diff_files, + Ok(d) => d, Err(TuicrError::UnsupportedOperation(_)) => crate::profile::time_with( "diff.load_unstaged_fallback_working_tree", - || vcs.get_working_tree_diff(highlighter), + || vcs.get_working_tree_diff(), profile_diff_result, )?, Err(e) => return Err(e), }; - let diff_files = Self::filter_ignored_diff_files(repo_root, diff_files); - let diff_files = if let Some(path) = path_filter { - Self::filter_by_path(diff_files, path) - } else { - diff_files - }; - Self::require_non_empty_diff_files(diff_files) + Self::finalize_diff(diff, repo_root, path_filter) } fn get_commit_range_diff_with_ignore( vcs: &dyn VcsBackend, repo_root: &Path, revision_range: &ResolvedRevisionRange<'_>, - highlighter: &SyntaxHighlighter, path_filter: Option<&str>, - ) -> Result> { - let diff_files = crate::profile::time_with( + ) -> Result { + let diff = crate::profile::time_with( "diff.load_commit_range", - || vcs.get_commit_range_diff(revision_range, highlighter), + || vcs.get_commit_range_diff(revision_range), profile_diff_result, )?; - let diff_files = Self::filter_ignored_diff_files(repo_root, diff_files); - let diff_files = if let Some(path) = path_filter { - Self::filter_by_path(diff_files, path) - } else { - diff_files - }; - Self::require_non_empty_diff_files(diff_files) + Self::finalize_diff(diff, repo_root, path_filter) } fn get_working_tree_with_commits_diff_with_ignore( vcs: &dyn VcsBackend, repo_root: &Path, commit_ids: &[String], - highlighter: &SyntaxHighlighter, path_filter: Option<&str>, - ) -> Result> { - let diff_files = crate::profile::time_with( + ) -> Result { + let diff = crate::profile::time_with( "diff.load_working_tree_with_commits", - || vcs.get_working_tree_with_commits_diff(commit_ids, highlighter), + || vcs.get_working_tree_with_commits_diff(commit_ids), profile_diff_result, )?; - let diff_files = Self::filter_ignored_diff_files(repo_root, diff_files); - let diff_files = if let Some(path) = path_filter { - Self::filter_by_path(diff_files, path) - } else { - diff_files - }; - Self::require_non_empty_diff_files(diff_files) + Self::finalize_diff(diff, repo_root, path_filter) } /// Resolve the staged/unstaged status the commit selector renders. @@ -3626,7 +3654,6 @@ impl App { fn get_change_status_with_ignore( vcs: &dyn VcsBackend, repo_root: &Path, - highlighter: &SyntaxHighlighter, path_filter: Option<&str>, ) -> Result { if path_filter.is_none() { @@ -3635,13 +3662,7 @@ impl App { if !crate::tuicrignore::has_ignore_rules(repo_root) { return Ok(status); } - return Self::verify_status_against_ignore( - vcs, - repo_root, - highlighter, - path_filter, - status, - ); + return Self::verify_status_against_ignore(vcs, repo_root, path_filter, status); } Err(TuicrError::UnsupportedOperation(_)) => {} Err(e) => return Err(e), @@ -3651,7 +3672,6 @@ impl App { Self::verify_status_against_ignore( vcs, repo_root, - highlighter, path_filter, VcsChangeStatus { staged: true, @@ -3666,29 +3686,16 @@ impl App { fn verify_status_against_ignore( vcs: &dyn VcsBackend, repo_root: &Path, - highlighter: &SyntaxHighlighter, path_filter: Option<&str>, assumed_status: VcsChangeStatus, ) -> Result { let staged = if assumed_status.staged { - Self::side_has_visible_changes( - vcs, - repo_root, - highlighter, - path_filter, - ChangeKind::Staged, - )? + Self::side_has_visible_changes(vcs, repo_root, path_filter, ChangeKind::Staged)? } else { false }; let unstaged = if assumed_status.unstaged { - Self::side_has_visible_changes( - vcs, - repo_root, - highlighter, - path_filter, - ChangeKind::Unstaged, - )? + Self::side_has_visible_changes(vcs, repo_root, path_filter, ChangeKind::Unstaged)? } else { false }; @@ -3698,7 +3705,6 @@ impl App { fn side_has_visible_changes( vcs: &dyn VcsBackend, repo_root: &Path, - highlighter: &SyntaxHighlighter, path_filter: Option<&str>, kind: ChangeKind, ) -> Result { @@ -3713,14 +3719,11 @@ impl App { // anything survives. This still happens for jj/hg today. let diff_result = match kind { ChangeKind::Staged => { - Self::get_staged_diff_with_ignore(vcs, repo_root, highlighter, path_filter) + Self::get_staged_diff_with_ignore(vcs, repo_root, path_filter) + } + ChangeKind::Unstaged => { + Self::get_unstaged_diff_with_ignore(vcs, repo_root, path_filter) } - ChangeKind::Unstaged => Self::get_unstaged_diff_with_ignore( - vcs, - repo_root, - highlighter, - path_filter, - ), }; match diff_result { Ok(_) => Ok(true), @@ -3759,14 +3762,12 @@ impl App { } fn load_staged_and_unstaged_selection(&mut self) -> Result<()> { - let highlighter = self.theme.syntax_highlighter(); - let diff_files = match Self::get_working_tree_diff_with_ignore( + let diff = match Self::get_working_tree_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), ) { - Ok(diff_files) => diff_files, + Ok(diff) => diff, Err(TuicrError::NoChanges) => { self.set_message("No staged or unstaged changes"); return Ok(()); @@ -3776,12 +3777,12 @@ impl App { self.session = Self::load_or_create_session(&self.vcs_info, SessionDiffSource::StagedAndUnstaged); - for file in &diff_files { + for file in &diff.0 { self.session.add_diff_file(file); } self.reset_persisted_session_tracking(); - self.diff_files = diff_files; + self.install_diff(diff); self.diff_source = DiffSource::StagedAndUnstaged; self.input_mode = InputMode::Normal; self.diff_state = DiffState::default(); @@ -3795,14 +3796,12 @@ impl App { } fn load_staged_selection(&mut self) -> Result<()> { - let highlighter = self.theme.syntax_highlighter(); - let diff_files = match Self::get_staged_diff_with_ignore( + let diff = match Self::get_staged_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), ) { - Ok(diff_files) => diff_files, + Ok(diff) => diff, Err(TuicrError::NoChanges) => { self.set_message("No staged changes"); return Ok(()); @@ -3811,12 +3810,12 @@ impl App { }; self.session = Self::load_or_create_session(&self.vcs_info, SessionDiffSource::Staged); - for file in &diff_files { + for file in &diff.0 { self.session.add_diff_file(file); } self.reset_persisted_session_tracking(); - self.diff_files = diff_files; + self.install_diff(diff); self.diff_source = DiffSource::Staged; self.input_mode = InputMode::Normal; self.diff_state = DiffState::default(); @@ -3830,14 +3829,12 @@ impl App { } fn load_unstaged_selection(&mut self) -> Result<()> { - let highlighter = self.theme.syntax_highlighter(); - let diff_files = match Self::get_unstaged_diff_with_ignore( + let diff = match Self::get_unstaged_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), ) { - Ok(diff_files) => diff_files, + Ok(diff) => diff, Err(TuicrError::NoChanges) => { self.set_message("No unstaged changes"); return Ok(()); @@ -3846,12 +3843,12 @@ impl App { }; self.session = Self::load_or_create_session(&self.vcs_info, SessionDiffSource::Unstaged); - for file in &diff_files { + for file in &diff.0 { self.session.add_diff_file(file); } self.reset_persisted_session_tracking(); - self.diff_files = diff_files; + self.install_diff(diff); self.diff_source = DiffSource::Unstaged; self.input_mode = InputMode::Normal; self.diff_state = DiffState::default(); @@ -3881,13 +3878,11 @@ impl App { prev_cursor_line.saturating_sub(start) }; - let highlighter = self.theme.syntax_highlighter(); - let diff_files = match &self.diff_source { + let diff = match &self.diff_source { DiffSource::CommitRange(commit_ids) => Self::get_commit_range_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, &ResolvedRevisionRange::from_commit_ids(commit_ids, RevisionDiffTarget::CommitList), - highlighter, self.path_filter.as_deref(), )?, DiffSource::StagedUnstagedAndCommits(commit_ids) => { @@ -3896,27 +3891,23 @@ impl App { self.vcs.as_ref(), &self.vcs_info.root_path, &ids, - highlighter, self.path_filter.as_deref(), )? } DiffSource::Staged => Self::get_staged_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), )?, DiffSource::Unstaged => Self::get_unstaged_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), )?, DiffSource::StagedAndUnstaged | DiffSource::WorkingTree => { Self::get_working_tree_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), )? } @@ -3932,13 +3923,13 @@ impl App { }; let mut invalidated = 0; - for file in &diff_files { + for file in &diff.0 { if self.session.add_diff_file(file) { invalidated += 1; } } - self.diff_files = diff_files; + self.install_diff(diff); self.clear_expanded_gaps(); self.sort_files_by_directory(false); @@ -5081,6 +5072,7 @@ impl App { if self.file_list_state.selected() < new_offset { self.file_list_state.select(new_offset); } + self.manual_file_list_scroll = true; } /// Scroll the file-list viewport up by `lines` without moving the @@ -5097,6 +5089,7 @@ impl App { if self.file_list_state.selected() > max_visible { self.file_list_state.select(max_visible); } + self.manual_file_list_scroll = true; } pub fn diff_annotation_at_screen_row(&self, screen_row: u16) -> Option { @@ -5630,7 +5623,9 @@ impl App { self.primed_walk_prev = false; self.down_released_since_arm = false; self.up_released_since_arm = false; + let prev = self.diff_state.current_file_idx; self.diff_state.current_file_idx = idx; + self.reprioritize_highlights_if_file_changed(prev); self.diff_state.cursor_line = self.calculate_file_scroll_offset(idx); let max_scroll = self.max_scroll_offset(); self.diff_state.scroll_offset = self.diff_state.cursor_line.min(max_scroll); @@ -6193,26 +6188,39 @@ impl App { if self.is_single_file_view { return; } + let prev = self.diff_state.current_file_idx; + if let Some(new_idx) = self.file_idx_at_cursor() { + self.diff_state.current_file_idx = new_idx; + self.file_list_state.select(new_idx); + } + self.reprioritize_highlights_if_file_changed(prev); + } + + fn file_idx_at_cursor(&self) -> Option { + if self.diff_files.is_empty() { + return None; + } let mut cumulative = self.review_comments_render_height(); if self.diff_state.cursor_line < cumulative { - if !self.diff_files.is_empty() { - self.diff_state.current_file_idx = 0; - self.file_list_state.select(0); - } - return; + return Some(0); } for (i, file) in self.diff_files.iter().enumerate() { let height = self.file_render_height(i, file); if cumulative + height > self.diff_state.cursor_line { - self.diff_state.current_file_idx = i; - self.file_list_state.select(i); - return; + return Some(i); } cumulative += height; } - if !self.diff_files.is_empty() { - self.diff_state.current_file_idx = self.diff_files.len() - 1; - self.file_list_state.select(self.diff_files.len() - 1); + Some(self.diff_files.len() - 1) + } + + fn reprioritize_highlights_if_file_changed(&mut self, prev: usize) { + if prev == self.diff_state.current_file_idx { + return; + } + self.manual_file_list_scroll = false; + if let Some(session) = self.highlight_session.as_ref() { + session.prioritize_around(self.diff_state.current_file_idx); } } @@ -7483,11 +7491,9 @@ impl App { self.saved_inline_selection = self.commit_selection_range; } - let highlighter = self.theme.syntax_highlighter(); let change_status = Self::get_change_status_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), )?; let has_staged_changes = change_status.staged; @@ -7556,22 +7562,17 @@ impl App { self.diff_source, DiffSource::CommitRange(_) | DiffSource::StagedUnstagedAndCommits(_) ) { - let highlighter = self.theme.syntax_highlighter(); match Self::get_working_tree_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), ) { - Ok(diff_files) => { - self.diff_files = diff_files; - self.diff_source = DiffSource::StagedAndUnstaged; - - // Update session for new files - for file in &self.diff_files { + Ok(diff) => { + for file in &diff.0 { self.session.add_diff_file(file); } - + self.install_diff(diff); + self.diff_source = DiffSource::StagedAndUnstaged; self.sort_files_by_directory(true); self.expand_all_dirs(); } @@ -7856,14 +7857,8 @@ impl App { use crate::forge::pr_open::prepare_open_pr; let local_checkout = Some(self.vcs_info.root_path.clone()); - let highlighter = self.theme.syntax_highlighter(); - let mut opened = prepare_open_pr( - details.clone(), - &patch, - commits, - local_checkout.as_deref(), - highlighter, - )?; + let mut opened = + prepare_open_pr(details.clone(), &patch, commits, local_checkout.as_deref())?; Self::load_or_apply_pr_session(&mut opened); let backend = create_forge_backend(&request.repository, local_checkout.clone()); self.enter_pr_diff_mode(backend, opened)?; @@ -8074,13 +8069,7 @@ impl App { summary.number, summary.number.to_string(), ); - let highlighter = self.theme.syntax_highlighter(); - let mut opened = open_pull_request( - backend.as_ref(), - target, - local_checkout.as_deref(), - highlighter, - )?; + let mut opened = open_pull_request(backend.as_ref(), target, local_checkout.as_deref())?; Self::load_or_apply_pr_session(&mut opened); // Sync thread + summary fetch — tests assert on // `app.forge_review_threads`/`forge_review_summaries` immediately @@ -8459,10 +8448,11 @@ impl App { }; // Collect selected entries in order from oldest to newest (end..start). + // Cloned eagerly so the immutable borrow of `self.commit_list` is + // released before any `&mut self` calls (install_diff, set_message). let selected_commits: Vec = (start..=end) .rev() - .filter_map(|i| self.commit_list.get(i)) - .cloned() + .filter_map(|i| self.commit_list.get(i).cloned()) .collect(); if selected_commits.is_empty() { @@ -8495,16 +8485,14 @@ impl App { } // Get the diff for the selected commits - let highlighter = self.theme.syntax_highlighter(); - let diff_files = Self::get_commit_range_diff_with_ignore( + let diff = Self::get_commit_range_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, &ResolvedRevisionRange::from_commit_ids(&selected_ids, RevisionDiffTarget::CommitList), - highlighter, self.path_filter.as_deref(), )?; - if diff_files.is_empty() { + if diff.0.is_empty() { self.set_message("No changes in selected commits"); return Ok(()); } @@ -8540,13 +8528,12 @@ impl App { self.session = session; // Add files to session - for file in &diff_files { + for file in &diff.0 { self.session.add_diff_file(file); } self.reset_persisted_session_tracking(); - // Update app state - self.diff_files = diff_files; + self.install_diff(diff); self.diff_source = DiffSource::CommitRange(selected_ids); self.input_mode = InputMode::Normal; @@ -8555,7 +8542,7 @@ impl App { self.file_list_state = FileListState::default(); // Set up inline commit selector for multi-commit reviews (newest-first display order) - self.review_commits = selected_commits.iter().rev().cloned().collect(); + self.review_commits = selected_commits.into_iter().rev().collect(); self.range_diff_files = Some(self.diff_files.clone()); self.commit_list = self.review_commits.clone(); self.commit_list_cursor = 0; @@ -8588,9 +8575,9 @@ impl App { // Check if all commits selected -> use cached range_diff_files if start == 0 && end == self.review_commits.len() - 1 - && let Some(ref files) = self.range_diff_files + && let Some(files) = self.range_diff_files.clone() { - self.diff_files = files.clone(); + self.install_diff((files, Vec::new())); let wrap = self.diff_state.wrap_lines; self.diff_state = DiffState::default(); self.diff_state.wrap_lines = wrap; @@ -8605,8 +8592,8 @@ impl App { } // Check cache for this subrange - if let Some(files) = self.commit_diff_cache.get(&(start, end)) { - self.diff_files = files.clone(); + if let Some(files) = self.commit_diff_cache.get(&(start, end)).cloned() { + self.install_diff((files, Vec::new())); let wrap = self.diff_state.wrap_lines; self.diff_state = DiffState::default(); self.diff_state.wrap_lines = wrap; @@ -8638,50 +8625,46 @@ impl App { .map(|c| c.id.clone()) .collect(); - let highlighter = self.theme.syntax_highlighter(); - let diff_files = if (has_staged || has_unstaged) && !selected_ids.is_empty() { + let empty: DiffWithJobs = (Vec::new(), Vec::new()); + let diff = if (has_staged || has_unstaged) && !selected_ids.is_empty() { match Self::get_working_tree_with_commits_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, &selected_ids, - highlighter, self.path_filter.as_deref(), ) { - Ok(files) => files, - Err(TuicrError::NoChanges) => Vec::new(), + Ok(d) => d, + Err(TuicrError::NoChanges) => empty, Err(e) => return Err(e), } } else if has_staged && has_unstaged { match Self::get_working_tree_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), ) { - Ok(files) => files, - Err(TuicrError::NoChanges) => Vec::new(), + Ok(d) => d, + Err(TuicrError::NoChanges) => empty, Err(e) => return Err(e), } } else if has_staged { match Self::get_staged_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), ) { - Ok(files) => files, - Err(TuicrError::NoChanges) => Vec::new(), + Ok(d) => d, + Err(TuicrError::NoChanges) => empty, Err(e) => return Err(e), } } else if has_unstaged { match Self::get_unstaged_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, - highlighter, self.path_filter.as_deref(), ) { - Ok(files) => files, - Err(TuicrError::NoChanges) => Vec::new(), + Ok(d) => d, + Err(TuicrError::NoChanges) => empty, Err(e) => return Err(e), } } else { @@ -8692,17 +8675,15 @@ impl App { &selected_ids, RevisionDiffTarget::CommitList, ), - highlighter, self.path_filter.as_deref(), ) { - Ok(files) => files, - Err(TuicrError::NoChanges) => Vec::new(), + Ok(d) => d, + Err(TuicrError::NoChanges) => empty, Err(e) => return Err(e), } }; - self.commit_diff_cache - .insert((start, end), diff_files.clone()); - self.diff_files = diff_files; + self.commit_diff_cache.insert((start, end), diff.0.clone()); + self.install_diff(diff); // Reset navigation, rebuild file tree + annotations let wrap = self.diff_state.wrap_lines; @@ -8724,15 +8705,13 @@ impl App { selected_ids: Vec, selected_commits: Vec, ) -> Result<()> { - let highlighter = self.theme.syntax_highlighter(); - let diff_files = match Self::get_working_tree_with_commits_diff_with_ignore( + let diff = match Self::get_working_tree_with_commits_diff_with_ignore( self.vcs.as_ref(), &self.vcs_info.root_path, &selected_ids, - highlighter, self.path_filter.as_deref(), ) { - Ok(diff_files) => diff_files, + Ok(d) => d, Err(TuicrError::NoChanges) => { self.set_message("No changes in selected commits + staged/unstaged"); return Ok(()); @@ -8743,12 +8722,12 @@ impl App { self.session = Self::load_or_create_staged_unstaged_and_commits_session(&self.vcs_info, &selected_ids); - for file in &diff_files { + for file in &diff.0 { self.session.add_diff_file(file); } self.reset_persisted_session_tracking(); - self.diff_files = diff_files; + self.install_diff(diff); self.diff_source = DiffSource::StagedUnstagedAndCommits(selected_ids); self.input_mode = InputMode::Normal; self.diff_state = DiffState::default(); @@ -10148,7 +10127,7 @@ mod commit_selection_tests { &self.info } - fn get_working_tree_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { Err(TuicrError::NoChanges) } @@ -10195,7 +10174,7 @@ mod commit_selection_tests { Theme::dark(), None, false, - Vec::new(), + (Vec::new(), Vec::new()), session, DiffSource::WorkingTree, InputMode::CommitSelect, @@ -10287,7 +10266,7 @@ mod target_selector_tests { &self.info } - fn get_working_tree_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { Err(TuicrError::NoChanges) } @@ -10356,7 +10335,7 @@ mod target_selector_tests { Theme::dark(), None, false, - Vec::new(), + (Vec::new(), Vec::new()), session, DiffSource::WorkingTree, InputMode::Normal, @@ -11756,7 +11735,7 @@ mod scroll_behavior_tests { &self.info } - fn get_working_tree_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { Err(TuicrError::NoChanges) } @@ -11837,7 +11816,7 @@ mod scroll_behavior_tests { Theme::dark(), None, false, - vec![file], + (vec![file], Vec::new()), session, DiffSource::WorkingTree, InputMode::Normal, @@ -12396,23 +12375,23 @@ mod change_status_tests { &self.info } - fn get_working_tree_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { Err(TuicrError::NoChanges) } - fn get_staged_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_staged_diff(&self) -> Result { if self.staged_files.is_empty() { Err(TuicrError::NoChanges) } else { - Ok(self.staged_files.clone()) + Ok((self.staged_files.clone(), Vec::new())) } } - fn get_unstaged_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_unstaged_diff(&self) -> Result { if self.unstaged_files.is_empty() { Err(TuicrError::NoChanges) } else { - Ok(self.unstaged_files.clone()) + Ok((self.unstaged_files.clone(), Vec::new())) } } @@ -12480,13 +12459,8 @@ mod change_status_tests { vcs.staged_files = vec![diff_file("ignored/generated.rs")]; vcs.unstaged_files = vec![diff_file("src/lib.rs")]; - let status = App::get_change_status_with_ignore( - &vcs, - dir.path(), - &SyntaxHighlighter::default(), - None, - ) - .expect("failed to get change status"); + let status = App::get_change_status_with_ignore(&vcs, dir.path(), None) + .expect("failed to get change status"); assert_eq!( status, @@ -12506,13 +12480,8 @@ mod change_status_tests { let dir = tempdir().expect("failed to create temp dir"); let vcs = mock_vcs(dir.path().to_path_buf()); - let status = App::get_change_status_with_ignore( - &vcs, - dir.path(), - &SyntaxHighlighter::default(), - None, - ) - .expect("failed to get change status"); + let status = App::get_change_status_with_ignore(&vcs, dir.path(), None) + .expect("failed to get change status"); assert_eq!( status, @@ -12541,20 +12510,17 @@ mod change_status_tests { fn info(&self) -> &VcsInfo { self.inner.info() } - fn get_working_tree_diff( - &self, - _highlighter: &SyntaxHighlighter, - ) -> Result> { + fn get_working_tree_diff(&self) -> Result { Err(TuicrError::UnsupportedOperation( "should not be called".into(), )) } - fn get_staged_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_staged_diff(&self) -> Result { Err(TuicrError::UnsupportedOperation( "should not be called".into(), )) } - fn get_unstaged_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_unstaged_diff(&self) -> Result { Err(TuicrError::UnsupportedOperation( "should not be called".into(), )) @@ -12600,13 +12566,8 @@ mod change_status_tests { unstaged_paths: vec![PathBuf::from("src/lib.rs")], }; - let status = App::get_change_status_with_ignore( - &vcs, - dir.path(), - &SyntaxHighlighter::default(), - None, - ) - .expect("failed to get change status"); + let status = App::get_change_status_with_ignore(&vcs, dir.path(), None) + .expect("failed to get change status"); assert_eq!( status, @@ -12637,7 +12598,7 @@ mod expand_gap_tests { &self.info } - fn get_working_tree_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { Err(TuicrError::NoChanges) } @@ -12716,7 +12677,7 @@ mod expand_gap_tests { Theme::dark(), None, false, - files, + (files, Vec::new()), session, DiffSource::WorkingTree, InputMode::Normal, @@ -13856,7 +13817,7 @@ mod submit_flow_tests { fn info(&self) -> &VcsInfo { &self.info } - fn get_working_tree_diff(&self, _h: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { Err(TuicrError::NoChanges) } fn fetch_context_lines( @@ -13953,7 +13914,7 @@ mod submit_flow_tests { Theme::dark(), None, false, - vec![diff_file], + (vec![diff_file], Vec::new()), session, DiffSource::PullRequest(Box::new(pr_source)), InputMode::Normal, @@ -14268,7 +14229,7 @@ mod submit_flow_tests { Theme::dark(), None, false, - Vec::new(), + (Vec::new(), Vec::new()), session, DiffSource::WorkingTree, InputMode::Normal, @@ -14940,11 +14901,8 @@ mod single_file_view_tests { fn info(&self) -> &VcsInfo { &self.0 } - fn get_working_tree_diff( - &self, - _hl: &crate::syntax::SyntaxHighlighter, - ) -> crate::error::Result> { - Ok(Vec::new()) + fn get_working_tree_diff(&self) -> crate::error::Result { + Ok((Vec::new(), Vec::new())) } fn fetch_context_lines( &self, @@ -15023,7 +14981,7 @@ mod single_file_view_tests { crate::theme::Theme::dark(), None, false, - files, + (files, Vec::new()), session, DiffSource::WorkingTree, InputMode::Normal, diff --git a/src/forge/pr_open.rs b/src/forge/pr_open.rs index 1a61f704..6523739d 100644 --- a/src/forge/pr_open.rs +++ b/src/forge/pr_open.rs @@ -19,15 +19,15 @@ use crate::forge::traits::{ ForgeBackend, PrSessionKey, PullRequestCommit, PullRequestDetails, PullRequestTarget, }; use crate::model::{DiffFile, ReviewSession, SessionDiffSource}; -use crate::syntax::SyntaxHighlighter; use crate::tuicrignore; +use crate::vcs::DiffWithJobs; use crate::vcs::diff_parser::{DiffFormat, parse_unified_diff}; /// Everything the App needs to enter PR review mode. #[derive(Debug)] pub struct OpenedPullRequest { pub details: PullRequestDetails, - pub diff_files: Vec, + pub diff: DiffWithJobs, pub session: ReviewSession, pub key: PrSessionKey, /// PR commits in newest-first display order. Empty when the forge @@ -45,10 +45,9 @@ pub fn open_pull_request( backend: &dyn ForgeBackend, target: PullRequestTarget, local_checkout: Option<&Path>, - highlighter: &SyntaxHighlighter, ) -> Result { let (details, patch, commits) = fetch_pr_data(backend, target)?; - prepare_open_pr(details, &patch, commits, local_checkout, highlighter) + prepare_open_pr(details, &patch, commits, local_checkout) } /// Network-only half of the PR open path: fetch PR metadata, the raw @@ -71,17 +70,16 @@ pub fn fetch_pr_data( } /// CPU-only half of the PR open path: parse the patch, apply -/// `.tuicrignore`, and build the session. Runs on the main thread because -/// `SyntaxHighlighter` is not trivially `Send`-cloneable. +/// `.tuicrignore`, and build the session. Highlighting is deferred to the +/// streaming worker via the returned `DiffWithJobs`. pub fn prepare_open_pr( details: PullRequestDetails, patch: &str, commits: Vec, local_checkout: Option<&Path>, - highlighter: &SyntaxHighlighter, ) -> Result { - let parsed = match parse_unified_diff(patch, DiffFormat::GitStyle, highlighter) { - Ok(files) => files, + let parsed = match parse_unified_diff(patch, DiffFormat::GitStyle) { + Ok(diff) => diff, Err(TuicrError::NoChanges) => { return Err(TuicrError::Forge(format!( "Pull request #{} has no file changes", @@ -91,13 +89,13 @@ pub fn prepare_open_pr( Err(e) => return Err(e), }; - let diff_files = match local_checkout { + let diff = match local_checkout { Some(root) => tuicrignore::filter_diff_files(root, parsed), None => parsed, }; let key = PrSessionKey::from_details(&details); - let session = build_session(&details, &key, &diff_files); + let session = build_session(&details, &key, &diff.0); // Forge returns commits oldest-first; the inline selector renders // newest-first so reverse here once. let mut commits = commits; @@ -105,7 +103,7 @@ pub fn prepare_open_pr( Ok(OpenedPullRequest { details, - diff_files, + diff, session, key, commits, @@ -252,11 +250,10 @@ index 1111111..2222222 100644 calls: RefCell::new(Vec::new()), }; let target = PullRequestTarget::with_repository(repo(), 125, "125"); - let highlighter = SyntaxHighlighter::default(); // when - let opened = open_pull_request(&backend, target, None, &highlighter).unwrap(); + let opened = open_pull_request(&backend, target, None).unwrap(); // then - assert_eq!(opened.diff_files.len(), 1); + assert_eq!(opened.diff.0.len(), 1); assert_eq!(opened.key.head_sha, "abcdef0123456789"); assert_eq!(opened.key.number, 125); assert_eq!(opened.session.diff_source, SessionDiffSource::PullRequest); @@ -319,13 +316,13 @@ rename to new_name.rs calls: RefCell::new(Vec::new()), }; let target = PullRequestTarget::with_repository(repo(), 125, "125"); - let highlighter = SyntaxHighlighter::default(); // when - let opened = open_pull_request(&backend, target, None, &highlighter).unwrap(); + let opened = open_pull_request(&backend, target, None).unwrap(); // then — all four files are recognized with correct statuses - assert_eq!(opened.diff_files.len(), 4); + assert_eq!(opened.diff.0.len(), 4); let statuses: Vec<(String, crate::model::FileStatus)> = opened - .diff_files + .diff + .0 .iter() .map(|f| (f.display_path().to_string_lossy().into_owned(), f.status)) .collect(); @@ -358,9 +355,8 @@ rename to new_name.rs calls: RefCell::new(Vec::new()), }; let target = PullRequestTarget::with_repository(repo(), 125, "125"); - let highlighter = SyntaxHighlighter::default(); // when - let err = open_pull_request(&backend, target, None, &highlighter).unwrap_err(); + let err = open_pull_request(&backend, target, None).unwrap_err(); // then let msg = err.to_string(); assert!( diff --git a/src/main.rs b/src/main.rs index 9997b44f..1471a124 100644 --- a/src/main.rs +++ b/src/main.rs @@ -29,6 +29,18 @@ use tuicr::{config, handler, profile, ui, update}; const CTRL_C_EXIT_TIMEOUT: Duration = Duration::from_secs(2); /// Hide the file list by default on narrow terminals. const MIN_WIDTH_FOR_FILE_LIST: u16 = 100; +/// Idle event-poll cadence: long enough to keep CPU usage trivial while no +/// input or background highlight results are pending. +const IDLE_POLL_TIMEOUT: Duration = Duration::from_millis(100); +/// Poll cadence while the highlight worker is still streaming results. +/// ~30 ms keeps perceived latency between worker emit and on-screen update +/// under one frame; CPU cost of the extra wakeups is negligible. +const STREAMING_POLL_TIMEOUT: Duration = Duration::from_millis(30); +/// Cap on events drained per tick. Holding j or wheel-scrolling fast queues +/// events faster than the per-event render can consume them; processing the +/// batch and rendering once kills perceived input lag. A cap keeps a paste +/// or stuck-key from monopolising the loop for too long before redraw. +const MAX_EVENTS_PER_TICK: usize = 64; fn main() -> anyhow::Result<()> { profile::init_from_env(); @@ -299,7 +311,7 @@ fn main() -> anyhow::Result<()> { let mut pending_ctrl_c: Option = None; // Main loop - loop { + 'main: loop { // Check for update result (non-blocking) if let Some(ref rx) = update_rx && let Ok( @@ -327,6 +339,10 @@ fn main() -> anyhow::Result<()> { app.poll_pr_submit_events(); app.poll_persisted_session_changes(); + // Drain any streaming highlight results before rendering so newly + // arrived spans land on screen this frame. + app.drain_highlight_updates(); + // Render. Bracket the frame in a synchronized-output pair // (CSI ?2026h/l) so terminals (and zellij) buffer the whole repaint // and present it atomically. Without this, scrolling over a slow link @@ -338,315 +354,343 @@ fn main() -> anyhow::Result<()> { })?; execute!(terminal.backend_mut(), EndSynchronizedUpdate)?; - // Handle events - if event::poll(Duration::from_millis(100))? { - let event = event::read()?; - // Down/Up Release flips the `*_released_since_arm` flag so the - // primed two-press file walk in single-file view requires a - // deliberate release + press; held-key auto-repeat (Repeat - // events) never satisfies the gate. Terminals without kitty - // REPORT_EVENT_TYPES support never emit Release, in which case - // `supports_keyboard_enhancement` is false and `cursor_down` / - // `cursor_up` skip the gate entirely. - if let Event::Key(key) = &event - && key.kind == KeyEventKind::Release - { - if matches!( - key.code, - crossterm::event::KeyCode::Down | crossterm::event::KeyCode::Char('j') - ) { - app.down_released_since_arm = true; - } - if matches!( - key.code, - crossterm::event::KeyCode::Up | crossterm::event::KeyCode::Char('k') - ) { - app.up_released_since_arm = true; - } - } - match event { - Event::Key(key) - if matches!(key.kind, KeyEventKind::Press | KeyEventKind::Repeat) => + // Shorter poll while highlight work is in flight keeps streaming + // latency under one frame; longer poll when idle to avoid wakeups. + let poll_timeout = if app.highlight_streaming() { + STREAMING_POLL_TIMEOUT + } else { + IDLE_POLL_TIMEOUT + }; + + // Drain queued events before redrawing; see MAX_EVENTS_PER_TICK. + if event::poll(poll_timeout)? { + for _ in 0..MAX_EVENTS_PER_TICK { + let event = event::read()?; + // Down/Up Release flips the `*_released_since_arm` flag so the + // primed two-press file walk in single-file view requires a + // deliberate release + press; held-key auto-repeat (Repeat + // events) never satisfies the gate. Terminals without kitty + // REPORT_EVENT_TYPES support never emit Release, in which case + // `supports_keyboard_enhancement` is false and `cursor_down` / + // `cursor_up` skip the gate entirely. + if let Event::Key(key) = &event + && key.kind == KeyEventKind::Release { - // Handle Ctrl+C twice to exit (works across all input modes) - // In Comment mode, first Ctrl+C also cancels the comment - if key.code == crossterm::event::KeyCode::Char('c') - && key - .modifiers - .contains(crossterm::event::KeyModifiers::CONTROL) + if matches!( + key.code, + crossterm::event::KeyCode::Down | crossterm::event::KeyCode::Char('j') + ) { + app.down_released_since_arm = true; + } + if matches!( + key.code, + crossterm::event::KeyCode::Up | crossterm::event::KeyCode::Char('k') + ) { + app.up_released_since_arm = true; + } + } + match event { + Event::Key(key) + if matches!(key.kind, KeyEventKind::Press | KeyEventKind::Repeat) => { - // If in comment mode, cancel the comment first - if app.input_mode == InputMode::Comment { - app.exit_comment_mode(); - } - - if let Some(first_press) = pending_ctrl_c - && first_press.elapsed() < CTRL_C_EXIT_TIMEOUT + // Handle Ctrl+C twice to exit (works across all input modes) + // In Comment mode, first Ctrl+C also cancels the comment + if key.code == crossterm::event::KeyCode::Char('c') + && key + .modifiers + .contains(crossterm::event::KeyModifiers::CONTROL) { - // Second Ctrl+C within timeout - exit immediately - app.should_quit = true; - continue; + // If in comment mode, cancel the comment first + if app.input_mode == InputMode::Comment { + app.exit_comment_mode(); + } + + if let Some(first_press) = pending_ctrl_c + && first_press.elapsed() < CTRL_C_EXIT_TIMEOUT + { + // Second Ctrl+C within timeout - exit immediately + app.should_quit = true; + continue 'main; + } + // First Ctrl+C (or timeout expired) - show warning and start timer + pending_ctrl_c = Some(Instant::now()); + app.set_message("Press Ctrl+C again to exit"); + continue 'main; } - // First Ctrl+C (or timeout expired) - show warning and start timer - pending_ctrl_c = Some(Instant::now()); - app.set_message("Press Ctrl+C again to exit"); - continue; - } - // Any other key clears the pending Ctrl+C state and message - if pending_ctrl_c.is_some() { - pending_ctrl_c = None; - app.message = None; - } + // Any other key clears the pending Ctrl+C state and message + if pending_ctrl_c.is_some() { + pending_ctrl_c = None; + app.message = None; + } - // Handle pending z command for zz/zt/zb viewport positioning - if pending_z { - pending_z = false; - match key.code { - crossterm::event::KeyCode::Char('z') => { - app.center_cursor(); - continue; - } - crossterm::event::KeyCode::Char('t') => { - app.cursor_to_top(); - continue; - } - crossterm::event::KeyCode::Char('b') => { - app.cursor_to_bottom(); - continue; + // Handle pending z command for zz/zt/zb viewport positioning + if pending_z { + pending_z = false; + match key.code { + crossterm::event::KeyCode::Char('z') => { + app.center_cursor(); + continue 'main; + } + crossterm::event::KeyCode::Char('t') => { + app.cursor_to_top(); + continue 'main; + } + crossterm::event::KeyCode::Char('b') => { + app.cursor_to_bottom(); + continue 'main; + } + _ => {} // Fall through to normal handling } - _ => {} // Fall through to normal handling } - } - // Handle pending Z command for ZZ (export+quit) / ZQ (quit) - if pending_shift_z { - pending_shift_z = false; - match key.code { - crossterm::event::KeyCode::Char('Z') => { - // ZZ: save session, export, and quit (same as :wq) - let _ = app.save_current_session_merging_external(); - if app.session.has_comments() { - handler::handle_export_and_quit(&mut app); - } else { + // Handle pending Z command for ZZ (export+quit) / ZQ (quit) + if pending_shift_z { + pending_shift_z = false; + match key.code { + crossterm::event::KeyCode::Char('Z') => { + // ZZ: save session, export, and quit (same as :wq) + let _ = app.save_current_session_merging_external(); + if app.session.has_comments() { + handler::handle_export_and_quit(&mut app); + } else { + app.should_quit = true; + } + continue 'main; + } + crossterm::event::KeyCode::Char('Q') => { + // ZQ: quit without exporting (same as q) app.should_quit = true; + continue 'main; } - continue; + _ => {} // Fall through to normal handling } - crossterm::event::KeyCode::Char('Q') => { - // ZQ: quit without exporting (same as q) - app.should_quit = true; - continue; - } - _ => {} // Fall through to normal handling } - } - // Handle pending d command for dd delete comment - if pending_d { - pending_d = false; - if key.code == crossterm::event::KeyCode::Char('d') { - if app.cursor_on_locked_comment() { - app.set_message( - "Comment already pushed to GitHub — read only in tuicr", - ); - } else if !app.delete_comment_at_cursor() { - if app.cursor_on_remote_thread() { - app.set_message("GitHub comment — read only in tuicr"); - } else { - app.set_message("No comment at cursor"); + // Handle pending d command for dd delete comment + if pending_d { + pending_d = false; + if key.code == crossterm::event::KeyCode::Char('d') { + if app.cursor_on_locked_comment() { + app.set_message( + "Comment already pushed to GitHub — read only in tuicr", + ); + } else if !app.delete_comment_at_cursor() { + if app.cursor_on_remote_thread() { + app.set_message("GitHub comment — read only in tuicr"); + } else { + app.set_message("No comment at cursor"); + } } + continue 'main; } - continue; + // Otherwise fall through to normal handling } - // Otherwise fall through to normal handling - } - // Handle pending leader command for panel focus, file list toggle, and review comments. - if pending_leader { - pending_leader = false; - match key.code { - crossterm::event::KeyCode::Char('e') => { - app.toggle_file_list(); - continue; - } - crossterm::event::KeyCode::Char('h') => { - if app.show_file_list { - app.focused_panel = app::FocusedPanel::FileList; + // Handle pending leader command for panel focus, file list toggle, and review comments. + if pending_leader { + pending_leader = false; + match key.code { + crossterm::event::KeyCode::Char('e') => { + app.toggle_file_list(); + continue 'main; } - continue; - } - crossterm::event::KeyCode::Char('l') => { - app.focused_panel = app::FocusedPanel::Diff; - continue; - } - crossterm::event::KeyCode::Char('k') => { - if app.focused_panel == app::FocusedPanel::Comments { - app.focused_panel = app::FocusedPanel::FileList; - } else if app.has_inline_commit_selector() { - app.focused_panel = app::FocusedPanel::CommitSelector; + crossterm::event::KeyCode::Char('h') => { + if app.show_file_list { + app.focused_panel = app::FocusedPanel::FileList; + } + continue 'main; } - continue; - } - crossterm::event::KeyCode::Char('j') => { - if app.focused_panel == app::FocusedPanel::FileList - && app.has_comment_navigator_items() - { - app.focused_panel = app::FocusedPanel::Comments; - } else { + crossterm::event::KeyCode::Char('l') => { app.focused_panel = app::FocusedPanel::Diff; + continue 'main; } - continue; - } - crossterm::event::KeyCode::Char('c') => { - app.enter_review_comment_mode(); - continue; - } - crossterm::event::KeyCode::Char('f') => { - app.toggle_single_file_view(); - continue; + crossterm::event::KeyCode::Char('k') => { + if app.focused_panel == app::FocusedPanel::Comments { + app.focused_panel = app::FocusedPanel::FileList; + } else if app.has_inline_commit_selector() { + app.focused_panel = app::FocusedPanel::CommitSelector; + } + continue 'main; + } + crossterm::event::KeyCode::Char('j') => { + if app.focused_panel == app::FocusedPanel::FileList + && app.has_comment_navigator_items() + { + app.focused_panel = app::FocusedPanel::Comments; + } else { + app.focused_panel = app::FocusedPanel::Diff; + } + continue 'main; + } + crossterm::event::KeyCode::Char('c') => { + app.enter_review_comment_mode(); + continue 'main; + } + crossterm::event::KeyCode::Char('f') => { + app.toggle_single_file_view(); + continue 'main; + } + _ => {} } - _ => {} + // Otherwise fall through to normal handling } - // Otherwise fall through to normal handling - } - // Editing the PR-tab filter is a sub-state of CommitSelect; - // route through the filter-specific key map so typed - // characters update the filter buffer rather than driving - // commit-list navigation. - let mut action = - if app.input_mode == InputMode::CommitSelect && app.pr_filter_editing() { + // Editing the PR-tab filter is a sub-state of CommitSelect; + // route through the filter-specific key map so typed + // characters update the filter buffer rather than driving + // commit-list navigation. + let mut action = if app.input_mode == InputMode::CommitSelect + && app.pr_filter_editing() + { map_target_filter_mode(key) } else { map_key_to_action(key, app.input_mode, app.leader_key) }; - // Handle pending command setters (these work in any mode) - match action { - Action::PendingZCommand => { - pending_z = true; - app.pending_count = None; - continue; - } - Action::PendingShiftZCommand => { - pending_shift_z = true; - app.pending_count = None; - continue; - } - Action::PendingDCommand => { - pending_d = true; - app.pending_count = None; - continue; - } - Action::PendingLeaderCommand => { - pending_leader = true; - app.pending_count = None; - continue; - } - _ => {} - } - - // Vim-style {count}{motion} (Normal mode only): digits accumulate - // into `pending_count`, then a following motion either scales its - // inner count parameter or is dispatched repeatedly. `{count}G` - // jumps to source line `count` (already existing behaviour). - if app.input_mode == InputMode::Normal { + // Handle pending command setters (these work in any mode) match action { - Action::Digit(d) => { - let n = app.pending_count.unwrap_or(0); - app.pending_count = Some( - (n.saturating_mul(10).saturating_add(d as usize)).min(999_999), - ); - continue; + Action::PendingZCommand => { + pending_z = true; + app.pending_count = None; + continue 'main; } - Action::GoToBottom if app.pending_count.is_some() => { - let count = app.pending_count.unwrap().max(1); + Action::PendingShiftZCommand => { + pending_shift_z = true; app.pending_count = None; - app.go_to_source_line(count as u32, tuicr::model::LineSide::New); - continue; + continue 'main; } - _ => { - if let Some(count) = app.pending_count.take() { - let count = count.max(1); - match &mut action { - Action::CursorDown(n) - | Action::CursorUp(n) - | Action::ScrollLeft(n) - | Action::ScrollRight(n) - | Action::ScrollViewDown(n) - | Action::ScrollViewUp(n) => { - *n = n.saturating_mul(count); - } - Action::NextFile - | Action::PrevFile - | Action::NextHunk - | Action::PrevHunk => { - // Dispatch `count - 1` extra times; the - // last one runs through normal dispatch - // below. - for _ in 1..count { - dispatch_action(&mut app, action.clone()); + Action::PendingDCommand => { + pending_d = true; + app.pending_count = None; + continue 'main; + } + Action::PendingLeaderCommand => { + pending_leader = true; + app.pending_count = None; + continue 'main; + } + _ => {} + } + + // Vim-style {count}{motion} (Normal mode only): digits accumulate + // into `pending_count`, then a following motion either scales its + // inner count parameter or is dispatched repeatedly. `{count}G` + // jumps to source line `count` (already existing behaviour). + if app.input_mode == InputMode::Normal { + match action { + Action::Digit(d) => { + let n = app.pending_count.unwrap_or(0); + app.pending_count = Some( + (n.saturating_mul(10).saturating_add(d as usize)) + .min(999_999), + ); + continue 'main; + } + Action::GoToBottom if app.pending_count.is_some() => { + let count = app.pending_count.unwrap().max(1); + app.pending_count = None; + app.go_to_source_line( + count as u32, + tuicr::model::LineSide::New, + ); + continue 'main; + } + _ => { + if let Some(count) = app.pending_count.take() { + let count = count.max(1); + match &mut action { + Action::CursorDown(n) + | Action::CursorUp(n) + | Action::ScrollLeft(n) + | Action::ScrollRight(n) + | Action::ScrollViewDown(n) + | Action::ScrollViewUp(n) => { + *n = n.saturating_mul(count); + } + Action::NextFile + | Action::PrevFile + | Action::NextHunk + | Action::PrevHunk => { + // Dispatch `count - 1` extra times; the + // last one runs through normal dispatch + // below. + for _ in 1..count { + dispatch_action(&mut app, action.clone()); + } + } + _ => { + // Count silently discarded for non-motion + // actions (mode changes, edits, etc.). } - } - _ => { - // Count silently discarded for non-motion - // actions (mode changes, edits, etc.). } } } } } - } - dispatch_action(&mut app, action); - if let Some(target) = app.take_pending_editor_target() { - match run_editor_from_tui(&mut terminal, &target) { - Ok(Ok(())) => { - if app.diff_source.includes_worktree_changes() { - match app.reload_diff_files() { - Ok((count, invalidated)) => { - let invalidated_suffix = if invalidated > 0 { - format!(", {invalidated} changed since last review") - } else { - String::new() - }; - app.set_message(format!( - "Opened {} and reloaded {count} files{invalidated_suffix}", - target.path.display() - )); - } - Err(err) => { - app.set_error(format!( - "Reload after editor failed: {err}" - )); + dispatch_action(&mut app, action); + if let Some(target) = app.take_pending_editor_target() { + match run_editor_from_tui(&mut terminal, &target) { + Ok(Ok(())) => { + if app.diff_source.includes_worktree_changes() { + match app.reload_diff_files() { + Ok((count, invalidated)) => { + let invalidated_suffix = if invalidated > 0 { + format!( + ", {invalidated} changed since last review" + ) + } else { + String::new() + }; + app.set_message(format!( + "Opened {} and reloaded {count} files{invalidated_suffix}", + target.path.display() + )); + } + Err(err) => { + app.set_error(format!( + "Reload after editor failed: {err}" + )); + } } + } else { + app.set_message(format!( + "Opened {}", + target.path.display() + )); } - } else { - app.set_message(format!("Opened {}", target.path.display())); + } + Ok(Err(err)) => app.set_error(err.to_string()), + Err(err) => { + app.set_error(format!("Failed to restore terminal: {err}")) } } - Ok(Err(err)) => app.set_error(err.to_string()), - Err(err) => app.set_error(format!("Failed to restore terminal: {err}")), + // Editor took over the terminal; force a redraw + // before reading any further queued events. + continue 'main; } } - } - Event::Mouse(mouse_event) => handle_mouse_event(&mut app, mouse_event), - Event::Paste(text) => { - // Bracketed-paste payload — route to whichever handler is - // currently accepting text input. Other modes ignore. - let action = Action::Paste(text); - match app.input_mode { - InputMode::Comment => handle_comment_action(&mut app, action), - InputMode::Command => handle_command_action(&mut app, action), - InputMode::Search => handle_search_action(&mut app, action), - InputMode::CommitSelect if app.pr_filter_editing() => { - handle_commit_select_action(&mut app, action) + Event::Mouse(mouse_event) => handle_mouse_event(&mut app, mouse_event), + Event::Paste(text) => { + // Bracketed-paste payload — route to whichever handler is + // currently accepting text input. Other modes ignore. + let action = Action::Paste(text); + match app.input_mode { + InputMode::Comment => handle_comment_action(&mut app, action), + InputMode::Command => handle_command_action(&mut app, action), + InputMode::Search => handle_search_action(&mut app, action), + InputMode::CommitSelect if app.pr_filter_editing() => { + handle_commit_select_action(&mut app, action) + } + _ => {} } - _ => {} } + _ => {} + } + if app.should_quit || !event::poll(Duration::ZERO)? { + break; } - _ => {} } } diff --git a/src/syntax/mod.rs b/src/syntax/mod.rs index b2a0e55b..48cda13e 100644 --- a/src/syntax/mod.rs +++ b/src/syntax/mod.rs @@ -4,6 +4,9 @@ use two_face::theme::EmbeddedThemeName; use crate::model::diff_types::LineOrigin; +pub(crate) mod streaming; +pub(crate) use streaming::{HighlightJob, HighlightJobKind}; + /// A single line of highlighted spans (style + text pairs). pub(crate) type HighlightedSpans = Vec<(Style, String)>; diff --git a/src/syntax/streaming.rs b/src/syntax/streaming.rs new file mode 100644 index 00000000..042ef977 --- /dev/null +++ b/src/syntax/streaming.rs @@ -0,0 +1,327 @@ +//! Streaming syntax-highlight pipeline. +//! +//! The diff is parsed (and rendered) without highlighting; a background +//! worker thread then produces highlight results and streams them to the +//! main thread via an mpsc channel. The main loop drains the channel each +//! iteration and patches the model in place, then redraws. +//! +//! Why this shape: VCS access (git2, jj/hg subprocess) is often `!Send`, +//! so the parse phase has to stay on the main thread; syntect's +//! `SyntaxSet` / `Theme` are `Sync`, so the highlight phase parallelises +//! cleanly across threads as long as inputs are owned strings. + +use std::collections::{HashMap, VecDeque}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::mpsc::{self, Sender}; +use std::sync::{Arc, Mutex}; +use std::thread; + +use crate::model::diff_types::LineOrigin; +use crate::syntax::{HighlightedLines, HighlightedSpans, SyntaxHighlighter}; + +/// Same cost ceiling as the previous synchronous full-file pass: skip +/// highlighting for files over this size to keep runaway diffs cheap. +const MAX_HIGHLIGHT_FILE_BYTES: usize = 1024 * 1024; + +/// A single unit of highlight work, produced during the (serial) parse phase +/// and consumed on the background worker. +#[derive(Debug)] +pub struct HighlightJob { + pub file_idx: usize, + pub syntax_path: PathBuf, + pub kind: HighlightJobKind, +} + +#[derive(Debug)] +pub enum HighlightJobKind { + /// Per-hunk highlight using only the lines inside the hunk. + Hunk { + hunk_idx: usize, + old_lines: Vec, + new_lines: Vec, + old_line_indices: Vec>, + new_line_indices: Vec>, + line_origins: Vec, + }, + /// Container-grammar (Vue, Svelte, ...) full-file context. Content is + /// fetched during parse so the worker never touches VCS state. + FullFile { + old_content: Option, + new_content: Option, + }, +} + +/// A highlight result the worker streams back. Each variant patches one +/// file in place. +pub(crate) struct HighlightUpdate { + pub file_idx: usize, + pub kind: HighlightUpdateKind, +} + +pub(crate) enum HighlightUpdateKind { + Hunk { + hunk_idx: usize, + line_spans: Vec>, + }, + FullFile { + old: Option, + new: Option, + }, +} + +/// Shared queue of pending jobs. Workers pop from the front under the +/// mutex; the App can reorder the queue by file-index proximity when the +/// user navigates, so visible files get highlighted first. +pub(crate) type SharedQueue = Arc>>; + +/// Spawn the highlight worker. Returns the join handle; the caller usually +/// doesn't need to wait on it (the worker exits when its send fails on a +/// dropped receiver, or when `cancel` is set). +pub(crate) fn spawn_highlight_worker( + queue: SharedQueue, + highlighter: Arc, + cancel: Arc, + tx: Sender, +) -> thread::JoinHandle<()> { + thread::spawn(move || run_worker(queue, highlighter, cancel, tx)) +} + +/// Run the highlight pipeline synchronously on the current thread. Used by +/// tests that want spans populated deterministically without a worker thread. +#[cfg(test)] +pub(crate) fn run_blocking( + jobs: Vec, + highlighter: &SyntaxHighlighter, +) -> Vec { + jobs.iter() + .filter_map(|job| run_job(job, highlighter)) + .collect() +} + +fn run_worker( + queue: SharedQueue, + highlighter: Arc, + cancel: Arc, + tx: Sender, +) { + let initial_len = queue.lock().unwrap().len(); + if initial_len == 0 { + return; + } + + let parallelism = thread::available_parallelism() + .map(|n| n.get()) + .unwrap_or(1) + .min(initial_len) + .max(1); + + thread::scope(|s| { + for _ in 0..parallelism { + let queue = Arc::clone(&queue); + let cancel = Arc::clone(&cancel); + let tx = tx.clone(); + let highlighter = Arc::clone(&highlighter); + s.spawn(move || { + loop { + if cancel.load(Ordering::Relaxed) { + return; + } + let job = queue.lock().unwrap().pop_front(); + let Some(job) = job else { + return; + }; + if let Some(update) = run_job(&job, &highlighter) + && tx.send(update).is_err() + { + return; + } + } + }); + } + }); +} + +fn run_job(job: &HighlightJob, h: &SyntaxHighlighter) -> Option { + match &job.kind { + HighlightJobKind::Hunk { + hunk_idx, + old_lines, + new_lines, + old_line_indices, + new_line_indices, + line_origins, + } => { + let old_highlighted = h.highlight_file_lines(&job.syntax_path, old_lines); + let new_highlighted = h.highlight_file_lines(&job.syntax_path, new_lines); + if old_highlighted.is_none() && new_highlighted.is_none() { + return None; + } + let line_spans: Vec> = (0..line_origins.len()) + .map(|i| { + h.highlighted_line_for_diff_with_background( + old_highlighted.as_deref(), + new_highlighted.as_deref(), + old_line_indices[i], + new_line_indices[i], + line_origins[i], + ) + }) + .collect(); + Some(HighlightUpdate { + file_idx: job.file_idx, + kind: HighlightUpdateKind::Hunk { + hunk_idx: *hunk_idx, + line_spans, + }, + }) + } + HighlightJobKind::FullFile { + old_content, + new_content, + } => { + let old = highlight_full(h, &job.syntax_path, old_content.as_deref()); + let new = highlight_full(h, &job.syntax_path, new_content.as_deref()); + if old.is_none() && new.is_none() { + return None; + } + Some(HighlightUpdate { + file_idx: job.file_idx, + kind: HighlightUpdateKind::FullFile { old, new }, + }) + } + } +} + +fn highlight_full( + h: &SyntaxHighlighter, + path: &std::path::Path, + content: Option<&str>, +) -> Option { + let content = content?; + if content.len() > MAX_HIGHLIGHT_FILE_BYTES || content.as_bytes().contains(&0u8) { + return None; + } + let lines: Vec = content.lines().map(crate::vcs::tabify).collect(); + h.highlight_file_lines(path, &lines) +} + +/// A running highlight session: receiver for streamed results, the cancel +/// token shared with the worker, and the shared job queue (so the App can +/// reprioritize pending work when the user navigates). +pub(crate) struct HighlightSession { + rx: mpsc::Receiver, + cancel: Arc, + queue: SharedQueue, +} + +impl HighlightSession { + /// Start a new session. Returns `None` if there is nothing to highlight, + /// in which case no worker is spawned. + pub(crate) fn start( + jobs: Vec, + highlighter: Arc, + ) -> Option { + if jobs.is_empty() { + return None; + } + let queue: SharedQueue = Arc::new(Mutex::new(VecDeque::from(jobs))); + let cancel = Arc::new(AtomicBool::new(false)); + let (tx, rx) = mpsc::channel(); + spawn_highlight_worker(Arc::clone(&queue), highlighter, Arc::clone(&cancel), tx); + Some(Self { rx, cancel, queue }) + } + + /// Re-resolve each job's `file_idx` by `syntax_path` against + /// `diff_files.display_path()`, drop jobs whose path is no longer present, + /// then start the session. Used after the parse-time `file_idx` may have + /// drifted from its file's position (commit-message insert, directory + /// sort, `.tuicrignore` re-filter). `HighlightJob::syntax_path` and + /// `DiffFile::display_path` are both `new_path or old_path`, so the keys + /// line up. + pub(crate) fn start_resolved( + mut jobs: Vec, + diff_files: &[crate::model::DiffFile], + highlighter: Arc, + ) -> Option { + let path_to_idx: HashMap<&Path, usize> = diff_files + .iter() + .enumerate() + .map(|(i, f)| (f.display_path().as_path(), i)) + .collect(); + jobs.retain_mut(|job| match path_to_idx.get(job.syntax_path.as_path()) { + Some(&idx) => { + job.file_idx = idx; + true + } + None => false, + }); + Self::start(jobs, highlighter) + } + + /// Signal the worker to stop. The receiver is also dropped, so any + /// in-flight result the worker tries to send will fail and the worker + /// exits at its next channel send. + pub(crate) fn cancel(self) { + self.cancel.store(true, Ordering::Relaxed); + // drop self → rx dropped + } + + pub(crate) fn try_recv(&self) -> Result { + self.rx.try_recv() + } + + /// Reorder the pending queue so jobs nearest `file_idx` run next. + /// Called when the user navigates to a new file, so the visible + /// viewport gets highlighted before far-away files. + pub(crate) fn prioritize_around(&self, file_idx: usize) { + let mut q = self.queue.lock().unwrap(); + q.make_contiguous() + .sort_by_key(|job| job.file_idx.abs_diff(file_idx)); + } +} + +/// Patch a streamed highlight result into the in-memory diff. Idempotent: +/// applying the same update twice produces the same final state. +pub(crate) fn apply_update( + files: &mut [crate::model::DiffFile], + highlighter: &SyntaxHighlighter, + update: HighlightUpdate, +) { + let Some(file) = files.get_mut(update.file_idx) else { + return; + }; + match update.kind { + HighlightUpdateKind::Hunk { + hunk_idx, + line_spans, + } => { + let Some(hunk) = file.hunks.get_mut(hunk_idx) else { + return; + }; + for (line, spans) in hunk.lines.iter_mut().zip(line_spans) { + if let Some(spans) = spans { + line.highlighted_spans = Some(spans); + } + } + } + HighlightUpdateKind::FullFile { old, new } => { + for hunk in &mut file.hunks { + for line in &mut hunk.lines { + let old_idx = line.old_lineno.map(|n| n.saturating_sub(1) as usize); + let new_idx = line.new_lineno.map(|n| n.saturating_sub(1) as usize); + let spans = highlighter.highlighted_line_for_diff_with_background( + old.as_deref(), + new.as_deref(), + old_idx, + new_idx, + line.origin, + ); + if spans.is_some() { + line.highlighted_spans = spans; + } + } + } + } + } +} diff --git a/src/theme/mod.rs b/src/theme/mod.rs index 0e0b277a..25ec3fb8 100644 --- a/src/theme/mod.rs +++ b/src/theme/mod.rs @@ -25,7 +25,7 @@ pub enum SyntaxThemeSource { /// Complete color theme for the application pub struct Theme { /// Cached syntax highlighter (lazily initialized) - highlighter: OnceLock, + highlighter: OnceLock>, // Base colors pub panel_bg: Color, @@ -2274,15 +2274,23 @@ pub fn resolve_theme_with_config( impl Theme { /// Get the syntax highlighter for this theme (lazily initialized, cached) pub fn syntax_highlighter(&self) -> &SyntaxHighlighter { - self.highlighter.get_or_init(|| match &self.syntax_theme { - SyntaxThemeSource::Embedded(theme) => { - SyntaxHighlighter::new(*theme, self.syntax_add_bg, self.syntax_del_bg) - } - SyntaxThemeSource::Custom(theme) => SyntaxHighlighter::with_theme( - *theme.clone(), - self.syntax_add_bg, - self.syntax_del_bg, - ), + self.syntax_highlighter_arc().as_ref() + } + + /// Same as `syntax_highlighter`, but yields an `Arc` clone that can be + /// moved into the highlight worker thread. + pub fn syntax_highlighter_arc(&self) -> &std::sync::Arc { + self.highlighter.get_or_init(|| { + std::sync::Arc::new(match &self.syntax_theme { + SyntaxThemeSource::Embedded(theme) => { + SyntaxHighlighter::new(*theme, self.syntax_add_bg, self.syntax_del_bg) + } + SyntaxThemeSource::Custom(theme) => SyntaxHighlighter::with_theme( + *theme.clone(), + self.syntax_add_bg, + self.syntax_del_bg, + ), + }) }) } diff --git a/src/tuicrignore.rs b/src/tuicrignore.rs index 8a87ce7e..46c00815 100644 --- a/src/tuicrignore.rs +++ b/src/tuicrignore.rs @@ -2,22 +2,19 @@ use std::path::Path; use ignore::gitignore::GitignoreBuilder; -use crate::model::DiffFile; +use crate::vcs::DiffWithJobs; -/// Apply `.tuicrignore` rules from the repository root to a diff file set. -pub fn filter_diff_files(repo_root: &Path, diff_files: Vec) -> Vec { +/// Apply `.tuicrignore` rules from the repository root to a diff file set, +/// re-indexing the accompanying highlight jobs. +pub fn filter_diff_files(repo_root: &Path, diff: DiffWithJobs) -> DiffWithJobs { let Some(matcher) = load_matcher(repo_root) else { - return diff_files; + return diff; }; - - diff_files - .into_iter() - .filter(|file| { - !matcher - .matched_path_or_any_parents(file.display_path(), false) - .is_ignore() - }) - .collect() + crate::vcs::filter_diff_with_jobs(diff, |file| { + !matcher + .matched_path_or_any_parents(file.display_path(), false) + .is_ignore() + }) } /// Apply `.tuicrignore` (and `.gitignore`, for `!`-unignore patterns) rules to a @@ -68,7 +65,7 @@ mod tests { use tempfile::tempdir; use super::*; - use crate::model::FileStatus; + use crate::model::{DiffFile, FileStatus}; fn make_diff_file(path: &str) -> DiffFile { DiffFile { @@ -83,6 +80,10 @@ mod tests { } } + fn filter_just_files(repo_root: &Path, files: Vec) -> Vec { + filter_diff_files(repo_root, (files, Vec::new())).0 + } + #[test] fn keeps_all_files_when_tuicrignore_is_missing() { let dir = tempdir().expect("failed to create temp dir"); @@ -91,7 +92,7 @@ mod tests { make_diff_file("target/debug/app"), ]; - let filtered = filter_diff_files(dir.path(), files); + let filtered = filter_just_files(dir.path(), files); assert_eq!(filtered.len(), 2); } @@ -117,7 +118,7 @@ mod tests { make_diff_file("Cargo.lock"), ]; - let filtered = filter_diff_files(dir.path(), files); + let filtered = filter_just_files(dir.path(), files); let kept_paths: Vec = filtered .iter() .map(|f| f.display_path().display().to_string()) @@ -139,7 +140,7 @@ mod tests { make_diff_file("src/main.rs"), ]; - let filtered = filter_diff_files(dir.path(), files); + let filtered = filter_just_files(dir.path(), files); let kept_paths: Vec = filtered .iter() .map(|f| f.display_path().display().to_string()) @@ -160,7 +161,7 @@ mod tests { make_diff_file("build.log"), ]; - let filtered = filter_diff_files(dir.path(), files); + let filtered = filter_just_files(dir.path(), files); let kept: Vec = filtered .iter() .map(|f| f.display_path().display().to_string()) @@ -183,7 +184,7 @@ mod tests { make_diff_file("src/lib.rs"), ]; - let filtered = filter_diff_files(dir.path(), files); + let filtered = filter_just_files(dir.path(), files); let kept: Vec = filtered .iter() .map(|f| f.display_path().display().to_string()) @@ -204,7 +205,7 @@ mod tests { make_diff_file("dist/bundle.js"), ]; - let filtered = filter_diff_files(dir.path(), files); + let filtered = filter_just_files(dir.path(), files); let kept: Vec = filtered .iter() .map(|f| f.display_path().display().to_string()) @@ -231,7 +232,7 @@ mod tests { }; let kept = make_diff_file("src/lib.rs"); - let filtered = filter_diff_files(dir.path(), vec![deleted, kept]); + let filtered = filter_just_files(dir.path(), vec![deleted, kept]); let kept_paths: Vec = filtered .iter() .map(|f| f.display_path().display().to_string()) diff --git a/src/ui/diff_side_by_side.rs b/src/ui/diff_side_by_side.rs index b5572936..df21fd24 100644 --- a/src/ui/diff_side_by_side.rs +++ b/src/ui/diff_side_by_side.rs @@ -1539,10 +1539,9 @@ mod remote_comments_side_by_side_snapshot_tests { use crate::model::{ DiffFile, DiffHunk, DiffLine, FileStatus, LineOrigin, ReviewSession, SessionDiffSource, }; - use crate::syntax::SyntaxHighlighter; use crate::theme::Theme; use crate::ui::render; - use crate::vcs::traits::{VcsBackend, VcsChangeStatus, VcsInfo, VcsType}; + use crate::vcs::traits::{DiffWithJobs, VcsBackend, VcsChangeStatus, VcsInfo, VcsType}; use ratatui::Terminal; use ratatui::backend::TestBackend; use ratatui::buffer::Buffer; @@ -1556,10 +1555,7 @@ mod remote_comments_side_by_side_snapshot_tests { fn info(&self) -> &VcsInfo { &self.info } - fn get_working_tree_diff( - &self, - _highlighter: &SyntaxHighlighter, - ) -> TuicrResult> { + fn get_working_tree_diff(&self) -> TuicrResult { Err(TuicrError::NoChanges) } fn fetch_context_lines( @@ -1683,7 +1679,7 @@ mod remote_comments_side_by_side_snapshot_tests { Theme::dark(), None, false, - vec![sample_diff_file()], + (vec![sample_diff_file()], Vec::new()), session, DiffSource::PullRequest(Box::new(pr)), InputMode::Normal, diff --git a/src/ui/diff_unified.rs b/src/ui/diff_unified.rs index bccb7175..2e6fc30c 100644 --- a/src/ui/diff_unified.rs +++ b/src/ui/diff_unified.rs @@ -1335,10 +1335,9 @@ mod remote_comments_snapshot_tests { use crate::model::{ DiffFile, DiffHunk, DiffLine, FileStatus, LineOrigin, ReviewSession, SessionDiffSource, }; - use crate::syntax::SyntaxHighlighter; use crate::theme::Theme; use crate::ui::render; - use crate::vcs::traits::{VcsBackend, VcsChangeStatus, VcsInfo, VcsType}; + use crate::vcs::traits::{DiffWithJobs, VcsBackend, VcsChangeStatus, VcsInfo, VcsType}; use ratatui::Terminal; use ratatui::backend::TestBackend; use ratatui::buffer::Buffer; @@ -1353,10 +1352,7 @@ mod remote_comments_snapshot_tests { fn info(&self) -> &VcsInfo { &self.info } - fn get_working_tree_diff( - &self, - _highlighter: &SyntaxHighlighter, - ) -> TuicrResult> { + fn get_working_tree_diff(&self) -> TuicrResult { Err(TuicrError::NoChanges) } fn fetch_context_lines( @@ -1504,7 +1500,7 @@ mod remote_comments_snapshot_tests { Theme::dark(), None, false, - vec![sample_diff_file()], + (vec![sample_diff_file()], Vec::new()), session, DiffSource::PullRequest(Box::new(pr)), InputMode::Normal, @@ -1536,7 +1532,7 @@ mod remote_comments_snapshot_tests { Theme::dark(), None, false, - diff_files, + (diff_files, Vec::new()), session, DiffSource::CommitRange(vec!["HEAD".to_string()]), InputMode::Normal, diff --git a/src/ui/file_list.rs b/src/ui/file_list.rs index e8c874bf..7c6efe16 100644 --- a/src/ui/file_list.rs +++ b/src/ui/file_list.rs @@ -67,20 +67,16 @@ pub(super) fn render_file_list(frame: &mut Frame, app: &mut App, area: Rect) { } let scroll_x = app.file_list_state.scroll_x; - // When diff panel is focused, sync file list selection to current file - // But preserve the current offset to not interfere with manual scrolling - if app.focused_panel == FocusedPanel::Diff { + // Skip when the user has wheel-scrolled the file list independently; + // see App::manual_file_list_scroll. + if app.focused_panel == FocusedPanel::Diff && !app.manual_file_list_scroll { let current_file_idx = app.diff_state.current_file_idx; for (tree_idx, item) in visible_items.iter().enumerate() { if let FileTreeItem::File { file_idx, .. } = item && *file_idx == current_file_idx { if app.file_list_state.selected() != tree_idx { - // Save current offset before changing selection - let current_offset = app.file_list_state.list_state.offset(); app.file_list_state.select(tree_idx); - // Restore offset to prevent auto-scrolling - *app.file_list_state.list_state.offset_mut() = current_offset; } break; } diff --git a/src/ui/selector.rs b/src/ui/selector.rs index 1545e9a0..a8f8f69f 100644 --- a/src/ui/selector.rs +++ b/src/ui/selector.rs @@ -470,12 +470,11 @@ mod selector_render_snapshot_tests { use crate::error::TuicrError; use crate::forge::selector::PullRequestsTab; use crate::forge::traits::{ForgeRepository, PullRequestSummary}; - use crate::model::{DiffFile, DiffLine, FileStatus, ReviewSession, SessionDiffSource}; - use crate::syntax::SyntaxHighlighter; + use crate::model::{DiffLine, FileStatus, ReviewSession, SessionDiffSource}; use crate::theme::Theme; use crate::ui::render; use crate::vcs::CommitInfo; - use crate::vcs::traits::{VcsBackend, VcsChangeStatus, VcsInfo, VcsType}; + use crate::vcs::traits::{DiffWithJobs, VcsBackend, VcsChangeStatus, VcsInfo, VcsType}; use chrono::{TimeZone, Utc}; use ratatui::Terminal; use ratatui::backend::TestBackend; @@ -492,10 +491,7 @@ mod selector_render_snapshot_tests { &self.info } - fn get_working_tree_diff( - &self, - _highlighter: &SyntaxHighlighter, - ) -> TuicrResult> { + fn get_working_tree_diff(&self) -> TuicrResult { Err(TuicrError::NoChanges) } @@ -560,7 +556,7 @@ mod selector_render_snapshot_tests { Theme::dark(), None, false, - Vec::new(), + (Vec::new(), Vec::new()), session, DiffSource::WorkingTree, InputMode::CommitSelect, diff --git a/src/ui/status_bar.rs b/src/ui/status_bar.rs index 2ec1a567..3f5d2f00 100644 --- a/src/ui/status_bar.rs +++ b/src/ui/status_bar.rs @@ -540,10 +540,9 @@ mod pr_header_snapshot_tests { use crate::error::Result as TuicrResult; use crate::error::TuicrError; use crate::forge::traits::{ForgeRepository, PrSessionKey}; - use crate::model::{DiffFile, DiffLine, FileStatus, ReviewSession, SessionDiffSource}; - use crate::syntax::SyntaxHighlighter; + use crate::model::{DiffLine, FileStatus, ReviewSession, SessionDiffSource}; use crate::theme::Theme; - use crate::vcs::traits::{VcsBackend, VcsInfo, VcsType}; + use crate::vcs::traits::{DiffWithJobs, VcsBackend, VcsInfo, VcsType}; use ratatui::Terminal; use ratatui::backend::TestBackend; use ratatui::buffer::Buffer; @@ -556,10 +555,7 @@ mod pr_header_snapshot_tests { fn info(&self) -> &VcsInfo { &self.info } - fn get_working_tree_diff( - &self, - _highlighter: &SyntaxHighlighter, - ) -> TuicrResult> { + fn get_working_tree_diff(&self) -> TuicrResult { Err(TuicrError::NoChanges) } fn fetch_context_lines( @@ -622,7 +618,7 @@ mod pr_header_snapshot_tests { Theme::dark(), None, false, - Vec::new(), + (Vec::new(), Vec::new()), session, DiffSource::PullRequest(Box::new(pr)), InputMode::Normal, diff --git a/src/ui/submit_modals.rs b/src/ui/submit_modals.rs index 2b78c18a..abf429cf 100644 --- a/src/ui/submit_modals.rs +++ b/src/ui/submit_modals.rs @@ -292,10 +292,9 @@ mod tests { use crate::model::ReviewSession; use crate::model::comment::{Comment, CommentType}; use crate::model::diff_types::FileStatus; - use crate::model::{DiffFile, DiffLine, SessionDiffSource}; - use crate::syntax::SyntaxHighlighter; + use crate::model::{DiffLine, SessionDiffSource}; use crate::theme::Theme; - use crate::vcs::traits::{VcsBackend, VcsChangeStatus, VcsInfo, VcsType}; + use crate::vcs::traits::{DiffWithJobs, VcsBackend, VcsChangeStatus, VcsInfo, VcsType}; use ratatui::Terminal; use ratatui::backend::TestBackend; use ratatui::buffer::Buffer; @@ -309,7 +308,7 @@ mod tests { fn info(&self) -> &VcsInfo { &self.info } - fn get_working_tree_diff(&self, _h: &SyntaxHighlighter) -> TuicrResult> { + fn get_working_tree_diff(&self) -> TuicrResult { Err(TuicrError::NoChanges) } fn fetch_context_lines( @@ -374,7 +373,7 @@ mod tests { Theme::dark(), None, false, - Vec::new(), + (Vec::new(), Vec::new()), session, DiffSource::PullRequest(Box::new(pr_source)), InputMode::Normal, diff --git a/src/vcs/diff_parser.rs b/src/vcs/diff_parser.rs index e1555892..5621778c 100644 --- a/src/vcs/diff_parser.rs +++ b/src/vcs/diff_parser.rs @@ -9,7 +9,8 @@ use std::path::PathBuf; use crate::error::{Result, TuicrError}; use crate::model::{DiffFile, DiffHunk, DiffLine, FileStatus, LineOrigin}; -use crate::syntax::{SyntaxHighlighter, needs_full_file_highlight}; +use crate::syntax::{HighlightJob, HighlightJobKind, SyntaxHighlighter, needs_full_file_highlight}; +use crate::vcs::DiffWithJobs; /// Diff format variants for different VCS tools. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -20,32 +21,26 @@ pub enum DiffFormat { GitStyle, } -/// Parse unified diff output into DiffFile structures. -pub fn parse_unified_diff( - diff_text: &str, - format: DiffFormat, - highlighter: &SyntaxHighlighter, -) -> Result> { +/// Parse unified diff output into DiffFile structures plus highlight jobs. +/// DiffLines come back with `highlighted_spans = None`; the streaming worker +/// fills them in. +pub fn parse_unified_diff(diff_text: &str, format: DiffFormat) -> Result { parse_unified_diff_lines( diff_text.lines().map(|line| Ok(Cow::Borrowed(line))), format, - highlighter, ) } -/// Parse unified diff lines into DiffFile structures. +/// Parse unified diff lines into DiffFile structures plus highlight jobs. /// /// This entry point is used by Git's sparse-checkout backend so large diffs can /// be parsed directly from command stdout instead of buffering the whole patch. -pub fn parse_unified_diff_lines<'a, I>( - diff_lines: I, - format: DiffFormat, - highlighter: &SyntaxHighlighter, -) -> Result> +pub fn parse_unified_diff_lines<'a, I>(diff_lines: I, format: DiffFormat) -> Result where I: Iterator>>, { let mut files: Vec = Vec::new(); + let mut jobs: Vec = Vec::new(); let mut lines = diff_lines.peekable(); let header_prefix = match format { @@ -94,15 +89,23 @@ where continue; } - let file_path = new_path.as_ref().or(old_path.as_ref()); - let mut hunks = Vec::new(); + let file_path = new_path.as_ref().or(old_path.as_ref()).cloned(); + let file_idx = files.len(); + let mut hunks: Vec = Vec::new(); // Parse hunks until next file or end while let Some(line) = peek_line(&mut lines)? { if line.starts_with("diff ") { break; } else if line.starts_with("@@") { - if let Some(hunk) = parse_hunk(&mut lines, file_path, highlighter)? { + let hunk_idx = hunks.len(); + if let Some(hunk) = parse_hunk( + &mut lines, + file_idx, + hunk_idx, + file_path.as_deref(), + &mut jobs, + )? { hunks.push(hunk); } } else { @@ -128,7 +131,7 @@ where return Err(TuicrError::NoChanges); } - Ok(files) + Ok((files, jobs)) } fn next_line<'a, I>(lines: &mut std::iter::Peekable) -> Result>> @@ -252,8 +255,10 @@ where fn parse_hunk<'a, I>( lines: &mut std::iter::Peekable, - file_path: Option<&PathBuf>, - highlighter: &SyntaxHighlighter, + file_idx: usize, + hunk_idx: usize, + file_path: Option<&std::path::Path>, + jobs: &mut Vec, ) -> Result> where I: Iterator>>, @@ -326,39 +331,39 @@ where line_numbers.push((old_ln, new_ln)); } - // Apply syntax highlighting by side-specific sequence to keep parser state valid. - // Container grammars skip per-hunk highlighting; the full-file post-pass - // (`enhance_with_full_file_highlight`) overwrites these spans anyway. - let highlight_sequences = - SyntaxHighlighter::split_diff_lines_for_highlighting(&line_contents, &line_origins); - let (old_highlighted_lines, new_highlighted_lines) = match file_path { - Some(path) if !needs_full_file_highlight(path) => ( - highlighter.highlight_file_lines(path, &highlight_sequences.old_lines), - highlighter.highlight_file_lines(path, &highlight_sequences.new_lines), - ), - _ => (None, None), - }; - - // Build DiffLines - let mut diff_lines: Vec = Vec::with_capacity(line_contents.len()); - for (idx, content) in line_contents.into_iter().enumerate() { - let origin = line_origins[idx]; - let (old_lineno, new_lineno) = line_numbers[idx]; - - let highlighted_spans = highlighter.highlighted_line_for_diff_with_background( - old_highlighted_lines.as_deref(), - new_highlighted_lines.as_deref(), - highlight_sequences.old_line_indices[idx], - highlight_sequences.new_line_indices[idx], - origin, - ); + let diff_lines: Vec = line_contents + .iter() + .enumerate() + .map(|(idx, content)| { + let (old_lineno, new_lineno) = line_numbers[idx]; + DiffLine { + origin: line_origins[idx], + content: content.clone(), + old_lineno, + new_lineno, + highlighted_spans: None, + } + }) + .collect(); - diff_lines.push(DiffLine { - origin, - content, - old_lineno, - new_lineno, - highlighted_spans, + // Container grammars skip per-hunk highlighting; the full-file post-pass + // emitted by the backend will fill spans for the whole file. + if let Some(path) = file_path + && !needs_full_file_highlight(path) + { + let sequences = + SyntaxHighlighter::split_diff_lines_for_highlighting(&line_contents, &line_origins); + jobs.push(HighlightJob { + file_idx, + syntax_path: path.to_path_buf(), + kind: HighlightJobKind::Hunk { + hunk_idx, + old_lines: sequences.old_lines, + new_lines: sequences.new_lines, + old_line_indices: sequences.old_line_indices, + new_line_indices: sequences.new_line_indices, + line_origins, + }, }); } @@ -465,11 +470,11 @@ mod tests { #[test] fn should_return_no_changes_for_empty_diff() { assert!(matches!( - parse_unified_diff("", DiffFormat::Hg, &SyntaxHighlighter::default()), + parse_unified_diff("", DiffFormat::Hg), Err(TuicrError::NoChanges) )); assert!(matches!( - parse_unified_diff("", DiffFormat::GitStyle, &SyntaxHighlighter::default()), + parse_unified_diff("", DiffFormat::GitStyle), Err(TuicrError::NoChanges) )); } @@ -531,8 +536,7 @@ mod tests { } "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert_eq!(result[0].status, FileStatus::Modified); assert_eq!(result[0].hunks.len(), 1); @@ -549,8 +553,7 @@ mod tests { + new "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; let lines = &result[0].hunks[0].lines; assert_eq!(lines[0].content, " old"); @@ -568,8 +571,7 @@ mod tests { +} "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert_eq!(result[0].status, FileStatus::Added); assert!(result[0].old_path.is_none()); @@ -589,8 +591,7 @@ mod tests { -} "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert_eq!(result[0].status, FileStatus::Deleted); assert_eq!( @@ -616,8 +617,7 @@ diff -r abc123 file2.rs -remove "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 2); assert_eq!( result[0].new_path.as_ref().unwrap().to_str().unwrap(), @@ -645,8 +645,7 @@ diff -r abc123 file2.rs } "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert_eq!(result[0].hunks.len(), 2); assert_eq!(result[0].hunks[0].old_start, 1); @@ -665,8 +664,7 @@ rename to new_name.rs +new content "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert_eq!(result[0].status, FileStatus::Renamed); assert_eq!( @@ -685,8 +683,7 @@ rename to new_name.rs Binary file image.png has changed "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert!(result[0].is_binary); assert!(result[0].hunks.is_empty()); @@ -700,8 +697,7 @@ rename from old_name.rs rename to new_name.rs "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert_eq!(result[0].status, FileStatus::Renamed); assert_eq!(result[0].old_path, Some(PathBuf::from("old_name.rs"))); @@ -717,8 +713,7 @@ copy from source.rs copy to dest.rs "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert_eq!(result[0].status, FileStatus::Copied); assert_eq!(result[0].old_path, Some(PathBuf::from("source.rs"))); @@ -739,8 +734,7 @@ copy to dest.rs +added line "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert_eq!(result[0].status, FileStatus::Copied); assert_eq!(result[0].old_path, Some(PathBuf::from("source.rs"))); @@ -760,8 +754,7 @@ copy to dest.rs \ No newline at end of file "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; assert_eq!(result.len(), 1); assert_eq!(result[0].hunks[0].lines.len(), 2); } @@ -779,8 +772,7 @@ copy to dest.rs context at 7->8 "#; - let result = - parse_unified_diff(diff, DiffFormat::Hg, &SyntaxHighlighter::default()).unwrap(); + let result = parse_unified_diff(diff, DiffFormat::Hg).unwrap().0; let lines = &result[0].hunks[0].lines; assert_eq!(lines[0].origin, LineOrigin::Context); @@ -817,8 +809,7 @@ copy to dest.rs line2 line3 "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert_eq!(files[0].new_path, Some(PathBuf::from("file.txt"))); assert_eq!(files[0].status, FileStatus::Modified); @@ -835,8 +826,7 @@ copy to dest.rs - old + new "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; let lines = &files[0].hunks[0].lines; assert_eq!(lines[0].content, " old"); @@ -857,8 +847,11 @@ copy to dest.rs ); "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let (mut files, jobs) = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap(); + let highlighter = SyntaxHighlighter::default(); + for update in crate::syntax::streaming::run_blocking(jobs, &highlighter) { + crate::syntax::streaming::apply_update(&mut files, &highlighter, update); + } let lines = &files[0].hunks[0].lines; assert_eq!(lines.len(), 5); @@ -880,8 +873,7 @@ new file mode 100644 +line1 +line2 "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert_eq!(files[0].status, FileStatus::Added); } @@ -896,8 +888,7 @@ deleted file mode 100644 -line1 -line2 "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert_eq!(files[0].status, FileStatus::Deleted); } @@ -909,8 +900,7 @@ deleted file mode 100644 rename from old.txt rename to new.txt "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert_eq!(files[0].status, FileStatus::Renamed); assert_eq!(files[0].old_path, Some(PathBuf::from("old.txt"))); @@ -930,8 +920,7 @@ rename to new.txt -old content +new content "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert_eq!(files[0].status, FileStatus::Renamed); assert_eq!(files[0].old_path, Some(PathBuf::from("old.txt"))); @@ -946,8 +935,7 @@ rename to new.txt copy from source.txt copy to dest.txt "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert_eq!(files[0].status, FileStatus::Copied); assert_eq!(files[0].old_path, Some(PathBuf::from("source.txt"))); @@ -967,8 +955,7 @@ copy to dest.txt original +added line "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert_eq!(files[0].status, FileStatus::Copied); assert_eq!(files[0].old_path, Some(PathBuf::from("source.txt"))); @@ -983,8 +970,7 @@ new file mode 100644 index 0000000000..abc1234567 Binary files /dev/null and b/image.png differ "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert!(files[0].is_binary); assert_eq!(files[0].status, FileStatus::Added); @@ -999,8 +985,7 @@ deleted file mode 100644 index abc1234567..0000000000 Binary files a/image.png and /dev/null differ "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert!(files[0].is_binary); assert_eq!(files[0].status, FileStatus::Deleted); @@ -1014,8 +999,7 @@ Binary files a/image.png and /dev/null differ index abc1234567..def7890123 100644 Binary files a/image.png and b/image.png differ "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert!(files[0].is_binary); assert_eq!(files[0].status, FileStatus::Modified); @@ -1038,8 +1022,7 @@ diff --git a/b.txt b/b.txt -foo +bar "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 2); assert_eq!(files[0].new_path, Some(PathBuf::from("a.txt"))); assert_eq!(files[1].new_path, Some(PathBuf::from("b.txt"))); @@ -1057,8 +1040,7 @@ diff --git a/b.txt b/b.txt +added2 more "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; let hunk = &files[0].hunks[0]; assert_eq!(hunk.lines[0].old_lineno, Some(5)); @@ -1084,8 +1066,7 @@ diff --git a/b.txt b/b.txt new file mode 100644 index 0000000000..e69de29bb2 "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert_eq!(files[0].status, FileStatus::Added); assert!(files[0].old_path.is_none()); @@ -1102,8 +1083,7 @@ index 0000000000..e69de29bb2 old mode 100644 new mode 100755 "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let files = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap().0; assert_eq!(files.len(), 1); assert_eq!(files[0].status, FileStatus::Modified); assert_eq!(files[0].old_path, Some(PathBuf::from("script.sh"))); @@ -1124,8 +1104,7 @@ literal 4 LcmeZB000M*0RR91 "#; - let files = - parse_unified_diff(diff, DiffFormat::GitStyle, &SyntaxHighlighter::default()).unwrap(); + let (files, _) = parse_unified_diff(diff, DiffFormat::GitStyle).unwrap(); assert_eq!(files.len(), 1); assert_eq!(files[0].old_path, Some(PathBuf::from("image.bin"))); @@ -1147,9 +1126,7 @@ LcmeZB000M*0RR91 .into_iter() .map(|line| Ok(Cow::Owned(line.to_string()))); - let files = - parse_unified_diff_lines(lines, DiffFormat::GitStyle, &SyntaxHighlighter::default()) - .unwrap(); + let (files, _) = parse_unified_diff_lines(lines, DiffFormat::GitStyle).unwrap(); assert_eq!(files.len(), 1); assert_eq!(files[0].new_path, Some(PathBuf::from("file.txt"))); diff --git a/src/vcs/file.rs b/src/vcs/file.rs index 76924501..3eabfda2 100644 --- a/src/vcs/file.rs +++ b/src/vcs/file.rs @@ -5,9 +5,9 @@ use ignore::WalkBuilder; use crate::error::{Result, TuicrError}; use crate::model::{DiffFile, DiffHunk, DiffLine, FileStatus, LineOrigin}; -use crate::syntax::SyntaxHighlighter; +use crate::syntax::{HighlightJob, HighlightJobKind, SyntaxHighlighter}; -use super::traits::{VcsBackend, VcsInfo, VcsType}; +use super::traits::{DiffWithJobs, VcsBackend, VcsInfo, VcsType}; /// A backend for reviewing files outside of a VCS repository (`--file`) /// and for the whole-repo `--all-files` mode. @@ -154,12 +154,15 @@ impl FileBackend { }) } + /// Build the [`DiffFile`] entry for one path, plus the data needed for a + /// background highlight job. Returns `None` to skip the file entirely + /// (binary, unreadable, empty). The job kind is `None` for `is_too_large` + /// placeholders that have no hunk to highlight. fn build_diff_file_for_path( &self, - highlighter: &SyntaxHighlighter, abs_path: &Path, file_size: u64, - ) -> Option { + ) -> Option<(DiffFile, Option)> { // Binary check first so a too-large binary is skipped (not surfaced // as a misleading is_too_large text placeholder), and so single-file // mode (which never went through `collect_text_files`) is also guarded. @@ -176,16 +179,19 @@ impl FileBackend { if file_size > MAX_FILE_BYTES { let hunks: Vec = Vec::new(); let content_hash = DiffFile::compute_content_hash(&hunks); - return Some(DiffFile { - old_path: None, - new_path: Some(rel_path), - status: FileStatus::Added, - hunks, - is_binary: false, - is_too_large: true, - is_commit_message: false, - content_hash, - }); + return Some(( + DiffFile { + old_path: None, + new_path: Some(rel_path), + status: FileStatus::Added, + hunks, + is_binary: false, + is_too_large: true, + is_commit_message: false, + content_hash, + }, + None, + )); } let content = std::fs::read_to_string(abs_path).ok()?; @@ -202,26 +208,15 @@ impl FileBackend { // Build line contents and origins for syntax highlighting let line_contents: Vec = lines.iter().map(|l| super::tabify(l)).collect(); let line_origins: Vec = vec![render_origin; line_contents.len()]; - - // Apply syntax highlighting - let highlight_sequences = + let sequences = SyntaxHighlighter::split_diff_lines_for_highlighting(&line_contents, &line_origins); - let new_highlighted_lines = - highlighter.highlight_file_lines(abs_path, &highlight_sequences.new_lines); - // Build DiffLines + // Build DiffLines without inline highlighting; the streaming worker + // will populate `highlighted_spans` once the diff is on screen. let mut diff_lines = Vec::with_capacity(lines.len()); for (i, content) in line_contents.iter().enumerate() { let line_num = (i + 1) as u32; - let highlighted_spans = highlighter.highlighted_line_for_diff_with_background( - None, - new_highlighted_lines.as_deref(), - None, - highlight_sequences.new_line_indices[i], - render_origin, - ); - // Pristine context lines need both old_lineno and new_lineno // populated so the side-by-side and unified renderers walk the // gutter math correctly. @@ -235,7 +230,7 @@ impl FileBackend { content: content.clone(), old_lineno, new_lineno: Some(line_num), - highlighted_spans, + highlighted_spans: None, }); } @@ -267,7 +262,7 @@ impl FileBackend { let hunks = vec![hunk]; let content_hash = DiffFile::compute_content_hash(&hunks); - Some(DiffFile { + let file = DiffFile { old_path: None, new_path: Some(rel_path), status: file_status, @@ -276,7 +271,18 @@ impl FileBackend { is_too_large: false, is_commit_message: false, content_hash, - }) + }; + + let job_kind = HighlightJobKind::Hunk { + hunk_idx: 0, + old_lines: sequences.old_lines, + new_lines: sequences.new_lines, + old_line_indices: sequences.old_line_indices, + new_line_indices: sequences.new_line_indices, + line_origins, + }; + + Some((file, Some(job_kind))) } } @@ -285,18 +291,29 @@ impl VcsBackend for FileBackend { &self.info } - fn get_working_tree_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { - let diff_files: Vec = self - .files - .iter() - .filter_map(|(p, size)| self.build_diff_file_for_path(highlighter, p, *size)) - .collect(); + fn get_working_tree_diff(&self) -> Result { + let mut diff_files: Vec = Vec::with_capacity(self.files.len()); + let mut jobs: Vec = Vec::with_capacity(self.files.len()); + + for (abs_path, size) in &self.files { + let Some((file, job_kind)) = self.build_diff_file_for_path(abs_path, *size) else { + continue; + }; + if let Some(kind) = job_kind { + jobs.push(HighlightJob { + file_idx: diff_files.len(), + syntax_path: abs_path.clone(), + kind, + }); + } + diff_files.push(file); + } if diff_files.is_empty() { return Err(TuicrError::NoChanges); } - Ok(diff_files) + Ok((diff_files, jobs)) } fn fetch_context_lines( @@ -401,11 +418,6 @@ mod tests { use std::fs; use super::*; - use crate::syntax::SyntaxHighlighter; - - fn highlighter() -> SyntaxHighlighter { - SyntaxHighlighter::default() - } #[test] fn single_file_mode_returns_one_diff_file() { @@ -414,9 +426,11 @@ mod tests { fs::write(&path, "alpha\nbeta\n").unwrap(); let backend = FileBackend::new(path.to_str().unwrap()).unwrap(); - let diffs = backend.get_working_tree_diff(&highlighter()).unwrap(); + let (diffs, jobs) = backend.get_working_tree_diff().unwrap(); assert_eq!(diffs.len(), 1); + assert_eq!(jobs.len(), 1); + assert_eq!(jobs[0].file_idx, 0); assert_eq!( diffs[0].new_path.as_deref().unwrap(), Path::new("hello.txt") @@ -432,7 +446,7 @@ mod tests { fs::write(dir.path().join("ignored.txt"), "skip me\n").unwrap(); let backend = FileBackend::new(dir.path().to_str().unwrap()).unwrap(); - let diffs = backend.get_working_tree_diff(&highlighter()).unwrap(); + let (diffs, _jobs) = backend.get_working_tree_diff().unwrap(); let names: Vec<_> = diffs .iter() @@ -459,7 +473,7 @@ mod tests { fs::write(sub.join("inner.txt"), "x\n").unwrap(); let backend = FileBackend::new(dir.path().to_str().unwrap()).unwrap(); - let diffs = backend.get_working_tree_diff(&highlighter()).unwrap(); + let (diffs, _jobs) = backend.get_working_tree_diff().unwrap(); assert_eq!(diffs.len(), 1); assert_eq!( @@ -481,7 +495,7 @@ mod tests { .unwrap(); let root = dir.path().canonicalize().unwrap(); let backend = FileBackend::new_pristine(vec![path.clone()], root).unwrap(); - let diffs = backend.get_working_tree_diff(&highlighter()).unwrap(); + let (diffs, _jobs) = backend.get_working_tree_diff().unwrap(); assert_eq!(diffs.len(), 1); let hunk = &diffs[0].hunks[0]; @@ -504,7 +518,7 @@ mod tests { fs::write(dir.path().join("a.txt"), "alpha\nbeta\n").unwrap(); let backend = FileBackend::new(dir.path().to_str().unwrap()).unwrap(); - let diffs = backend.get_working_tree_diff(&highlighter()).unwrap(); + let (diffs, _jobs) = backend.get_working_tree_diff().unwrap(); assert_eq!(diffs.len(), 1); let hunk = &diffs[0].hunks[0]; @@ -524,7 +538,7 @@ mod tests { let root = dir.path().canonicalize().unwrap(); let backend = FileBackend::new_pristine(vec![text_path.clone(), bin_path], root).unwrap(); - let diffs = backend.get_working_tree_diff(&highlighter()).unwrap(); + let (diffs, _jobs) = backend.get_working_tree_diff().unwrap(); assert_eq!(diffs.len(), 1); assert_eq!(diffs[0].new_path.as_deref().unwrap(), Path::new("text.txt")); diff --git a/src/vcs/git/cli.rs b/src/vcs/git/cli.rs index bb2531d9..3c647289 100644 --- a/src/vcs/git/cli.rs +++ b/src/vcs/git/cli.rs @@ -10,13 +10,13 @@ use chrono::{TimeZone, Utc}; use crate::error::{Result, TuicrError}; use crate::model::{DiffFile, DiffHunk, DiffLine, FileStatus, LineOrigin, LineSide}; -use crate::syntax::SyntaxHighlighter; +use crate::syntax::{HighlightJob, HighlightJobKind}; use crate::vcs::diff_parser::{self, DiffFormat}; use crate::vcs::{ - ChangeKind, CommitInfo, DiffWhitespaceMode, ResolvedRevisionRange, RevisionDiffTarget, - VcsBackend, VcsChangeStatus, VcsInfo, + ChangeKind, CommitInfo, DiffWhitespaceMode, DiffWithJobs, ResolvedRevisionRange, + RevisionDiffTarget, VcsBackend, VcsChangeStatus, VcsInfo, }; -use crate::vcs::{container_file_paths, enhance_with_full_file_highlight, tabify}; +use crate::vcs::{append_container_full_file_jobs, container_file_paths, tabify}; use super::{ GitRepoMode, RevisionExpression, git_bool_config_enabled, git_command_error, @@ -89,19 +89,18 @@ impl GitCliBackend { include_untracked: bool, old_source: GitContentSource<'_>, new_source: GitContentSource<'_>, - highlighter: &SyntaxHighlighter, - ) -> Result> { + ) -> Result { if self.whitespace_mode.ignores_all() { args.insert(1, "--ignore-all-space".to_string()); } - let mut files = match run_git_diff_command(&self.root_path, args, highlighter) { - Ok(files) => files, - Err(TuicrError::NoChanges) => Vec::new(), + let (mut files, mut jobs) = match run_git_diff_command(&self.root_path, args) { + Ok(diff) => diff, + Err(TuicrError::NoChanges) => (Vec::new(), Vec::new()), Err(err) => return Err(err), }; if include_untracked { - append_untracked_cli_diffs(&self.root_path, &mut files, highlighter)?; + append_untracked_cli_diffs(&self.root_path, &mut files, &mut jobs)?; } normalize_git_cli_paths(&mut files); @@ -113,9 +112,10 @@ impl GitCliBackend { git_source_content_cache(&self.root_path, old_source, &files, LineSide::Old); let new_cache = git_source_content_cache(&self.root_path, new_source, &files, LineSide::New); - enhance_with_full_file_highlight( - &mut files, - highlighter, + + append_container_full_file_jobs( + &files, + &mut jobs, |path| { read_path_from_git_source_cached( &self.root_path, @@ -133,7 +133,7 @@ impl GitCliBackend { ) }, ); - Ok(files) + Ok((files, jobs)) } fn read_file_content( @@ -187,17 +187,16 @@ impl VcsBackend for GitCliBackend { true } - fn get_working_tree_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { self.get_cli_diff( strings(["diff", "--no-ext-diff", "--binary", "HEAD", "--"]), true, GitContentSource::Revision("HEAD"), GitContentSource::Workdir, - highlighter, ) } - fn get_staged_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { + fn get_staged_diff(&self) -> Result { let old_source = if run_git_command(&self.root_path, &["rev-parse", "--verify", "HEAD"]).is_ok() { GitContentSource::Revision("HEAD") @@ -209,17 +208,15 @@ impl VcsBackend for GitCliBackend { false, old_source, GitContentSource::Index, - highlighter, ) } - fn get_unstaged_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { + fn get_unstaged_diff(&self) -> Result { self.get_cli_diff( strings(["diff", "--no-ext-diff", "--binary", "--"]), true, GitContentSource::Index, GitContentSource::Workdir, - highlighter, ) } @@ -326,8 +323,7 @@ impl VcsBackend for GitCliBackend { fn get_commit_range_diff( &self, revision_range: &ResolvedRevisionRange<'_>, - highlighter: &SyntaxHighlighter, - ) -> Result> { + ) -> Result { if revision_range.commit_ids.is_empty() { return Err(TuicrError::NoChanges); } @@ -354,7 +350,6 @@ impl VcsBackend for GitCliBackend { false, GitContentSource::Revision(&base_rev), GitContentSource::Revision(&newest_rev), - highlighter, ) } @@ -375,11 +370,7 @@ impl VcsBackend for GitCliBackend { Ok(parse_commit_records(&output, &branch_tip_names)) } - fn get_working_tree_with_commits_diff( - &self, - commit_ids: &[String], - highlighter: &SyntaxHighlighter, - ) -> Result> { + fn get_working_tree_with_commits_diff(&self, commit_ids: &[String]) -> Result { if commit_ids.is_empty() { return Err(TuicrError::NoChanges); } @@ -396,7 +387,6 @@ impl VcsBackend for GitCliBackend { true, GitContentSource::Revision(&base_rev), GitContentSource::Workdir, - highlighter, ) } @@ -578,11 +568,7 @@ fn has_untracked_changes(workdir: &Path, pathspecs: &[String]) -> Result { Ok(false) } -fn run_git_diff_command( - workdir: &Path, - args: Vec, - highlighter: &SyntaxHighlighter, -) -> Result> { +fn run_git_diff_command(workdir: &Path, args: Vec) -> Result { let mut child = Command::new("git") .current_dir(workdir) .args(&args) @@ -608,8 +594,7 @@ fn run_git_diff_command( let diff_lines = BufReader::new(stdout) .lines() .map(|line| line.map(Cow::Owned).map_err(TuicrError::from)); - let parse_result = - diff_parser::parse_unified_diff_lines(diff_lines, DiffFormat::GitStyle, highlighter); + let parse_result = diff_parser::parse_unified_diff_lines(diff_lines, DiffFormat::GitStyle); let status = child.wait()?; let stderr = stderr_reader @@ -630,16 +615,19 @@ fn run_git_diff_command( fn append_untracked_cli_diffs( workdir: &Path, files: &mut Vec, - highlighter: &SyntaxHighlighter, + jobs: &mut Vec, ) -> Result { let pathspecs = sparse_checkout_untracked_pathspecs(workdir)?; let previous_len = files.len(); for_each_untracked_path(workdir, &pathspecs, |path| { let full_path = workdir.join(&path); - let Some(file) = build_untracked_diff_file(&path, &full_path, highlighter) else { + let Some((file, job)) = build_untracked_diff_file(&path, &full_path, files.len()) else { return Ok(()); }; files.push(file); + if let Some(job) = job { + jobs.push(job); + } Ok(()) })?; Ok(files.len().saturating_sub(previous_len)) @@ -690,16 +678,16 @@ fn is_simple_sparse_path(pattern: &str) -> bool { fn build_untracked_diff_file( path: &Path, full_path: &Path, - highlighter: &SyntaxHighlighter, -) -> Option { + file_idx: usize, +) -> Option<(DiffFile, Option)> { let metadata = full_path.metadata().ok()?; if metadata.len() > MAX_UNTRACKED_FILE_SIZE { - return Some(diff_file_without_hunks(path, false, true)); + return Some((diff_file_without_hunks(path, false, true), None)); } let bytes = fs::read(full_path).ok()?; if bytes.contains(&0) { - return Some(diff_file_without_hunks(path, true, false)); + return Some((diff_file_without_hunks(path, true, false), None)); } let content = String::from_utf8_lossy(&bytes); @@ -709,10 +697,30 @@ fn build_untracked_diff_file( .collect(); if lines.is_empty() { - return Some(diff_file_without_hunks(path, false, false)); - } + return Some((diff_file_without_hunks(path, false, false), None)); + } + + let line_count = lines.len(); + let line_origins = vec![LineOrigin::Addition; line_count]; + let new_line_indices: Vec> = (0..line_count).map(Some).collect(); + // Untracked files surface as a single all-additions hunk; emit a per-hunk + // highlight job mirroring what `parse_unified_diff_lines` would have + // produced. Container grammars (Vue/Svelte/...) will be picked up by the + // FullFile pass in `get_cli_diff`, so always emit the Hunk job here and + // let the worker drop it if the syntax set has no grammar for the path. + let job = HighlightJob { + file_idx, + syntax_path: path.to_path_buf(), + kind: HighlightJobKind::Hunk { + hunk_idx: 0, + old_lines: Vec::new(), + new_lines: lines.clone(), + old_line_indices: vec![None; line_count], + new_line_indices, + line_origins, + }, + }; - let highlighted = highlighter.highlight_file_lines(path, &lines); let diff_lines: Vec = lines .into_iter() .enumerate() @@ -721,13 +729,7 @@ fn build_untracked_diff_file( content, old_lineno: None, new_lineno: Some((idx + 1) as u32), - highlighted_spans: highlighter.highlighted_line_for_diff_with_background( - None, - highlighted.as_deref(), - None, - Some(idx), - LineOrigin::Addition, - ), + highlighted_spans: None, }) .collect(); @@ -742,16 +744,19 @@ fn build_untracked_diff_file( }]; let content_hash = DiffFile::compute_content_hash(&hunks); - Some(DiffFile { - old_path: None, - new_path: Some(path.to_path_buf()), - status: FileStatus::Added, - hunks, - is_binary: false, - is_too_large: false, - is_commit_message: false, - content_hash, - }) + Some(( + DiffFile { + old_path: None, + new_path: Some(path.to_path_buf()), + status: FileStatus::Added, + hunks, + is_binary: false, + is_too_large: false, + is_commit_message: false, + content_hash, + }, + Some(job), + )) } fn diff_file_without_hunks(path: &Path, is_binary: bool, is_too_large: bool) -> DiffFile { @@ -1199,10 +1204,9 @@ mod tests { fs::remove_file(workdir.join(path)).expect("failed to remove file"); } - fn summarize_files( - files: Vec, - ) -> Vec<(Option, Option, FileStatus)> { - let mut summary: Vec<_> = files + fn summarize_files(diff: DiffWithJobs) -> Vec<(Option, Option, FileStatus)> { + let mut summary: Vec<_> = diff + .0 .into_iter() .map(|file| (file.old_path, file.new_path, file.status)) .collect(); @@ -1395,14 +1399,11 @@ mod tests { fn reads_commit_range_diff_in_sparse_index() { let (_temp_dir, backend, ids) = setup_sparse_index_repo(); - let files = backend - .get_commit_range_diff( - &ResolvedRevisionRange::from_owned_commit_ids( - vec![ids[1].clone()], - RevisionDiffTarget::CommitList, - ), - &SyntaxHighlighter::default(), - ) + let (files, _) = backend + .get_commit_range_diff(&ResolvedRevisionRange::from_owned_commit_ids( + vec![ids[1].clone()], + RevisionDiffTarget::CommitList, + )) .expect("failed to get sparse commit range diff"); assert_eq!(files.len(), 1); @@ -1417,7 +1418,7 @@ mod tests { let (_temp_dir, backend, _ids) = setup_sparse_index_repo(); assert!(matches!( - backend.get_working_tree_diff(&SyntaxHighlighter::default()), + backend.get_working_tree_diff(), Err(TuicrError::NoChanges) )); } @@ -1430,8 +1431,8 @@ mod tests { write_file(workdir, "keep/new.txt", "new sparse file\n"); write_file(workdir, "hidden/outside.txt", "outside cone\n"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) + let (files, _) = backend + .get_working_tree_diff() .expect("failed to get sparse working tree diff"); let paths: Vec<_> = files @@ -1452,8 +1453,8 @@ mod tests { backend .stage_file(Path::new("keep/file.txt")) .expect("failed to stage file"); - let files = backend - .get_staged_diff(&SyntaxHighlighter::default()) + let (files, _) = backend + .get_staged_diff() .expect("failed to get sparse staged diff"); assert_eq!(files.len(), 1); @@ -1514,37 +1515,28 @@ mod tests { #[test] fn cli_diff_outputs_match_libgit2_for_shared_git_operations() { let (_temp_dir, cli_backend, repo, ids) = setup_standard_parity_repo(); - let highlighter = SyntaxHighlighter::default(); assert_eq!( - summarize_files(cli_backend.get_working_tree_diff(&highlighter).unwrap()), + summarize_files(cli_backend.get_working_tree_diff().unwrap()), summarize_files( - diff::get_working_tree_diff(&repo, DiffWhitespaceMode::Normal, &highlighter) - .unwrap() + diff::get_working_tree_diff(&repo, DiffWhitespaceMode::Normal).unwrap() ) ); assert_eq!( - summarize_files(cli_backend.get_staged_diff(&highlighter).unwrap()), - summarize_files( - diff::get_staged_diff(&repo, DiffWhitespaceMode::Normal, &highlighter).unwrap() - ) + summarize_files(cli_backend.get_staged_diff().unwrap()), + summarize_files(diff::get_staged_diff(&repo, DiffWhitespaceMode::Normal).unwrap()) ); assert_eq!( - summarize_files(cli_backend.get_unstaged_diff(&highlighter).unwrap()), - summarize_files( - diff::get_unstaged_diff(&repo, DiffWhitespaceMode::Normal, &highlighter).unwrap() - ) + summarize_files(cli_backend.get_unstaged_diff().unwrap()), + summarize_files(diff::get_unstaged_diff(&repo, DiffWhitespaceMode::Normal).unwrap()) ); assert_eq!( summarize_files( cli_backend - .get_commit_range_diff( - &ResolvedRevisionRange::from_owned_commit_ids( - vec![ids[1].clone()], - RevisionDiffTarget::CommitList, - ), - &highlighter - ) + .get_commit_range_diff(&ResolvedRevisionRange::from_owned_commit_ids( + vec![ids[1].clone()], + RevisionDiffTarget::CommitList, + )) .unwrap() ), summarize_files( @@ -1555,7 +1547,6 @@ mod tests { RevisionDiffTarget::CommitList, ), DiffWhitespaceMode::Normal, - &highlighter, ) .unwrap() ) @@ -1563,7 +1554,7 @@ mod tests { assert_eq!( summarize_files( cli_backend - .get_working_tree_with_commits_diff(&[ids[1].clone()], &highlighter) + .get_working_tree_with_commits_diff(&[ids[1].clone()]) .unwrap() ), summarize_files( @@ -1571,7 +1562,6 @@ mod tests { &repo, &[ids[1].clone()], DiffWhitespaceMode::Normal, - &highlighter, ) .unwrap() ) @@ -1616,7 +1606,6 @@ mod tests { // endpoints, not the first selected commit's parent. // Otherwise changes already present in the left ref appear in review. let (_temp_dir, cli_backend, repo, left_id, right_id) = setup_merge_range_repo(); - let highlighter = SyntaxHighlighter::default(); let revisions = format!("{left_id}..{right_id}"); let cli_range = cli_backend @@ -1630,7 +1619,7 @@ mod tests { } ); let cli_files = cli_backend - .get_commit_range_diff(&cli_range, &highlighter) + .get_commit_range_diff(&cli_range) .expect("failed to get cli range diff"); let libgit2_range = @@ -1642,13 +1631,9 @@ mod tests { head: right_id, } ); - let libgit2_files = diff::get_commit_range_diff( - &repo, - &libgit2_range, - DiffWhitespaceMode::Normal, - &highlighter, - ) - .expect("failed to get libgit2 range diff"); + let libgit2_files = + diff::get_commit_range_diff(&repo, &libgit2_range, DiffWhitespaceMode::Normal) + .expect("failed to get libgit2 range diff"); assert_eq!( summarize_files(cli_files), @@ -1686,13 +1671,13 @@ mod tests { write_file(workdir, "file.txt", " alpha \n beta\n"); assert!(matches!( - backend.get_working_tree_diff(&SyntaxHighlighter::default()), + backend.get_working_tree_diff(), Err(TuicrError::NoChanges) )); write_file(workdir, "file.txt", " alpha \ngamma\n"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) + let (files, _) = backend + .get_working_tree_diff() .expect("non-whitespace edit should still produce a diff"); assert_eq!(files.len(), 1); } diff --git a/src/vcs/git/diff.rs b/src/vcs/git/diff.rs index 4bf448c7..7ac86e21 100644 --- a/src/vcs/git/diff.rs +++ b/src/vcs/git/diff.rs @@ -3,17 +3,16 @@ use std::path::{Path, PathBuf}; use crate::error::{Result, TuicrError}; use crate::model::{DiffFile, DiffHunk, DiffLine, FileStatus, LineOrigin}; -use crate::syntax::{SyntaxHighlighter, needs_full_file_highlight}; +use crate::syntax::{HighlightJob, HighlightJobKind, SyntaxHighlighter, needs_full_file_highlight}; use crate::vcs::traits::{ ChangeKind, DiffWhitespaceMode, ResolvedRevisionRange, RevisionDiffTarget, }; -use crate::vcs::{enhance_with_full_file_highlight, tabify}; +use crate::vcs::{DiffWithJobs, append_container_full_file_jobs, tabify}; pub fn get_working_tree_diff( repo: &Repository, whitespace_mode: DiffWhitespaceMode, - highlighter: &SyntaxHighlighter, -) -> Result> { +) -> Result { // Unborn HEAD (fresh `git init` / `git clone` of an empty remote) has no // tree to compare against; diff against an empty baseline so freshly // staged/added files still surface in the working-tree review. @@ -25,17 +24,17 @@ pub fn get_working_tree_diff( opts.recurse_untracked_dirs(true); let diff = repo.diff_tree_to_workdir_with_index(head.as_ref(), Some(&mut opts))?; - let mut files = parse_diff(&diff, highlighter)?; - enhance_with_full_file_highlight( - &mut files, - highlighter, + let (files, mut jobs) = parse_diff(&diff)?; + append_container_full_file_jobs( + &files, + &mut jobs, |path| { head.as_ref() .and_then(|tree| read_path_from_tree(repo, tree, path)) }, |path| read_path_from_workdir(repo, path), ); - Ok(files) + Ok((files, jobs)) } /// Get the staged diff (index vs HEAD) @@ -43,27 +42,25 @@ pub fn get_working_tree_diff( pub fn get_staged_diff( repo: &Repository, whitespace_mode: DiffWhitespaceMode, - highlighter: &SyntaxHighlighter, -) -> Result> { +) -> Result { let head = repo.head().ok().and_then(|h| h.peel_to_tree().ok()); let index = repo.index()?; let mut opts = diff_options(whitespace_mode); let diff = repo.diff_tree_to_index(head.as_ref(), Some(&index), Some(&mut opts))?; - let mut files = parse_diff(&diff, highlighter)?; - enhance_with_full_file_highlight( - &mut files, - highlighter, + let (files, mut jobs) = parse_diff(&diff)?; + append_container_full_file_jobs( + &files, + &mut jobs, |path| { head.as_ref() .and_then(|tree| read_path_from_tree(repo, tree, path)) }, |path| read_path_from_index(repo, &index, path), ); - Ok(files) + Ok((files, jobs)) } -/// Get the unstaged diff (working tree vs index) /// List the changed paths for either the staged or unstaged diff, without /// materializing hunks or running the syntax highlighter. Lets callers verify /// `get_change_status` against ignore rules cheaply — full diff parsing only @@ -103,11 +100,11 @@ pub fn list_changed_paths(repo: &Repository, kind: ChangeKind) -> Result Result> { +) -> Result { let index = repo.index()?; let mut opts = diff_options(whitespace_mode); opts.include_untracked(true); @@ -115,14 +112,14 @@ pub fn get_unstaged_diff( opts.recurse_untracked_dirs(true); let diff = repo.diff_index_to_workdir(Some(&index), Some(&mut opts))?; - let mut files = parse_diff(&diff, highlighter)?; - enhance_with_full_file_highlight( - &mut files, - highlighter, + let (files, mut jobs) = parse_diff(&diff)?; + append_container_full_file_jobs( + &files, + &mut jobs, |path| read_path_from_index(repo, &index, path), |path| read_path_from_workdir(repo, path), ); - Ok(files) + Ok((files, jobs)) } /// Get the diff for a range of commits. @@ -132,8 +129,7 @@ pub fn get_commit_range_diff( repo: &Repository, revision_range: &ResolvedRevisionRange<'_>, whitespace_mode: DiffWhitespaceMode, - highlighter: &SyntaxHighlighter, -) -> Result> { +) -> Result { let (old_tree, new_tree) = match &revision_range.diff_target { RevisionDiffTarget::CommitList => { commit_list_range_trees(repo, &revision_range.commit_ids)? @@ -148,7 +144,7 @@ pub fn get_commit_range_diff( } }; - diff_commit_trees(repo, old_tree, new_tree, whitespace_mode, highlighter) + diff_commit_trees(repo, old_tree, new_tree, whitespace_mode) } fn commit_list_range_trees<'repo>( @@ -186,15 +182,14 @@ fn diff_commit_trees( old_tree: Option>, new_tree: git2::Tree<'_>, whitespace_mode: DiffWhitespaceMode, - highlighter: &SyntaxHighlighter, -) -> Result> { +) -> Result { let mut opts = diff_options(whitespace_mode); let diff = repo.diff_tree_to_tree(old_tree.as_ref(), Some(&new_tree), Some(&mut opts))?; - let mut files = parse_diff(&diff, highlighter)?; - enhance_with_full_file_highlight( - &mut files, - highlighter, + let (files, mut jobs) = parse_diff(&diff)?; + append_container_full_file_jobs( + &files, + &mut jobs, |path| { old_tree .as_ref() @@ -202,7 +197,7 @@ fn diff_commit_trees( }, |path| read_path_from_tree(repo, &new_tree, path), ); - Ok(files) + Ok((files, jobs)) } /// Get a combined diff from the parent of the oldest commit through to the working tree. @@ -211,8 +206,7 @@ pub fn get_working_tree_with_commits_diff( repo: &Repository, commit_ids: &[String], whitespace_mode: DiffWhitespaceMode, - highlighter: &SyntaxHighlighter, -) -> Result> { +) -> Result { if commit_ids.is_empty() { return Err(TuicrError::NoChanges); } @@ -232,10 +226,10 @@ pub fn get_working_tree_with_commits_diff( opts.recurse_untracked_dirs(true); let diff = repo.diff_tree_to_workdir_with_index(old_tree.as_ref(), Some(&mut opts))?; - let mut files = parse_diff(&diff, highlighter)?; - enhance_with_full_file_highlight( - &mut files, - highlighter, + let (files, mut jobs) = parse_diff(&diff)?; + append_container_full_file_jobs( + &files, + &mut jobs, |path| { old_tree .as_ref() @@ -243,7 +237,7 @@ pub fn get_working_tree_with_commits_diff( }, |path| read_path_from_workdir(repo, path), ); - Ok(files) + Ok((files, jobs)) } fn diff_options(whitespace_mode: DiffWhitespaceMode) -> DiffOptions { @@ -268,11 +262,12 @@ fn read_path_from_index(repo: &Repository, index: &git2::Index, path: &Path) -> Some(String::from_utf8_lossy(blob.content()).into_owned()) } -fn parse_diff(diff: &Diff, highlighter: &SyntaxHighlighter) -> Result> { +fn parse_diff(diff: &Diff) -> Result { let mut files: Vec = Vec::new(); + let mut jobs: Vec = Vec::new(); // Untracked files larger than this are shown in the file list but their - // content is not parsed — they are likely logs, dumps, or build artefacts. + // content is not parsed - they are likely logs, dumps, or build artefacts. const MAX_UNTRACKED_FILE_SIZE: u64 = 10 * 1_024 * 1_024; for (delta_idx, delta) in diff.deltas().enumerate() { @@ -292,10 +287,11 @@ fn parse_diff(diff: &Diff, highlighter: &SyntaxHighlighter) -> Result MAX_UNTRACKED_FILE_SIZE; let syntax_path = new_path.as_ref().or(old_path.as_ref()).map(|p| p.as_path()); + let file_idx = files.len(); let hunks = if is_binary || is_too_large { Vec::new() } else { - parse_hunks(diff, delta_idx, highlighter, syntax_path)? + parse_hunks(diff, delta_idx, file_idx, syntax_path, &mut jobs)? }; let content_hash = DiffFile::compute_content_hash(&hunks); @@ -315,14 +311,15 @@ fn parse_diff(diff: &Diff, highlighter: &SyntaxHighlighter) -> Result, + jobs: &mut Vec, ) -> Result> { let mut hunks: Vec = Vec::new(); @@ -360,37 +357,38 @@ fn parse_hunks( line_numbers.push((line.old_lineno(), line.new_lineno())); } - let sequences = - SyntaxHighlighter::split_diff_lines_for_highlighting(&line_contents, &line_origins); - // Container grammars skip per-hunk highlighting; the full-file - // post-pass overwrites these spans anyway. - let (old_highlighted, new_highlighted) = match file_path { - Some(path) if !needs_full_file_highlight(path) => ( - highlighter.highlight_file_lines(path, &sequences.old_lines), - highlighter.highlight_file_lines(path, &sequences.new_lines), - ), - _ => (None, None), - }; - let mut lines: Vec = Vec::with_capacity(line_contents.len()); - for (idx, content) in line_contents.into_iter().enumerate() { - let origin = line_origins[idx]; + for (idx, content) in line_contents.iter().enumerate() { let (old_lineno, new_lineno) = line_numbers[idx]; - - let highlighted_spans = highlighter.highlighted_line_for_diff_with_background( - old_highlighted.as_deref(), - new_highlighted.as_deref(), - sequences.old_line_indices[idx], - sequences.new_line_indices[idx], - origin, - ); - lines.push(DiffLine { - origin, - content, + origin: line_origins[idx], + content: content.clone(), old_lineno, new_lineno, - highlighted_spans, + highlighted_spans: None, + }); + } + + // Container grammars skip per-hunk highlighting; the full-file + // post-pass will fill spans for the whole file in one shot. + if let Some(path) = file_path + && !needs_full_file_highlight(path) + { + let sequences = SyntaxHighlighter::split_diff_lines_for_highlighting( + &line_contents, + &line_origins, + ); + jobs.push(HighlightJob { + file_idx, + syntax_path: path.to_path_buf(), + kind: HighlightJobKind::Hunk { + hunk_idx, + old_lines: sequences.old_lines, + new_lines: sequences.new_lines, + old_line_indices: sequences.old_line_indices, + new_line_indices: sequences.new_line_indices, + line_origins, + }, }); } @@ -440,9 +438,8 @@ mod tests { let diff = repo .diff_tree_to_tree(Some(&head), Some(&head), None) .unwrap(); - let highlighter = SyntaxHighlighter::default(); - let result = parse_diff(&diff, &highlighter); + let result = parse_diff(&diff); assert!(matches!(result, Err(TuicrError::NoChanges))); } @@ -464,12 +461,8 @@ mod tests { ) .expect("failed to update file"); - let files = get_working_tree_diff( - &repo, - DiffWhitespaceMode::Normal, - &SyntaxHighlighter::default(), - ) - .expect("failed to get diff"); + let (files, _jobs) = + get_working_tree_diff(&repo, DiffWhitespaceMode::Normal).expect("failed to get diff"); assert_eq!(files.len(), 1); let lines = &files[0].hunks[0].lines; @@ -492,14 +485,17 @@ mod tests { let edited = "\n\n\n"; fs::write(temp_dir.path().join("App.vue"), edited).expect("failed to update file"); - let files = get_working_tree_diff( - &repo, - DiffWhitespaceMode::Normal, - &SyntaxHighlighter::default(), - ) - .expect("failed to get diff"); + let (mut files, jobs) = + get_working_tree_diff(&repo, DiffWhitespaceMode::Normal).expect("failed to get diff"); assert_eq!(files.len(), 1); + // Drive the streaming highlight pipeline synchronously so we can + // assert on the populated spans. + let highlighter = SyntaxHighlighter::default(); + for update in crate::syntax::streaming::run_blocking(jobs, &highlighter) { + crate::syntax::streaming::apply_update(&mut files, &highlighter, update); + } + let changed_lines: Vec<_> = files[0].hunks[0] .lines .iter() @@ -530,13 +526,11 @@ mod tests { fs::write(temp_dir.path().join("file.txt"), "unstaged\n").expect("failed to update file"); - let highlighter = SyntaxHighlighter::default(); - - let unstaged = get_unstaged_diff(&repo, DiffWhitespaceMode::Normal, &highlighter) - .expect("unstaged diff failed"); - assert_eq!(unstaged.len(), 1); + let (unstaged_files, _) = + get_unstaged_diff(&repo, DiffWhitespaceMode::Normal).expect("unstaged diff failed"); + assert_eq!(unstaged_files.len(), 1); assert!(matches!( - get_staged_diff(&repo, DiffWhitespaceMode::Normal, &highlighter), + get_staged_diff(&repo, DiffWhitespaceMode::Normal), Err(TuicrError::NoChanges) )); @@ -546,11 +540,11 @@ mod tests { .expect("failed to add file to index"); index.write().expect("failed to write index"); - let staged = get_staged_diff(&repo, DiffWhitespaceMode::Normal, &highlighter) - .expect("staged diff failed"); - assert_eq!(staged.len(), 1); + let (staged_files, _) = + get_staged_diff(&repo, DiffWhitespaceMode::Normal).expect("staged diff failed"); + assert_eq!(staged_files.len(), 1); assert!(matches!( - get_unstaged_diff(&repo, DiffWhitespaceMode::Normal, &highlighter), + get_unstaged_diff(&repo, DiffWhitespaceMode::Normal), Err(TuicrError::NoChanges) )); } @@ -567,12 +561,8 @@ mod tests { index.write().expect("write index"); // when - let files = get_working_tree_diff( - &repo, - DiffWhitespaceMode::Normal, - &SyntaxHighlighter::default(), - ) - .expect("unborn HEAD should produce a diff against an empty tree"); + let (files, _jobs) = get_working_tree_diff(&repo, DiffWhitespaceMode::Normal) + .expect("unborn HEAD should produce a diff against an empty tree"); // then the staged file shows up as an addition rather than crashing // with `reference 'refs/heads/main' not found` @@ -590,24 +580,16 @@ mod tests { fs::write(temp_dir.path().join("file.txt"), " alpha \n beta\n") .expect("failed to update file"); - let files = get_working_tree_diff( - &repo, - DiffWhitespaceMode::IgnoreAll, - &SyntaxHighlighter::default(), - ) - .expect("whitespace-only edit may surface as a no-op diff file"); + let (files, _) = get_working_tree_diff(&repo, DiffWhitespaceMode::IgnoreAll) + .expect("whitespace-only edit may surface as a no-op diff file"); assert_eq!(files.len(), 1); assert!(files[0].hunks.is_empty()); fs::write(temp_dir.path().join("file.txt"), " alpha \ngamma\n") .expect("failed to update file"); - let files = get_working_tree_diff( - &repo, - DiffWhitespaceMode::IgnoreAll, - &SyntaxHighlighter::default(), - ) - .expect("non-whitespace edit should still produce a diff"); + let (files, _) = get_working_tree_diff(&repo, DiffWhitespaceMode::IgnoreAll) + .expect("non-whitespace edit should still produce a diff"); assert_eq!(files.len(), 1); } @@ -627,12 +609,8 @@ mod tests { permissions.set_mode(0o755); fs::set_permissions(path, permissions).expect("failed to update mode"); - let files = get_working_tree_diff( - &repo, - DiffWhitespaceMode::IgnoreAll, - &SyntaxHighlighter::default(), - ) - .expect("mode-only edit should still produce a diff"); + let (files, _) = get_working_tree_diff(&repo, DiffWhitespaceMode::IgnoreAll) + .expect("mode-only edit should still produce a diff"); assert_eq!(files.len(), 1); assert!(files[0].hunks.is_empty()); } diff --git a/src/vcs/git/libgit2.rs b/src/vcs/git/libgit2.rs index 6c72791b..bb2e7e74 100644 --- a/src/vcs/git/libgit2.rs +++ b/src/vcs/git/libgit2.rs @@ -3,12 +3,12 @@ use std::path::{Path, PathBuf}; use std::sync::Once; use crate::error::{Result, TuicrError}; -use crate::model::{DiffFile, DiffLine, FileStatus}; -use crate::syntax::SyntaxHighlighter; +use crate::model::{DiffLine, FileStatus}; use super::{context, diff, repository, staging}; use crate::vcs::traits::{ - ChangeKind, CommitInfo, DiffWhitespaceMode, ResolvedRevisionRange, VcsBackend, VcsInfo, VcsType, + ChangeKind, CommitInfo, DiffWhitespaceMode, DiffWithJobs, ResolvedRevisionRange, VcsBackend, + VcsInfo, VcsType, }; /// Git backend implementation using the git2/libgit2 library. @@ -99,16 +99,16 @@ impl VcsBackend for Libgit2Backend { false } - fn get_working_tree_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { - diff::get_working_tree_diff(&self.repo, self.whitespace_mode, highlighter) + fn get_working_tree_diff(&self) -> Result { + diff::get_working_tree_diff(&self.repo, self.whitespace_mode) } - fn get_staged_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { - diff::get_staged_diff(&self.repo, self.whitespace_mode, highlighter) + fn get_staged_diff(&self) -> Result { + diff::get_staged_diff(&self.repo, self.whitespace_mode) } - fn get_unstaged_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { - diff::get_unstaged_diff(&self.repo, self.whitespace_mode, highlighter) + fn get_unstaged_diff(&self) -> Result { + diff::get_unstaged_diff(&self.repo, self.whitespace_mode) } fn list_changed_paths(&self, kind: ChangeKind) -> Result> { @@ -165,14 +165,8 @@ impl VcsBackend for Libgit2Backend { fn get_commit_range_diff( &self, revision_range: &ResolvedRevisionRange<'_>, - highlighter: &SyntaxHighlighter, - ) -> Result> { - diff::get_commit_range_diff( - &self.repo, - revision_range, - self.whitespace_mode, - highlighter, - ) + ) -> Result { + diff::get_commit_range_diff(&self.repo, revision_range, self.whitespace_mode) } fn get_commits_info(&self, ids: &[String]) -> Result> { @@ -191,17 +185,8 @@ impl VcsBackend for Libgit2Backend { .collect()) } - fn get_working_tree_with_commits_diff( - &self, - commit_ids: &[String], - highlighter: &SyntaxHighlighter, - ) -> Result> { - diff::get_working_tree_with_commits_diff( - &self.repo, - commit_ids, - self.whitespace_mode, - highlighter, - ) + fn get_working_tree_with_commits_diff(&self, commit_ids: &[String]) -> Result { + diff::get_working_tree_with_commits_diff(&self.repo, commit_ids, self.whitespace_mode) } fn stage_file(&self, path: &Path) -> Result<()> { diff --git a/src/vcs/git/mod.rs b/src/vcs/git/mod.rs index 1ee25aa8..defd83b9 100644 --- a/src/vcs/git/mod.rs +++ b/src/vcs/git/mod.rs @@ -9,13 +9,12 @@ use std::ffi::OsStr; use std::path::{Path, PathBuf}; use crate::error::{Result, TuicrError}; -use crate::model::{DiffFile, DiffLine, FileStatus}; +use crate::model::{DiffLine, FileStatus}; use crate::process::{CommandOutputError, CommandOutputErrorKind, run_command_output}; -use crate::syntax::SyntaxHighlighter; use super::traits::{ - ChangeKind, CommitInfo, DiffWhitespaceMode, ResolvedRevisionRange, VcsBackend, VcsChangeStatus, - VcsInfo, + ChangeKind, CommitInfo, DiffWhitespaceMode, DiffWithJobs, ResolvedRevisionRange, VcsBackend, + VcsChangeStatus, VcsInfo, }; use cli::GitCliBackend; pub use libgit2::Libgit2Backend; @@ -248,24 +247,24 @@ impl VcsBackend for GitBackend { } } - fn get_working_tree_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { match self { - Self::Libgit2(backend) => backend.get_working_tree_diff(highlighter), - Self::Cli(backend) => backend.get_working_tree_diff(highlighter), + Self::Libgit2(backend) => backend.get_working_tree_diff(), + Self::Cli(backend) => backend.get_working_tree_diff(), } } - fn get_staged_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { + fn get_staged_diff(&self) -> Result { match self { - Self::Libgit2(backend) => backend.get_staged_diff(highlighter), - Self::Cli(backend) => backend.get_staged_diff(highlighter), + Self::Libgit2(backend) => backend.get_staged_diff(), + Self::Cli(backend) => backend.get_staged_diff(), } } - fn get_unstaged_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { + fn get_unstaged_diff(&self) -> Result { match self { - Self::Libgit2(backend) => backend.get_unstaged_diff(highlighter), - Self::Cli(backend) => backend.get_unstaged_diff(highlighter), + Self::Libgit2(backend) => backend.get_unstaged_diff(), + Self::Cli(backend) => backend.get_unstaged_diff(), } } @@ -338,11 +337,10 @@ impl VcsBackend for GitBackend { fn get_commit_range_diff( &self, revision_range: &ResolvedRevisionRange<'_>, - highlighter: &SyntaxHighlighter, - ) -> Result> { + ) -> Result { match self { - Self::Libgit2(backend) => backend.get_commit_range_diff(revision_range, highlighter), - Self::Cli(backend) => backend.get_commit_range_diff(revision_range, highlighter), + Self::Libgit2(backend) => backend.get_commit_range_diff(revision_range), + Self::Cli(backend) => backend.get_commit_range_diff(revision_range), } } @@ -353,18 +351,10 @@ impl VcsBackend for GitBackend { } } - fn get_working_tree_with_commits_diff( - &self, - commit_ids: &[String], - highlighter: &SyntaxHighlighter, - ) -> Result> { + fn get_working_tree_with_commits_diff(&self, commit_ids: &[String]) -> Result { match self { - Self::Libgit2(backend) => { - backend.get_working_tree_with_commits_diff(commit_ids, highlighter) - } - Self::Cli(backend) => { - backend.get_working_tree_with_commits_diff(commit_ids, highlighter) - } + Self::Libgit2(backend) => backend.get_working_tree_with_commits_diff(commit_ids), + Self::Cli(backend) => backend.get_working_tree_with_commits_diff(commit_ids), } } diff --git a/src/vcs/hg/mod.rs b/src/vcs/hg/mod.rs index c2a02a81..fa547b00 100644 --- a/src/vcs/hg/mod.rs +++ b/src/vcs/hg/mod.rs @@ -6,14 +6,13 @@ use std::process::Command; use chrono::{TimeZone, Utc}; use crate::error::{Result, TuicrError}; -use crate::model::{DiffFile, DiffLine, FileStatus, LineOrigin}; -use crate::syntax::SyntaxHighlighter; +use crate::model::{DiffLine, FileStatus, LineOrigin}; use crate::vcs::diff_parser::{self, DiffFormat}; use crate::vcs::traits::{ - CommitInfo, DiffWhitespaceMode, ResolvedRevisionRange, RevisionDiffTarget, VcsBackend, VcsInfo, - VcsType, + CommitInfo, DiffWhitespaceMode, DiffWithJobs, ResolvedRevisionRange, RevisionDiffTarget, + VcsBackend, VcsInfo, VcsType, }; -use crate::vcs::{BATCH_BOUNDARY, apply_container_full_file_highlight, parse_batched_files}; +use crate::vcs::{BATCH_BOUNDARY, append_container_full_file_jobs_from_rev, parse_batched_files}; /// Parse an hg description into (summary, optional body). fn parse_hg_description(desc: &str) -> (String, Option) { @@ -101,7 +100,7 @@ impl VcsBackend for HgBackend { &self.info } - fn get_working_tree_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { let args = self.diff_args(&["diff"]); let diff_output = run_hg_command(&self.info.root_path, args.iter().copied())?; @@ -109,16 +108,16 @@ impl VcsBackend for HgBackend { return Err(TuicrError::NoChanges); } - let mut files = diff_parser::parse_unified_diff(&diff_output, DiffFormat::Hg, highlighter)?; - apply_container_full_file_highlight( + let (files, mut jobs) = diff_parser::parse_unified_diff(&diff_output, DiffFormat::Hg)?; + append_container_full_file_jobs_from_rev( &self.info.root_path, ".", None, - &mut files, - highlighter, + &files, + &mut jobs, hg_cat_batch, )?; - Ok(files) + Ok((files, jobs)) } fn fetch_context_lines( @@ -267,8 +266,7 @@ impl VcsBackend for HgBackend { fn get_commit_range_diff( &self, revision_range: &ResolvedRevisionRange<'_>, - highlighter: &SyntaxHighlighter, - ) -> Result> { + ) -> Result { let commit_ids = &revision_range.commit_ids; if commit_ids.is_empty() { return Err(TuicrError::NoChanges); @@ -321,16 +319,16 @@ impl VcsBackend for HgBackend { return Err(TuicrError::NoChanges); } - let mut files = diff_parser::parse_unified_diff(&diff_output, DiffFormat::Hg, highlighter)?; - apply_container_full_file_highlight( + let (files, mut jobs) = diff_parser::parse_unified_diff(&diff_output, DiffFormat::Hg)?; + append_container_full_file_jobs_from_rev( &self.info.root_path, &from_rev, Some(newest_short), - &mut files, - highlighter, + &files, + &mut jobs, hg_cat_batch, )?; - Ok(files) + Ok((files, jobs)) } fn get_commits_info(&self, ids: &[String]) -> Result> { @@ -394,11 +392,7 @@ impl VcsBackend for HgBackend { Ok(ids.iter().filter_map(|id| by_id.remove(id)).collect()) } - fn get_working_tree_with_commits_diff( - &self, - commit_ids: &[String], - highlighter: &SyntaxHighlighter, - ) -> Result> { + fn get_working_tree_with_commits_diff(&self, commit_ids: &[String]) -> Result { if commit_ids.is_empty() { return Err(TuicrError::NoChanges); } @@ -436,16 +430,16 @@ impl VcsBackend for HgBackend { return Err(TuicrError::NoChanges); } - let mut files = diff_parser::parse_unified_diff(&diff_output, DiffFormat::Hg, highlighter)?; - apply_container_full_file_highlight( + let (files, mut jobs) = diff_parser::parse_unified_diff(&diff_output, DiffFormat::Hg)?; + append_container_full_file_jobs_from_rev( &self.info.root_path, &from_rev, None, - &mut files, - highlighter, + &files, + &mut jobs, hg_cat_batch, )?; - Ok(files) + Ok((files, jobs)) } } @@ -604,9 +598,7 @@ mod tests { assert_eq!(backend.info().root_path, expected_path); assert_eq!(backend.info().vcs_type, VcsType::Mercurial); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (files, _jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); assert_eq!(files.len(), 1); assert_eq!( @@ -630,14 +622,14 @@ mod tests { .expect("Failed to create hg backend"); assert!(matches!( - backend.get_working_tree_diff(&SyntaxHighlighter::default()), + backend.get_working_tree_diff(), Err(TuicrError::NoChanges) )); fs::write(temp.path().join("hello.txt"), " hello ship \n") .expect("Failed to write non-whitespace edit"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) + let (files, _) = backend + .get_working_tree_diff() .expect("non-whitespace edit should still produce a diff"); assert_eq!(files.len(), 1); } @@ -674,20 +666,14 @@ mod tests { .expect("Expected whitespace commit"); assert!(matches!( - backend.get_working_tree_with_commits_diff( - std::slice::from_ref(&whitespace_commit.id), - &SyntaxHighlighter::default() - ), + backend.get_working_tree_with_commits_diff(std::slice::from_ref(&whitespace_commit.id)), Err(TuicrError::NoChanges) )); fs::write(temp.path().join("hello.txt"), " hello ship \n") .expect("Failed to write non-whitespace edit"); - let files = backend - .get_working_tree_with_commits_diff( - std::slice::from_ref(&whitespace_commit.id), - &SyntaxHighlighter::default(), - ) + let (files, _) = backend + .get_working_tree_with_commits_diff(std::slice::from_ref(&whitespace_commit.id)) .expect("non-whitespace edit should still produce a diff"); assert_eq!(files.len(), 1); } @@ -816,17 +802,15 @@ mod tests { // Get diff for the last two commits (Second and Third) let commit_ids = vec![commits[1].id.clone(), commits[0].id.clone()]; - let diff_result = backend.get_commit_range_diff( - &ResolvedRevisionRange::from_owned_commit_ids( + let diff_result = + backend.get_commit_range_diff(&ResolvedRevisionRange::from_owned_commit_ids( commit_ids, RevisionDiffTarget::CommitList, - ), - &SyntaxHighlighter::default(), - ); + )); // Note: Sapling (Meta's hg fork) may fail with "id_dag_snapshot()" error // in certain temporary directory configurations. Skip the test in that case. - let diff = match diff_result { + let (diff, _jobs) = match diff_result { Ok(d) => d, Err(TuicrError::VcsCommand(msg)) if msg.contains("id_dag_snapshot") => { eprintln!("Skipping test: Sapling-specific issue with tempdir repos"); @@ -906,9 +890,7 @@ mod tests { let backend = HgBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::Normal) .expect("Failed to create hg backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (files, _jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); // hg should show the rename assert!(!files.is_empty(), "Expected at least one file change"); @@ -979,9 +961,7 @@ mod tests { let backend = HgBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::Normal) .expect("Failed to create hg backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (files, _jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); assert!(!files.is_empty(), "Expected at least one file change"); @@ -1039,9 +1019,7 @@ mod tests { let backend = HgBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::Normal) .expect("Failed to create hg backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (files, _jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); assert_eq!(files.len(), 1, "Expected one file"); @@ -1095,11 +1073,14 @@ mod tests { let backend = HgBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::Normal) .expect("Failed to create hg backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (mut files, jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); assert_eq!(files.len(), 1); + let highlighter = crate::syntax::SyntaxHighlighter::default(); + for update in crate::syntax::streaming::run_blocking(jobs, &highlighter) { + crate::syntax::streaming::apply_update(&mut files, &highlighter, update); + } + let changed_lines: Vec<_> = files[0].hunks[0] .lines .iter() @@ -1147,9 +1128,7 @@ mod tests { let backend = HgBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::Normal) .expect("Failed to create hg backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (files, _jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); assert_eq!(files.len(), 1, "Expected one file"); diff --git a/src/vcs/jj/mod.rs b/src/vcs/jj/mod.rs index bd879136..4b470a8b 100644 --- a/src/vcs/jj/mod.rs +++ b/src/vcs/jj/mod.rs @@ -8,14 +8,13 @@ use std::process::Command; use chrono::{DateTime, Utc}; use crate::error::{Result, TuicrError}; -use crate::model::{DiffFile, DiffLine, FileStatus, LineOrigin}; -use crate::syntax::SyntaxHighlighter; +use crate::model::{DiffLine, FileStatus, LineOrigin}; use crate::vcs::diff_parser::{self, DiffFormat}; use crate::vcs::traits::{ - CommitInfo, DiffWhitespaceMode, ResolvedRevisionRange, RevisionDiffTarget, VcsBackend, VcsInfo, - VcsType, + CommitInfo, DiffWhitespaceMode, DiffWithJobs, ResolvedRevisionRange, RevisionDiffTarget, + VcsBackend, VcsInfo, VcsType, }; -use crate::vcs::{BATCH_BOUNDARY, apply_container_full_file_highlight, parse_batched_files}; +use crate::vcs::{BATCH_BOUNDARY, append_container_full_file_jobs_from_rev, parse_batched_files}; /// Parse a jj description into (summary, optional body). fn parse_description(desc: &str) -> (String, Option) { @@ -138,7 +137,7 @@ impl VcsBackend for JjBackend { &self.info } - fn get_working_tree_diff(&self, highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { let args = self.diff_args(&["diff", "--git"]); let diff_output = run_jj_command(&self.info.root_path, args.iter().copied())?; @@ -146,17 +145,17 @@ impl VcsBackend for JjBackend { return Err(TuicrError::NoChanges); } - let mut files = - diff_parser::parse_unified_diff(&diff_output, DiffFormat::GitStyle, highlighter)?; - apply_container_full_file_highlight( + let (files, mut jobs) = + diff_parser::parse_unified_diff(&diff_output, DiffFormat::GitStyle)?; + append_container_full_file_jobs_from_rev( &self.info.root_path, "@-", None, - &mut files, - highlighter, + &files, + &mut jobs, jj_show_batch, )?; - Ok(files) + Ok((files, jobs)) } fn fetch_context_lines( @@ -324,8 +323,7 @@ impl VcsBackend for JjBackend { fn get_commit_range_diff( &self, revision_range: &ResolvedRevisionRange<'_>, - highlighter: &SyntaxHighlighter, - ) -> Result> { + ) -> Result { let commit_ids = &revision_range.commit_ids; if commit_ids.is_empty() { return Err(TuicrError::NoChanges); @@ -346,17 +344,17 @@ impl VcsBackend for JjBackend { return Err(TuicrError::NoChanges); } - let mut files = - diff_parser::parse_unified_diff(&diff_output, DiffFormat::GitStyle, highlighter)?; - apply_container_full_file_highlight( + let (files, mut jobs) = + diff_parser::parse_unified_diff(&diff_output, DiffFormat::GitStyle)?; + append_container_full_file_jobs_from_rev( &self.info.root_path, &from_rev, Some(newest), - &mut files, - highlighter, + &files, + &mut jobs, jj_show_batch, )?; - Ok(files) + Ok((files, jobs)) } fn get_commits_info(&self, ids: &[String]) -> Result> { @@ -410,11 +408,7 @@ impl VcsBackend for JjBackend { Ok(ids.iter().filter_map(|id| by_id.remove(id)).collect()) } - fn get_working_tree_with_commits_diff( - &self, - commit_ids: &[String], - highlighter: &SyntaxHighlighter, - ) -> Result> { + fn get_working_tree_with_commits_diff(&self, commit_ids: &[String]) -> Result { if commit_ids.is_empty() { return Err(TuicrError::NoChanges); } @@ -432,17 +426,17 @@ impl VcsBackend for JjBackend { return Err(TuicrError::NoChanges); } - let mut files = - diff_parser::parse_unified_diff(&diff_output, DiffFormat::GitStyle, highlighter)?; - apply_container_full_file_highlight( + let (files, mut jobs) = + diff_parser::parse_unified_diff(&diff_output, DiffFormat::GitStyle)?; + append_container_full_file_jobs_from_rev( &self.info.root_path, &from_rev, None, - &mut files, - highlighter, + &files, + &mut jobs, jj_show_batch, )?; - Ok(files) + Ok((files, jobs)) } } @@ -601,9 +595,7 @@ mod tests { assert_eq!(backend.info().root_path, expected_path); assert_eq!(backend.info().vcs_type, VcsType::Jujutsu); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (files, _jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); assert_eq!(files.len(), 1); assert_eq!( @@ -626,16 +618,16 @@ mod tests { JjBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::IgnoreAll) .expect("Failed to create jj backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) + let (files, _) = backend + .get_working_tree_diff() .expect("whitespace-only edit may surface as a no-op diff file"); assert_eq!(files.len(), 1); assert!(files[0].hunks.is_empty()); fs::write(temp.path().join("hello.txt"), " hello ship \n") .expect("Failed to write non-whitespace edit"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) + let (files, _) = backend + .get_working_tree_diff() .expect("non-whitespace edit should still produce a diff"); assert_eq!(files.len(), 1); } @@ -671,22 +663,16 @@ mod tests { .find(|commit| commit.summary == "Whitespace commit") .expect("Expected whitespace commit"); - let files = backend - .get_working_tree_with_commits_diff( - std::slice::from_ref(&whitespace_commit.id), - &SyntaxHighlighter::default(), - ) + let (files, _) = backend + .get_working_tree_with_commits_diff(std::slice::from_ref(&whitespace_commit.id)) .expect("whitespace-only edit may surface as a no-op diff file"); assert_eq!(files.len(), 1); assert!(files[0].hunks.is_empty()); fs::write(temp.path().join("hello.txt"), " hello ship \n") .expect("Failed to write non-whitespace edit"); - let files = backend - .get_working_tree_with_commits_diff( - std::slice::from_ref(&whitespace_commit.id), - &SyntaxHighlighter::default(), - ) + let (files, _) = backend + .get_working_tree_with_commits_diff(std::slice::from_ref(&whitespace_commit.id)) .expect("non-whitespace edit should still produce a diff"); assert_eq!(files.len(), 1); } @@ -841,14 +827,11 @@ mod tests { let newest = &named_commits[0]; // Third commit let commit_ids = vec![oldest.id.clone(), newest.id.clone()]; - let diff = backend - .get_commit_range_diff( - &ResolvedRevisionRange::from_owned_commit_ids( - commit_ids, - RevisionDiffTarget::CommitList, - ), - &SyntaxHighlighter::default(), - ) + let (diff, _jobs) = backend + .get_commit_range_diff(&ResolvedRevisionRange::from_owned_commit_ids( + commit_ids, + RevisionDiffTarget::CommitList, + )) .expect("Failed to get commit range diff"); // Should have changes @@ -901,9 +884,7 @@ mod tests { let backend = JjBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::Normal) .expect("Failed to create jj backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (files, _jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); // jj should detect the rename // Note: jj may show this as delete + add if it doesn't detect the rename @@ -952,9 +933,7 @@ mod tests { let backend = JjBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::Normal) .expect("Failed to create jj backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (files, _jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); assert_eq!(files.len(), 1, "Expected one file"); @@ -987,9 +966,7 @@ mod tests { let backend = JjBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::Normal) .expect("Failed to create jj backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (files, _jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); assert_eq!(files.len(), 1, "Expected one file"); @@ -1080,11 +1057,14 @@ mod tests { let backend = JjBackend::from_path(temp.path().to_path_buf(), DiffWhitespaceMode::Normal) .expect("Failed to create jj backend"); - let files = backend - .get_working_tree_diff(&SyntaxHighlighter::default()) - .expect("Failed to get diff"); + let (mut files, jobs) = backend.get_working_tree_diff().expect("Failed to get diff"); assert_eq!(files.len(), 1); + let highlighter = crate::syntax::SyntaxHighlighter::default(); + for update in crate::syntax::streaming::run_blocking(jobs, &highlighter) { + crate::syntax::streaming::apply_update(&mut files, &highlighter, update); + } + let changed_lines: Vec<_> = files[0].hunks[0] .lines .iter() diff --git a/src/vcs/mod.rs b/src/vcs/mod.rs index 3ed2b433..d951d06a 100644 --- a/src/vcs/mod.rs +++ b/src/vcs/mod.rs @@ -26,8 +26,8 @@ pub use hg::HgBackend; pub use jj::JjBackend; pub use pr_noop::PrNoopVcs; pub use traits::{ - ChangeKind, CommitInfo, DiffWhitespaceMode, ResolvedRevisionRange, RevisionDiffTarget, - VcsBackend, VcsChangeStatus, VcsInfo, + ChangeKind, CommitInfo, DiffWhitespaceMode, DiffWithJobs, ResolvedRevisionRange, + RevisionDiffTarget, VcsBackend, VcsChangeStatus, VcsInfo, }; use std::collections::HashMap; @@ -35,9 +35,7 @@ use std::path::{Path, PathBuf}; use crate::error::{Result, TuicrError}; use crate::model::{DiffFile, LineSide}; -use crate::syntax::{ - HighlightedLines, HighlightedSpans, SyntaxHighlighter, needs_full_file_highlight, -}; +use crate::syntax::{HighlightJob, HighlightJobKind, needs_full_file_highlight}; /// Boundary marker emitted between files in batched `hg cat` / `jj file show` /// output. The long random suffix makes accidental collision with real source @@ -75,6 +73,29 @@ pub(crate) fn read_workdir_file(root: &Path, rel: &Path) -> Option { std::fs::read_to_string(root.join(rel)).ok() } +/// Drop files for which `keep` returns false, then re-index highlight jobs to +/// match the kept files' new positions. Jobs whose file was dropped are also +/// dropped. Order of kept files (and remaining jobs) is preserved. +pub fn filter_diff_with_jobs(diff: DiffWithJobs, keep: impl Fn(&DiffFile) -> bool) -> DiffWithJobs { + let (files, mut jobs) = diff; + let mut remap: HashMap = HashMap::with_capacity(files.len()); + let mut kept: Vec = Vec::with_capacity(files.len()); + for (old_idx, file) in files.into_iter().enumerate() { + if keep(&file) { + remap.insert(old_idx, kept.len()); + kept.push(file); + } + } + jobs.retain_mut(|job| match remap.get(&job.file_idx) { + Some(&new_idx) => { + job.file_idx = new_idx; + true + } + None => false, + }); + (kept, jobs) +} + /// Parse the output of a batched `hg cat` / `jj file show` invocation whose /// template prefixed each file with `\n{BATCH_BOUNDARY}\n{path}\n` before /// emitting `{data}`. Returns a `path → data` map. @@ -92,17 +113,20 @@ pub(crate) fn parse_batched_files(output: &str) -> HashMap { .collect() } -/// Re-highlight container-grammar files (Vue, Svelte, etc) using their full -/// content at the requested revisions. `new_rev = None` reads the new side -/// from the working tree on disk instead of calling `fetch_batch`. The -/// `fetch_batch` closure is the backend-specific batched-fetch primitive -/// (`hg cat -r REV ...` or `jj file show -r REV ...`). -pub(crate) fn apply_container_full_file_highlight( +/// Batch-fetch container-grammar files from a VCS at a given revision, then +/// emit a `FullFile` highlight job for each. `new_rev = None` reads the new +/// side from disk instead of calling `fetch_batch`. The `fetch_batch` closure +/// is the backend-specific batched-fetch primitive (`hg cat -r REV ...` or +/// `jj file show -r REV ...`). +/// +/// The fetch happens on the parse thread because backends here drive +/// subprocesses; the actual highlighting runs later on the worker thread. +pub(crate) fn append_container_full_file_jobs_from_rev( root: &Path, old_rev: &str, new_rev: Option<&str>, - files: &mut [DiffFile], - highlighter: &SyntaxHighlighter, + files: &[DiffFile], + jobs: &mut Vec, fetch_batch: F, ) -> Result<()> where @@ -123,9 +147,9 @@ where let workdir = new_rev.is_none().then(|| root.to_path_buf()); - enhance_with_full_file_highlight( + append_container_full_file_jobs( files, - highlighter, + jobs, |p| old_map.get(p).cloned(), |p| match (new_map.get(p), workdir.as_deref()) { (Some(content), _) => Some(content.clone()), @@ -137,36 +161,23 @@ where Ok(()) } -/// Files larger than this skip the full-file highlight pass and fall back to -/// per-hunk highlighting. Keeps a runaway-cost ceiling on diffs that include -/// huge generated artefacts (lockfiles, vendored bundles, fixtures). -const MAX_HIGHLIGHT_FILE_BYTES: usize = 1024 * 1024; - -/// Re-highlight each diff line using full-file context, for files whose -/// grammar needs it (Vue, Svelte, Astro, MDX). Other files keep their existing -/// per-hunk highlighting unchanged. -/// -/// `fetch_old`/`fetch_new` return the entire content of the file at the old -/// and new sides respectively (or `None` if unavailable). When a side is -/// available, every diff line on that side is replaced with the span at its -/// 1-based lineno from the full-file highlight. Lines whose side could not be -/// fetched keep whatever the parser already assigned. +/// Walk `files` and append one `FullFile` highlight job per container-grammar +/// file (Vue, Svelte, ...). The closures fetch the file's full content on +/// each side; either may return `None` if that side is unavailable. /// -/// Runs in three phases: fetch (serial, since fetch closures may close over -/// `!Send` state such as `git2::Repository`), highlight (parallel via -/// `std::thread::scope`, since syntect's `SyntaxSet` and `Theme` are `Sync` -/// and each file's syntect work is independent), and apply spans (serial, -/// needs `&mut files`). -pub(crate) fn enhance_with_full_file_highlight( - files: &mut [DiffFile], - highlighter: &SyntaxHighlighter, +/// Fetching happens here, in the parse phase, because the closures may close +/// over VCS state that is `!Send` (e.g. `git2::Repository`, an `hg` child +/// process). The highlighting itself runs later on the worker thread, against +/// the `String`s embedded in the job. +pub(crate) fn append_container_full_file_jobs( + files: &[DiffFile], + jobs: &mut Vec, mut fetch_old: F, mut fetch_new: G, ) where F: FnMut(&Path) -> Option, G: FnMut(&Path) -> Option, { - let mut jobs: Vec = Vec::new(); for (idx, file) in files.iter().enumerate() { if file.is_binary || file.is_too_large || file.hunks.is_empty() { continue; @@ -185,116 +196,12 @@ pub(crate) fn enhance_with_full_file_highlight( jobs.push(HighlightJob { file_idx: idx, syntax_path: syntax_path.to_path_buf(), - old_content, - new_content, + kind: HighlightJobKind::FullFile { + old_content, + new_content, + }, }); } - - if jobs.is_empty() { - return; - } - - let results = highlight_jobs_parallel(&jobs, highlighter); - - for (idx, old, new) in results { - if old.is_none() && new.is_none() { - continue; - } - apply_full_file_spans(&mut files[idx], highlighter, old.as_deref(), new.as_deref()); - } -} - -struct HighlightJob { - file_idx: usize, - syntax_path: PathBuf, - old_content: Option, - new_content: Option, -} - -type HighlightResult = (usize, Option, Option); - -fn highlight_jobs_parallel( - jobs: &[HighlightJob], - highlighter: &SyntaxHighlighter, -) -> Vec { - let parallelism = std::thread::available_parallelism() - .map(|n| n.get()) - .unwrap_or(1) - .min(jobs.len()); - - if parallelism <= 1 { - return jobs - .iter() - .map(|j| highlight_single_job(j, highlighter)) - .collect(); - } - - let chunk_size = jobs.len().div_ceil(parallelism); - std::thread::scope(|scope| { - let handles: Vec<_> = jobs - .chunks(chunk_size) - .map(|chunk| { - scope.spawn(move || { - chunk - .iter() - .map(|j| highlight_single_job(j, highlighter)) - .collect::>() - }) - }) - .collect(); - handles - .into_iter() - .flat_map(|h| h.join().expect("highlight thread panicked")) - .collect() - }) -} - -fn highlight_single_job(job: &HighlightJob, highlighter: &SyntaxHighlighter) -> HighlightResult { - let old = job - .old_content - .as_deref() - .and_then(|c| highlight_content(highlighter, &job.syntax_path, c)); - let new = job - .new_content - .as_deref() - .and_then(|c| highlight_content(highlighter, &job.syntax_path, c)); - (job.file_idx, old, new) -} - -fn highlight_content( - highlighter: &SyntaxHighlighter, - path: &Path, - content: &str, -) -> Option { - if content.len() > MAX_HIGHLIGHT_FILE_BYTES || content.as_bytes().contains(&0u8) { - return None; - } - let lines: Vec = content.lines().map(tabify).collect(); - highlighter.highlight_file_lines(path, &lines) -} - -fn apply_full_file_spans( - file: &mut DiffFile, - highlighter: &SyntaxHighlighter, - old_highlight: Option<&[Option]>, - new_highlight: Option<&[Option]>, -) { - for hunk in &mut file.hunks { - for line in &mut hunk.lines { - let old_idx = line.old_lineno.map(|n| n.saturating_sub(1) as usize); - let new_idx = line.new_lineno.map(|n| n.saturating_sub(1) as usize); - let spans = highlighter.highlighted_line_for_diff_with_background( - old_highlight, - new_highlight, - old_idx, - new_idx, - line.origin, - ); - if spans.is_some() { - line.highlighted_spans = spans; - } - } - } } /// Detect the VCS type and return the appropriate backend. @@ -436,6 +343,7 @@ mod tests { fn highlight_n_vue_files(n: usize) -> Vec { use crate::syntax::SyntaxHighlighter; + use crate::syntax::streaming::{apply_update, run_blocking}; let mut files = Vec::with_capacity(n); let mut content_map: HashMap = HashMap::new(); @@ -447,12 +355,16 @@ mod tests { } let highlighter = SyntaxHighlighter::default(); - enhance_with_full_file_highlight( - &mut files, - &highlighter, + let mut jobs = Vec::new(); + append_container_full_file_jobs( + &files, + &mut jobs, |p| content_map.get(p).map(|(o, _)| o.clone()), |p| content_map.get(p).map(|(_, n)| n.clone()), ); + for update in run_blocking(jobs, &highlighter) { + apply_update(&mut files, &highlighter, update); + } files } @@ -477,124 +389,22 @@ mod tests { } #[test] - fn enhance_full_file_highlight_serial_path_one_file() { - // Single file takes the serial branch in highlight_jobs_parallel. + fn streaming_full_file_highlight_serial_one_file() { + // Single file takes the serial branch in the streaming worker. let files = highlight_n_vue_files(1); assert_all_lines_highlighted(&files); } #[test] - fn enhance_full_file_highlight_parallel_path_many_files() { - // 12 files exceeds typical parallelism, forcing chunked thread::scope. + fn streaming_full_file_highlight_parallel_many_files() { + // 12 files exceeds typical parallelism, forcing the scoped pool. let files = highlight_n_vue_files(12); assert_eq!(files.len(), 12); assert_all_lines_highlighted(&files); } - fn synth_vue_file(idx: usize) -> (DiffFile, String, String) { - let mut html = String::from("\n\n"); - - let mut script = - String::from("\n\n"); - - let mut style = String::from("\n"); - - let old = format!("{html}{script}{style}"); - let new = old.replace("const value0 = ref(0)", "const value0 = ref(42)"); - - let target_line = new - .lines() - .position(|l| l.starts_with("const value0 = ref(42)")) - .expect("synth content must contain target line") as u32 - + 1; - let file = vue_diff_file( - idx, - "const value0 = ref(0)", - "const value0 = ref(42)", - target_line, - ); - (file, old, new) - } - - /// Manual bench: parallel vs serial highlight at realistic scales. - /// Run with: `cargo test --release vcs::tests::bench_highlight_parallel_vs_serial -- --ignored --nocapture` - #[test] - #[ignore] - fn bench_highlight_parallel_vs_serial() { - use crate::syntax::SyntaxHighlighter; - use std::time::Instant; - - let highlighter = SyntaxHighlighter::default(); - let scales = [1usize, 5, 12, 25, 50]; - let runs = 5; - - for &n in &scales { - let mut files_template = Vec::with_capacity(n); - let mut content_map: HashMap = HashMap::new(); - for i in 0..n { - let (file, old, new) = synth_vue_file(i); - content_map.insert(file.new_path.clone().unwrap(), (old, new)); - files_template.push(file); - } - - let jobs: Vec = files_template - .iter() - .enumerate() - .map(|(idx, f)| { - let path = f.new_path.clone().unwrap(); - let (old, new) = content_map.get(&path).unwrap(); - HighlightJob { - file_idx: idx, - syntax_path: path, - old_content: Some(old.clone()), - new_content: Some(new.clone()), - } - }) - .collect(); - - // Warmup - let _ = highlight_jobs_parallel(&jobs, &highlighter); - - let mut par_total = std::time::Duration::ZERO; - for _ in 0..runs { - let t = Instant::now(); - let _ = highlight_jobs_parallel(&jobs, &highlighter); - par_total += t.elapsed(); - } - let par_mean = par_total / runs as u32; - - let mut ser_total = std::time::Duration::ZERO; - for _ in 0..runs { - let t = Instant::now(); - let _: Vec = jobs - .iter() - .map(|j| highlight_single_job(j, &highlighter)) - .collect(); - ser_total += t.elapsed(); - } - let ser_mean = ser_total / runs as u32; - - let speedup = ser_mean.as_secs_f64() / par_mean.as_secs_f64().max(1e-9); - println!( - "N={n:>3}: serial={ser_mean:>10.2?} parallel={par_mean:>10.2?} speedup={speedup:.2}x" - ); - } - } - #[test] - fn enhance_full_file_highlight_results_match_input_order() { + fn streaming_full_file_highlight_results_match_input_order() { // Each file's highlighted spans must land on that file's hunk lines, // not a neighbour's. Distinguishable by line content. let files = highlight_n_vue_files(6); diff --git a/src/vcs/pr_noop.rs b/src/vcs/pr_noop.rs index a9d8f502..ceb9b193 100644 --- a/src/vcs/pr_noop.rs +++ b/src/vcs/pr_noop.rs @@ -12,10 +12,9 @@ use std::path::Path; use crate::error::{Result, TuicrError}; -use crate::model::{DiffFile, DiffLine, FileStatus}; -use crate::syntax::SyntaxHighlighter; +use crate::model::{DiffLine, FileStatus}; -use super::traits::{VcsBackend, VcsInfo}; +use super::traits::{DiffWithJobs, VcsBackend, VcsInfo}; pub struct PrNoopVcs { info: VcsInfo, @@ -32,7 +31,7 @@ impl VcsBackend for PrNoopVcs { &self.info } - fn get_working_tree_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_working_tree_diff(&self) -> Result { Err(TuicrError::UnsupportedOperation( "PR mode does not read from the local working tree".to_string(), )) @@ -96,9 +95,7 @@ mod tests { // given let vcs = PrNoopVcs::new(info()); // when - let err = vcs - .get_working_tree_diff(&SyntaxHighlighter::default()) - .unwrap_err(); + let err = vcs.get_working_tree_diff().unwrap_err(); // then assert!( err.to_string() diff --git a/src/vcs/traits.rs b/src/vcs/traits.rs index f49e322e..362d10c3 100644 --- a/src/vcs/traits.rs +++ b/src/vcs/traits.rs @@ -4,7 +4,11 @@ use std::path::{Path, PathBuf}; use crate::error::Result; use crate::model::{DiffFile, DiffLine, FileStatus}; -use crate::syntax::SyntaxHighlighter; +use crate::syntax::HighlightJob; + +/// Result of a diff fetch: parsed files (with `highlighted_spans = None` on +/// every line) plus the highlight work the streaming worker will run. +pub type DiffWithJobs = (Vec, Vec); /// Information about the VCS type #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -174,18 +178,20 @@ pub trait VcsBackend: Send { false } - /// Get the working tree diff (staged + unstaged changes) - fn get_working_tree_diff(&self, highlighter: &SyntaxHighlighter) -> Result>; + /// Get the working tree diff (staged + unstaged changes). + /// Returns the parsed diff plus highlight jobs the streaming worker + /// will run on a background thread. + fn get_working_tree_diff(&self) -> Result; /// Get the staged diff (index vs HEAD) - fn get_staged_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_staged_diff(&self) -> Result { Err(crate::error::TuicrError::UnsupportedOperation( "Staged diff not supported for this VCS".into(), )) } /// Get the unstaged diff (working tree vs index) - fn get_unstaged_diff(&self, _highlighter: &SyntaxHighlighter) -> Result> { + fn get_unstaged_diff(&self) -> Result { Err(crate::error::TuicrError::UnsupportedOperation( "Unstaged diff not supported for this VCS".into(), )) @@ -248,8 +254,7 @@ pub trait VcsBackend: Send { fn get_commit_range_diff( &self, _revision_range: &ResolvedRevisionRange<'_>, - _highlighter: &SyntaxHighlighter, - ) -> Result> { + ) -> Result { Err(crate::error::TuicrError::UnsupportedOperation( "Commit range diff not supported for this VCS".into(), )) @@ -264,11 +269,7 @@ pub trait VcsBackend: Send { /// Get a combined diff from the parent of the oldest commit through to the working tree. /// This shows both committed and working tree changes in a single diff. /// Returns error if not supported (default). - fn get_working_tree_with_commits_diff( - &self, - _commit_ids: &[String], - _highlighter: &SyntaxHighlighter, - ) -> Result> { + fn get_working_tree_with_commits_diff(&self, _commit_ids: &[String]) -> Result { Err(crate::error::TuicrError::UnsupportedOperation( "Working tree + commits diff not supported for this VCS".into(), ))