diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dacfcb9..00a6ea2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,7 +48,7 @@ jobs: run: cargo fmt --all -- --check - name: Run clippy - run: cargo clippy --all-targets --all-features -- -D warnings + run: cargo clippy --all-targets --all-features --locked -- -D warnings - name: Run tests - run: cargo test --all-targets --all-features + run: cargo test --all-targets --all-features --locked diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1bbd0bb..f3cebf3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -330,7 +330,6 @@ jobs: uses: ./.github/workflows/publish-npm.yml with: plan: ${{ needs.plan.outputs.val }} - secrets: inherit # publish jobs get escalated permissions permissions: "contents": "read" diff --git a/README.md b/README.md index e3e77f4..045bc61 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,7 @@ cargo install --git https://github.com/AksharP5/blippy - `blippy`: launch the TUI - `blippy --version`: show version information +- `blippy --help`: show command help - `blippy sync`: scan local repos and cache GitHub remotes - `blippy auth reset`: remove stored auth token from keychain - `blippy cache reset`: remove local cache database diff --git a/dist-workspace.toml b/dist-workspace.toml index f8da503..35c123c 100644 --- a/dist-workspace.toml +++ b/dist-workspace.toml @@ -18,6 +18,8 @@ hosting = "github" # Use a custom npm job because the built-in publisher requires NPM_TOKEN. publish-jobs = ["homebrew", "./publish-npm"] github-custom-job-permissions = { "publish-npm" = { contents = "read", "id-token" = "write" } } +# The generated caller inherits every repository secret. Keep the narrower OIDC-only workflow. +allow-dirty = ["ci"] # Homebrew tap to push formula updates to tap = "AksharP5/homebrew-tap" # Archive formats for generated bundles diff --git a/keybinds.example.toml b/keybinds.example.toml index a5fdd6e..9ce97ed 100644 --- a/keybinds.example.toml +++ b/keybinds.example.toml @@ -8,6 +8,7 @@ # - single chars: "q", "/", "1" # - named keys: "enter", "esc", "tab", "space", "left", "right", "home", "end", "up", "down", "pageup", "pagedown", "backspace" # - modifiers: "ctrl+...", "alt+...", "shift+..." +# Plain characters remain text in editors and search fields. Use a modified or named key there. [keybinds] quit = "ctrl+c" diff --git a/src/app.rs b/src/app.rs index b7652e9..e3cc0bb 100644 --- a/src/app.rs +++ b/src/app.rs @@ -403,6 +403,7 @@ struct PullRequestState { pull_request_files_issue_id: Option, pull_request_id: Option, pull_request_files: Vec, + pull_request_view_state_loaded: bool, pull_request_viewed_files: HashSet, pull_request_collapsed_hunks: HashMap>, pull_request_review_comments: Vec, @@ -428,6 +429,7 @@ impl Default for PullRequestState { pull_request_files_issue_id: None, pull_request_id: None, pull_request_files: Vec::new(), + pull_request_view_state_loaded: false, pull_request_viewed_files: HashSet::new(), pull_request_collapsed_hunks: HashMap::new(), pull_request_review_comments: Vec::new(), diff --git a/src/app/editor.rs b/src/app/editor.rs index d70a328..d005e9e 100644 --- a/src/app/editor.rs +++ b/src/app/editor.rs @@ -230,7 +230,9 @@ impl App { self.comment_editor.backspace_text(); } } - KeyCode::Char(ch) => { + KeyCode::Char(ch) + if key.modifiers.is_empty() || key.modifiers == KeyModifiers::SHIFT => + { if self.comment_editor.create_issue_confirm_visible() { return; } diff --git a/src/app/input.rs b/src/app/input.rs index a0e3467..ad56592 100644 --- a/src/app/input.rs +++ b/src/app/input.rs @@ -2,31 +2,41 @@ use super::*; impl App { pub fn on_key(&mut self, key: KeyEvent) { - let key = match self.keybinds.remap_key(key) { - Some(key) => key, - None => return, - }; if matches!(self.view, View::CommentPresetName | View::CommentEditor) { + let Some(key) = self.keybinds.remap_text_key(key) else { + return; + }; self.handle_editor_key(key); return; } - if self.view == View::RepoPicker - && self.search.repo_search_mode - && self.handle_repo_search_key(key) - { - return; + if self.view == View::RepoPicker && self.search.repo_search_mode { + let Some(key) = self.keybinds.remap_text_key(key) else { + return; + }; + if self.handle_repo_search_key(key) { + return; + } } - if self.view == View::Issues - && self.search.issue_search_mode - && self.handle_issue_search_key(key) - { - return; + if self.view == View::Issues && self.search.issue_search_mode { + let Some(key) = self.keybinds.remap_text_key(key) else { + return; + }; + if self.handle_issue_search_key(key) { + return; + } } - if matches!(self.view, View::LabelPicker | View::AssigneePicker) - && self.handle_popup_filter_key(key) - { - return; + if matches!(self.view, View::LabelPicker | View::AssigneePicker) { + let Some(key) = self.keybinds.remap_text_key(key) else { + return; + }; + if self.handle_popup_filter_key(key) { + return; + } } + let key = match self.keybinds.remap_key(key) { + Some(key) => key, + None => return, + }; if key.modifiers.contains(KeyModifiers::CONTROL) && key.code == KeyCode::Char('r') && self.view == View::RepoPicker diff --git a/src/app/pull_request.rs b/src/app/pull_request.rs index aa16941..b81d46d 100644 --- a/src/app/pull_request.rs +++ b/src/app/pull_request.rs @@ -15,6 +15,10 @@ impl App { .contains(file_path) } + pub fn pull_request_view_state_loaded(&self) -> bool { + self.pull_request.pull_request_view_state_loaded + } + pub fn pull_request_hunk_is_collapsed(&self, file_path: &str, hunk_start: usize) -> bool { self.pull_request .pull_request_collapsed_hunks @@ -60,6 +64,9 @@ impl App { } pub fn selected_pull_request_file_view_toggle(&self) -> Option<(String, bool)> { + if !self.pull_request_view_state_loaded() { + return None; + } let file = self.selected_pull_request_file_row()?; let viewed = self.pull_request_file_is_viewed(file.filename.as_str()); Some((file.filename.clone(), !viewed)) @@ -71,6 +78,7 @@ impl App { viewed_files: HashSet, ) { self.pull_request.pull_request_id = pull_request_id; + self.pull_request.pull_request_view_state_loaded = true; self.pull_request.pull_request_viewed_files = viewed_files; self.pull_request .pull_request_viewed_files @@ -209,6 +217,7 @@ impl App { pub fn set_pull_request_files(&mut self, issue_id: i64, files: Vec) { self.pull_request.pull_request_files_issue_id = Some(issue_id); self.pull_request.pull_request_id = None; + self.pull_request.pull_request_view_state_loaded = false; self.pull_request.pull_request_files = files; let mut active_file_paths = HashSet::new(); for file in &self.pull_request.pull_request_files { @@ -342,6 +351,7 @@ impl App { pub(super) fn reset_pull_request_state(&mut self) { self.pull_request.pull_request_files_issue_id = None; self.pull_request.pull_request_id = None; + self.pull_request.pull_request_view_state_loaded = false; self.pull_request.pull_request_files.clear(); self.pull_request.pull_request_viewed_files.clear(); self.pull_request.pull_request_collapsed_hunks.clear(); diff --git a/src/app/state.rs b/src/app/state.rs index 7341ad0..f88cea3 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -251,6 +251,8 @@ impl App { self.context.path = path.map(ToString::to_string); self.context.issue_id = None; self.context.issue_number = None; + self.sync.syncing = false; + self.reset_issue_sync_state(); self.sync.repo_permissions_syncing = false; self.sync.repo_permissions_sync_requested = true; self.sync.repo_issue_metadata_editable = None; @@ -263,6 +265,7 @@ impl App { self.linked.pull_request_lookups.clear(); self.linked.issue_lookups.clear(); self.linked.navigation_origin = None; + self.interaction.pending_issue_actions.clear(); self.clear_linked_picker_state(); self.reset_pull_request_state(); self.search.repo_search_mode = false; @@ -273,13 +276,24 @@ impl App { } pub fn set_current_issue(&mut self, issue_id: i64, issue_number: i64) { + let issue_changed = self.context.issue_id != Some(issue_id); self.context.issue_id = Some(issue_id); self.context.issue_number = Some(issue_number); - if self.pull_request.pull_request_files_issue_id != Some(issue_id) { + if issue_changed { + self.reset_issue_sync_state(); self.reset_pull_request_state(); } } + fn reset_issue_sync_state(&mut self) { + self.sync.comment_syncing = false; + self.sync.pull_request_files_syncing = false; + self.sync.pull_request_review_comments_syncing = false; + self.sync.comment_sync_requested = false; + self.sync.pull_request_files_sync_requested = false; + self.sync.pull_request_review_comments_sync_requested = false; + } + pub fn update_issue_state_by_number(&mut self, issue_number: i64, state: &str) { for issue in &mut self.issues { if issue.number == issue_number { diff --git a/src/app/tests/part2.rs b/src/app/tests/part2.rs index 8c31620..986484c 100644 --- a/src/app/tests/part2.rs +++ b/src/app/tests/part2.rs @@ -125,6 +125,22 @@ fn slash_search_matches_issue_number() { ); } +#[test] +fn custom_plain_keybinding_does_not_rewrite_search_text() { + let mut config = Config::default(); + config + .keybinds + .insert("refresh".to_string(), "x".to_string()); + let mut app = App::new(config); + app.set_view(View::Issues); + app.on_key(KeyEvent::new(KeyCode::Char('/'), KeyModifiers::NONE)); + + app.on_key(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)); + app.on_key(KeyEvent::new(KeyCode::Char('r'), KeyModifiers::NONE)); + + assert_eq!(app.issue_query(), "xr"); +} + #[test] fn reopen_action_for_closed_issue() { let mut app = App::new(Config::default()); @@ -610,6 +626,9 @@ fn selected_pull_request_file_view_toggle_flips_current_state() { patch: Some("@@ -1,1 +1,1 @@\n-old\n+new".to_string()), }], ); + assert!(app.selected_pull_request_file_view_toggle().is_none()); + + app.set_pull_request_view_state(Some("PR_id".to_string()), std::collections::HashSet::new()); let (path, viewed) = app .selected_pull_request_file_view_toggle() diff --git a/src/app/tests/part3.rs b/src/app/tests/part3.rs index 187e0ae..0fea51f 100644 --- a/src/app/tests/part3.rs +++ b/src/app/tests/part3.rs @@ -697,6 +697,38 @@ fn create_issue_editor_supports_title_and_body_entry() { assert_eq!(app.take_action(), Some(AppAction::SubmitCreatedIssue)); } +#[test] +fn custom_plain_keybinding_does_not_rewrite_editor_text() { + let mut config = Config::default(); + config + .keybinds + .insert("refresh".to_string(), "x".to_string()); + let mut app = App::new(config); + app.open_issue_comment_editor(View::Issues); + + app.on_key(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)); + app.on_key(KeyEvent::new(KeyCode::Char('r'), KeyModifiers::NONE)); + + assert_eq!(app.editor().text(), "xr"); +} + +#[test] +fn custom_modified_submit_key_still_works_in_editor() { + let mut config = Config::default(); + config + .keybinds + .insert("submit".to_string(), "ctrl+s".to_string()); + let mut app = App::new(config); + app.open_issue_comment_editor(View::Issues); + + app.on_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert_eq!(app.take_action(), None); + + app.on_key(KeyEvent::new(KeyCode::Char('s'), KeyModifiers::CONTROL)); + + assert_eq!(app.take_action(), Some(AppAction::SubmitIssueComment)); +} + #[test] fn create_issue_confirm_cancel_keeps_editor_open() { let mut app = App::new(Config::default()); diff --git a/src/app/tests/part4.rs b/src/app/tests/part4.rs index b5d9484..7330efc 100644 --- a/src/app/tests/part4.rs +++ b/src/app/tests/part4.rs @@ -36,6 +36,26 @@ fn repo_picker_search_filters_entries() { assert_eq!(app.filtered_repo_rows().len(), 2); } +#[test] +fn changing_repo_clears_repo_scoped_in_flight_state() { + let mut app = App::new(Config::default()); + app.set_current_repo_with_path("acme", "one", None); + app.set_current_issue(10, 7); + app.set_syncing(true); + app.set_comment_syncing(true); + app.set_pull_request_files_syncing(true); + app.set_pull_request_review_comments_syncing(true); + app.set_pending_issue_action(7, super::super::PendingIssueAction::Closing); + + app.set_current_repo_with_path("acme", "two", None); + + assert!(!app.syncing()); + assert!(!app.comment_syncing()); + assert!(!app.pull_request_files_syncing()); + assert!(!app.pull_request_review_comments_syncing()); + assert_eq!(app.pending_issue_badge(7), None); +} + #[test] fn ctrl_g_resets_repo_picker_query_when_reopened() { let mut app = App::new(Config::default()); diff --git a/src/cli.rs b/src/cli.rs index 1b74413..1b4d39e 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,38 +1,28 @@ -use anyhow::Result; +use anyhow::{Result, bail}; + +pub const USAGE: &str = "Usage: blippy [COMMAND]\n\nCommands:\n sync Scan local repos and cache GitHub remotes\n auth reset Remove the stored auth token\n cache reset Remove the local cache\n -h, --help Show this help\n -V, --version\n Show version information"; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CliCommand { AuthReset, CacheReset, + Help, Sync, Version, } pub fn parse_args(args: &[String]) -> Result> { - if args.len() <= 1 { - return Ok(None); - } - - let command = args.get(1).map(String::as_str); - let subcommand = args.get(2).map(String::as_str); - - if command == Some("--version") || command == Some("-V") { - return Ok(Some(CliCommand::Version)); - } - - if command == Some("auth") && subcommand == Some("reset") { - return Ok(Some(CliCommand::AuthReset)); - } - - if command == Some("cache") && subcommand == Some("reset") { - return Ok(Some(CliCommand::CacheReset)); - } - - if command == Some("sync") { - return Ok(Some(CliCommand::Sync)); - } - - Ok(None) + let command = args.iter().skip(1).map(String::as_str).collect::>(); + let parsed = match command.as_slice() { + [] => None, + ["--version"] | ["-V"] => Some(CliCommand::Version), + ["--help"] | ["-h"] | ["help"] => Some(CliCommand::Help), + ["auth", "reset"] => Some(CliCommand::AuthReset), + ["cache", "reset"] => Some(CliCommand::CacheReset), + ["sync"] => Some(CliCommand::Sync), + _ => bail!("Unknown command: {}\n\n{}", command.join(" "), USAGE), + }; + Ok(parsed) } #[cfg(test)] @@ -90,4 +80,29 @@ mod tests { let parsed = parse_args(&args).expect("parse succeeds"); assert_eq!(parsed, Some(CliCommand::Version)); } + + #[test] + fn parse_args_returns_help() { + let args = vec!["blippy".to_string(), "--help".to_string()]; + let parsed = parse_args(&args).expect("parse succeeds"); + assert_eq!(parsed, Some(CliCommand::Help)); + } + + #[test] + fn parse_args_rejects_unknown_commands() { + let args = vec!["blippy".to_string(), "typo".to_string()]; + let error = parse_args(&args).expect_err("unknown command fails"); + assert!(error.to_string().contains("Unknown command: typo")); + } + + #[test] + fn parse_args_rejects_extra_arguments() { + let args = vec![ + "blippy".to_string(), + "cache".to_string(), + "reset".to_string(), + "extra".to_string(), + ]; + assert!(parse_args(&args).is_err()); + } } diff --git a/src/discovery.rs b/src/discovery.rs index d7b9d20..44fbb81 100644 --- a/src/discovery.rs +++ b/src/discovery.rs @@ -80,8 +80,7 @@ fn scan_repos_in_dir( continue; } - let git_dir = path.join(".git"); - if git_dir.is_dir() { + if is_git_repo(&path) { repos.push(DiscoveredRepo { path }); continue; } @@ -112,6 +111,11 @@ fn scan_repos_in_dir( Ok(repos) } +pub fn is_git_repo(path: &Path) -> bool { + let git_entry = path.join(".git"); + git_entry.is_dir() || git_entry.is_file() +} + fn excluded_dirs() -> HashSet<&'static str> { let names = [ ".git", @@ -167,6 +171,19 @@ mod tests { let _ = fs::remove_dir_all(&root); } + #[test] + fn scan_repos_in_dir_finds_git_files_used_by_worktrees() { + let root = unique_temp_dir("worktree"); + let repo_path = root.join("work").join("repo"); + fs::create_dir_all(&repo_path).expect("create repo"); + fs::write(repo_path.join(".git"), "gitdir: /tmp/example.git").expect("create .git"); + + let repos = scan_repos_in_dir(&root, 4, &excluded_dirs()).expect("scan"); + assert_eq!(repos, vec![DiscoveredRepo { path: repo_path }]); + + let _ = fs::remove_dir_all(&root); + } + #[test] fn scan_repos_in_dir_skips_excluded_dirs() { let root = unique_temp_dir("excluded"); diff --git a/src/git.rs b/src/git.rs index b7c413a..a2e1232 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{Context, Result, bail}; #[derive(Debug, Clone, PartialEq, Eq)] pub struct RepoSlug { @@ -110,20 +110,20 @@ pub fn list_github_remotes_at(path: &std::path::Path) -> Result> .arg("-C") .arg(path) .args(["remote", "-v"]) - .output(); - - let output = match output { - Ok(output) => output, - Err(error) => { - if error.kind() == std::io::ErrorKind::NotFound { - return Ok(Vec::new()); - } - return Err(error.into()); - } - }; + .output() + .with_context(|| format!("failed to inspect Git remotes at {}", path.display()))?; if !output.status.success() { - return Ok(Vec::new()); + let stderr = String::from_utf8_lossy(&output.stderr); + let message = stderr.trim(); + if message.is_empty() { + bail!( + "git remote -v failed at {} with {}", + path.display(), + output.status + ); + } + bail!("git remote -v failed at {}: {}", path.display(), message); } let stdout = String::from_utf8_lossy(&output.stdout); @@ -250,6 +250,18 @@ mod tests { let _ = fs::remove_dir_all(&dir); } + #[test] + fn list_github_remotes_reports_broken_git_metadata() { + let dir = unique_temp_dir("broken-git-remote"); + fs::write(dir.join(".git"), "gitdir: /definitely/missing/blippy\n") + .expect("write broken git file"); + + let error = super::list_github_remotes_at(&dir).expect_err("broken repo must fail"); + assert!(error.to_string().contains("git remote -v failed")); + + let _ = fs::remove_dir_all(&dir); + } + fn unique_temp_dir(label: &str) -> PathBuf { let nanos = SystemTime::now() .duration_since(UNIX_EPOCH) diff --git a/src/github/issues.rs b/src/github/issues.rs index 82d29ab..8a0dd65 100644 --- a/src/github/issues.rs +++ b/src/github/issues.rs @@ -109,7 +109,9 @@ impl GitHubClient { Some(pull_number) => pull_number, None => continue, }; - if !html_url.contains("/pull/") || !seen.insert(pull_number) { + if !item_url_matches_repo(html_url, owner, repo, "pull", pull_number) + || !seen.insert(pull_number) + { continue; } linked.push((pull_number, html_url.to_string())); @@ -167,7 +169,9 @@ impl GitHubClient { Some(issue_number) => issue_number, None => continue, }; - if !html_url.contains("/issues/") || !seen.insert(issue_number) { + if !item_url_matches_repo(html_url, owner, repo, "issues", issue_number) + || !seen.insert(issue_number) + { continue; } linked.push((issue_number, html_url.to_string())); @@ -304,3 +308,39 @@ impl GitHubClient { Ok(assignees) } } + +fn item_url_matches_repo( + html_url: &str, + owner: &str, + repo: &str, + route: &str, + number: i64, +) -> bool { + let expected = format!("https://github.com/{}/{}/{}/{}", owner, repo, route, number); + html_url + .trim_end_matches('/') + .eq_ignore_ascii_case(&expected) +} + +#[cfg(test)] +mod tests { + use super::item_url_matches_repo; + + #[test] + fn linked_item_url_must_match_the_current_repo() { + assert!(item_url_matches_repo( + "https://github.com/acme/blippy/pull/20", + "Acme", + "Blippy", + "pull", + 20, + )); + assert!(!item_url_matches_repo( + "https://github.com/other/blippy/pull/20", + "acme", + "blippy", + "pull", + 20, + )); + } +} diff --git a/src/github/pull_requests.rs b/src/github/pull_requests.rs index da03a01..372d3c7 100644 --- a/src/github/pull_requests.rs +++ b/src/github/pull_requests.rs @@ -59,16 +59,6 @@ impl GitHubClient { } } "#; - let id_only_query = r#" - query($owner: String!, $repo: String!, $number: Int!) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $number) { - id - } - } - } - "#; - let mut cursor: Option = None; let mut pull_request_id: Option = None; let mut viewed_files = HashSet::new(); @@ -80,26 +70,7 @@ impl GitHubClient { "number": pull_number, "cursor": cursor, }); - let response = match self.graphql(query, payload).await { - Ok(response) => response, - Err(_) => { - let fallback = self - .graphql( - id_only_query, - serde_json::json!({ - "owner": owner, - "repo": repo, - "number": pull_number, - }), - ) - .await?; - let pull_request_id = fallback["data"]["repository"]["pullRequest"] - .get("id") - .and_then(serde_json::Value::as_str) - .map(ToString::to_string); - return Ok((pull_request_id, HashSet::new())); - } - }; + let response = self.graphql(query, payload).await?; let pull_request = &response["data"]["repository"]["pullRequest"]; if pull_request.is_null() { return Ok((None, HashSet::new())); @@ -261,8 +232,7 @@ impl GitHubClient { ) -> Result> { let thread_map = self .list_pull_request_review_thread_map(owner, repo, pull_number) - .await - .unwrap_or_default(); + .await?; let mut page = 1; let mut comments = Vec::new(); diff --git a/src/keybinds.rs b/src/keybinds.rs index c516f1b..a716081 100644 --- a/src/keybinds.rs +++ b/src/keybinds.rs @@ -327,6 +327,20 @@ impl Keybinds { Some(key) } + pub fn remap_text_key(&self, key: KeyEvent) -> Option { + if matches!(key.code, KeyCode::Char(_)) + && (key.modifiers.is_empty() || key.modifiers == KeyModifiers::SHIFT) + { + return Some(key); + } + + match self.remap_key(key) { + Some(mapped) if !matches!(mapped.code, KeyCode::Char(_)) => Some(mapped), + Some(_) => Some(key), + None => None, + } + } + pub fn binding_label(&self, action: &str) -> String { let binding = match self.action_bindings.get(action) { Some(binding) => binding, diff --git a/src/main.rs b/src/main.rs index 9fccbe9..dcd58d8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -44,17 +44,18 @@ use crate::app::{ PullRequestFile, PullRequestReviewComment, ReviewSide, View, WorkItemMode, }; use crate::auth::{SystemAuth, clear_auth_token, resolve_auth_token}; -use crate::cli::{CliCommand, parse_args}; +use crate::cli::{CliCommand, USAGE, parse_args}; use crate::clipboard::SystemClipboard; use crate::config::Config; use crate::discovery::{home_dir, quick_scan}; use crate::git::list_github_remotes_at; use crate::github::GitHubClient; -use crate::repo_index::index_repo_path; +use crate::repo_index::{index_repo_path, prune_missing_local_repos}; use crate::store::delete_db; use crate::store::{ comment_now_epoch, comments_for_issue, get_repo_by_slug, list_issues, list_local_repos, - prune_comments, touch_comments_for_issue, update_issue_comments_count, + prune_comments, replace_comments_for_issue, touch_comments_for_issue, + update_issue_comments_count, }; use crate::sync::{SyncStats, sync_repo_with_progress}; @@ -206,6 +207,10 @@ fn handle_command(command: CliCommand) -> Result<()> { match command { CliCommand::AuthReset => handle_auth_reset(), CliCommand::CacheReset => handle_cache_reset(), + CliCommand::Help => { + println!("{}", USAGE); + Ok(()) + } CliCommand::Sync => handle_sync(), CliCommand::Version => { println!("blippy {}", env!("CARGO_PKG_VERSION")); @@ -247,6 +252,7 @@ fn handle_sync() -> Result<()> { for repo in &repos { indexed += index_repo_path(&conn, &repo.path)?; } + prune_missing_local_repos(&conn)?; let duration = start.elapsed(); println!( @@ -371,6 +377,26 @@ enum LinkedIssueTarget { Probe, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct RepoIdentity { + owner: String, + repo: String, +} + +impl RepoIdentity { + fn new(owner: &str, repo: &str) -> Self { + Self { + owner: owner.to_string(), + repo: repo.to_string(), + } + } + + fn is_current(&self, app: &App) -> bool { + app.current_owner() == Some(self.owner.as_str()) + && app.current_repo() == Some(self.repo.as_str()) + } +} + fn load_comments_for_issue( app: &mut App, conn: &rusqlite::Connection, @@ -395,6 +421,9 @@ enum ScanMode { enum AppEvent { ReposUpdated, ScanFinished, + ScanFailed { + message: String, + }, SyncProgress { owner: String, repo: String, @@ -424,6 +453,7 @@ enum AppEvent { files: Vec, pull_request_id: Option, viewed_files: HashSet, + view_state_error: Option, }, PullRequestFilesFailed { issue_id: i64, @@ -481,49 +511,60 @@ enum AppEvent { message: String, }, LinkedPullRequestResolved { + repo: RepoIdentity, issue_number: i64, pull_requests: Vec<(i64, String)>, target: LinkedPullRequestTarget, }, LinkedPullRequestLookupFailed { + repo: RepoIdentity, issue_number: i64, message: String, target: LinkedPullRequestTarget, }, LinkedIssueResolved { + repo: RepoIdentity, pull_number: i64, issues: Vec<(i64, String)>, target: LinkedIssueTarget, }, LinkedIssueLookupFailed { + repo: RepoIdentity, pull_number: i64, message: String, target: LinkedIssueTarget, }, IssueUpdated { + repo: RepoIdentity, issue_number: i64, message: String, }, IssueCreated { + repo: RepoIdentity, issue_number: i64, }, IssueCreateFailed { + repo: RepoIdentity, message: String, }, IssueLabelsUpdated { + repo: RepoIdentity, issue_number: i64, labels: String, }, IssueAssigneesUpdated { + repo: RepoIdentity, issue_number: i64, assignees: String, }, IssueCommentUpdated { + repo: RepoIdentity, issue_number: i64, comment_id: i64, body: String, }, IssueCommentDeleted { + repo: RepoIdentity, issue_number: i64, comment_id: i64, count: usize, @@ -576,10 +617,21 @@ impl TerminalGuard { fn init() -> Result { enable_raw_mode()?; let mut stdout = io::stdout(); - execute!(stdout, EnterAlternateScreen, EnableMouseCapture)?; + if let Err(error) = execute!(stdout, EnterAlternateScreen, EnableMouseCapture) { + let _ = disable_raw_mode(); + let _ = execute!(io::stdout(), LeaveAlternateScreen, DisableMouseCapture); + return Err(error.into()); + } let backend = CrosstermBackend::new(stdout); - let terminal = Terminal::new(backend)?; + let terminal = match Terminal::new(backend) { + Ok(terminal) => terminal, + Err(error) => { + let _ = disable_raw_mode(); + let _ = execute!(io::stdout(), LeaveAlternateScreen, DisableMouseCapture); + return Err(error.into()); + } + }; Ok(Self { terminal }) } diff --git a/src/main/tests.rs b/src/main/tests.rs index 20aceb6..1d15548 100644 --- a/src/main/tests.rs +++ b/src/main/tests.rs @@ -310,6 +310,7 @@ fn issue_updated_marks_pull_request_merged() { let (event_tx, event_rx) = channel(); event_tx .send(super::AppEvent::IssueUpdated { + repo: super::RepoIdentity::new("acme", "blippy"), issue_number: 92, message: "merged".to_string(), }) @@ -325,6 +326,93 @@ fn issue_updated_marks_pull_request_merged() { assert_eq!(merged_state, Some("merged")); } +#[test] +fn issue_events_from_another_repo_do_not_change_current_repo() { + let conn = rusqlite::Connection::open_in_memory().expect("conn"); + let mut app = crate::app::App::new(Config::default()); + app.set_current_repo_with_path("acme", "current", None); + app.set_view(View::Issues); + app.set_issues(vec![IssueRow { + id: 34, + repo_id: 2, + number: 92, + state: "open".to_string(), + title: "Current repo issue".to_string(), + body: String::new(), + labels: String::new(), + assignees: String::new(), + comments_count: 0, + updated_at: None, + is_pr: false, + }]); + + let (event_tx, event_rx) = channel(); + event_tx + .send(super::AppEvent::IssueUpdated { + repo: super::RepoIdentity::new("acme", "previous"), + issue_number: 92, + message: "closed".to_string(), + }) + .expect("send event"); + super::main_events::handle_events(&mut app, &conn, &event_rx).expect("handle events"); + + assert_eq!(app.issues()[0].state, "open"); + assert_ne!(app.status(), "#92 closed"); +} + +#[test] +fn failed_link_probe_is_not_retried_automatically() { + let conn = rusqlite::Connection::open_in_memory().expect("conn"); + let mut app = crate::app::App::new(Config::default()); + app.set_current_repo_with_path("acme", "blippy", None); + assert!(app.begin_linked_pull_request_lookup(7)); + + let (event_tx, event_rx) = channel(); + event_tx + .send(super::AppEvent::LinkedPullRequestLookupFailed { + repo: super::RepoIdentity::new("acme", "blippy"), + issue_number: 7, + message: "offline".to_string(), + target: super::LinkedPullRequestTarget::Probe, + }) + .expect("send event"); + super::main_events::handle_events(&mut app, &conn, &event_rx).expect("handle events"); + + assert!(app.linked_pull_request_known(7)); + assert!(!app.begin_linked_pull_request_lookup(7)); +} + +#[test] +fn pull_request_view_state_failure_stays_unknown() { + let conn = rusqlite::Connection::open_in_memory().expect("conn"); + let mut app = crate::app::App::new(Config::default()); + app.set_current_repo_with_path("acme", "blippy", None); + app.set_current_issue(33, 92); + + let (event_tx, event_rx) = channel(); + event_tx + .send(super::AppEvent::PullRequestFilesUpdated { + issue_id: 33, + files: vec![crate::app::PullRequestFile { + filename: "src/main.rs".to_string(), + status: "modified".to_string(), + additions: 1, + deletions: 1, + patch: None, + }], + pull_request_id: None, + viewed_files: std::collections::HashSet::new(), + view_state_error: Some("offline".to_string()), + }) + .expect("send event"); + super::main_events::handle_events(&mut app, &conn, &event_rx).expect("handle events"); + + assert_eq!(app.pull_request_files().len(), 1); + assert!(!app.pull_request_view_state_loaded()); + assert!(app.selected_pull_request_file_view_toggle().is_none()); + assert!(app.status().contains("view state unavailable")); +} + #[test] fn submit_created_issue_requires_non_empty_title() { let conn = rusqlite::Connection::open_in_memory().expect("conn"); diff --git a/src/main_action_utils/pr_review_actions.rs b/src/main_action_utils/pr_review_actions.rs index eee4981..1018da4 100644 --- a/src/main_action_utils/pr_review_actions.rs +++ b/src/main_action_utils/pr_review_actions.rs @@ -205,6 +205,11 @@ pub(crate) fn toggle_pull_request_file_viewed( token: &str, event_tx: Sender, ) -> Result<()> { + if !app.pull_request_view_state_loaded() { + app.request_pull_request_files_sync(); + app.set_status("Loading pull request view state".to_string()); + return Ok(()); + } let (path, viewed) = match app.selected_pull_request_file_view_toggle() { Some(toggle) => toggle, None => { diff --git a/src/main_data.rs b/src/main_data.rs index e59dca6..4ad6a12 100644 --- a/src/main_data.rs +++ b/src/main_data.rs @@ -1,4 +1,5 @@ use super::*; +use std::path::Path; pub(super) fn initialize_app(app: &mut App, conn: &rusqlite::Connection) -> Result<()> { let repo_root = crate::git::repo_root()?; @@ -90,29 +91,36 @@ pub(super) fn start_scan(event_tx: Sender, mode: ScanMode) -> Result<( let cwd = env::current_dir()?; let home = home_dir().unwrap_or(cwd.clone()); thread::spawn(move || { - let conn = match crate::store::open_db() { - Ok(conn) => conn, - Err(_) => return, + let result = run_scan(&cwd, &home, mode, &event_tx); + let event = match result { + Ok(()) => AppEvent::ScanFinished, + Err(error) => AppEvent::ScanFailed { + message: error.to_string(), + }, }; + let _ = event_tx.send(event); + }); - if matches!(mode, ScanMode::QuickOnly | ScanMode::QuickAndFull) { - let quick = quick_scan(&cwd, 4, 2).unwrap_or_default(); - for repo in &quick { - let _ = index_repo_path(&conn, &repo.path); - } - let _ = event_tx.send(AppEvent::ReposUpdated); - } + Ok(()) +} + +fn run_scan(cwd: &Path, home: &Path, mode: ScanMode, event_tx: &Sender) -> Result<()> { + let conn = crate::store::open_db()?; - if matches!(mode, ScanMode::FullOnly | ScanMode::QuickAndFull) { - let full = crate::discovery::full_scan(&home).unwrap_or_default(); - for repo in &full { - let _ = index_repo_path(&conn, &repo.path); - } - let _ = event_tx.send(AppEvent::ReposUpdated); + if matches!(mode, ScanMode::QuickOnly | ScanMode::QuickAndFull) { + for repo in quick_scan(cwd, 4, 2)? { + index_repo_path(&conn, &repo.path)?; } + let _ = event_tx.send(AppEvent::ReposUpdated); + } - let _ = event_tx.send(AppEvent::ScanFinished); - }); + if matches!(mode, ScanMode::FullOnly | ScanMode::QuickAndFull) { + for repo in crate::discovery::full_scan(home)? { + index_repo_path(&conn, &repo.path)?; + } + } + prune_missing_local_repos(&conn)?; + let _ = event_tx.send(AppEvent::ReposUpdated); Ok(()) } diff --git a/src/main_events.rs b/src/main_events.rs index bfcafe2..dea40af 100644 --- a/src/main_events.rs +++ b/src/main_events.rs @@ -19,14 +19,26 @@ pub(super) fn handle_events( app.set_status(String::new()); } } + AppEvent::ScanFailed { message } => { + app.set_scanning(false); + if app.view() == View::RepoPicker { + app.set_status(format!("Scan failed: {}", message)); + } + } AppEvent::SyncFinished { owner, repo, stats } => { - app.set_syncing(false); if app.current_owner() == Some(owner.as_str()) && app.current_repo() == Some(repo.as_str()) { + app.set_syncing(false); refresh_current_repo_issues(app, conn)?; - app.request_repo_labels_sync(); let (open_count, closed_count) = app.issue_counts(); + if let Some(reason) = stats.incomplete_reason { + app.set_status(format!( + "Sync incomplete after {} issues: {}", + stats.issues, reason + )); + continue; + } if stats.not_modified { app.set_status(format!( "No issue changes (open: {}, closed: {})", @@ -34,6 +46,7 @@ pub(super) fn handle_events( )); continue; } + app.request_repo_labels_sync(); app.set_status(format!( "Synced {} issues (open: {}, closed: {})", stats.issues, open_count, closed_count @@ -49,11 +62,9 @@ pub(super) fn handle_events( if app.current_owner() == Some(owner.as_str()) && app.current_repo() == Some(repo.as_str()) { - refresh_current_repo_issues(app, conn)?; - let (open_count, closed_count) = app.issue_counts(); app.set_status(format!( - "Syncing page {}: {} issues cached (open: {}, closed: {})", - page, stats.issues, open_count, closed_count + "Syncing page {}: {} issues cached", + page, stats.issues )); } } @@ -62,30 +73,34 @@ pub(super) fn handle_events( repo, message, } => { - app.set_syncing(false); if app.current_owner() == Some(owner.as_str()) && app.current_repo() == Some(repo.as_str()) { + app.set_syncing(false); app.set_status(format!("Sync failed: {}", message)); } } AppEvent::CommentsUpdated { issue_id, count } => { - app.set_comment_syncing(false); if app.current_issue_id() == Some(issue_id) { + app.set_comment_syncing(false); load_comments_for_issue(app, conn, issue_id)?; app.set_status(format!("Updated {} comments", count)); } } AppEvent::CommentsFailed { issue_id, message } => { - app.set_comment_syncing(false); if app.current_issue_id() == Some(issue_id) { + app.set_comment_syncing(false); app.set_status(format!("Comments unavailable: {}", message)); } } AppEvent::IssueUpdated { + repo, issue_number, message, } => { + if !repo.is_current(app) { + continue; + } if message.starts_with("closed") || message.starts_with("close failed") || message.starts_with("reopened") @@ -112,7 +127,10 @@ pub(super) fn handle_events( app.request_comment_sync(); } } - AppEvent::IssueCreated { issue_number } => { + AppEvent::IssueCreated { repo, issue_number } => { + if !repo.is_current(app) { + continue; + } app.set_work_item_mode(WorkItemMode::Issues); app.set_issue_filter(IssueFilter::Open); refresh_current_repo_issues(app, conn)?; @@ -128,22 +146,33 @@ pub(super) fn handle_events( app.set_status(format!("Created issue #{}", issue_number)); app.request_sync(); } - AppEvent::IssueCreateFailed { message } => { + AppEvent::IssueCreateFailed { repo, message } => { + if !repo.is_current(app) { + continue; + } app.set_status(format!("Issue creation failed: {}", message)); } AppEvent::IssueLabelsUpdated { + repo, issue_number, labels, } => { + if !repo.is_current(app) { + continue; + } app.clear_pending_issue_action(issue_number); app.update_issue_labels_by_number(issue_number, labels.as_str()); app.set_status(format!("#{} labels updated", issue_number)); app.request_sync(); } AppEvent::IssueAssigneesUpdated { + repo, issue_number, assignees, } => { + if !repo.is_current(app) { + continue; + } app.clear_pending_issue_action(issue_number); app.update_issue_assignees_by_number(issue_number, assignees.as_str()); app.set_status(format!("#{} assignees updated", issue_number)); @@ -154,32 +183,40 @@ pub(super) fn handle_events( files, pull_request_id, viewed_files, + view_state_error, } => { - app.set_pull_request_files_syncing(false); if app.current_issue_id() == Some(issue_id) { + app.set_pull_request_files_syncing(false); let count = files.len(); app.set_pull_request_files(issue_id, files); + if let Some(message) = view_state_error { + app.set_status(format!( + "Loaded {} changed files; view state unavailable: {}", + count, message + )); + continue; + } app.set_pull_request_view_state(pull_request_id, viewed_files); app.set_status(format!("Loaded {} changed files", count)); } } AppEvent::PullRequestFilesFailed { issue_id, message } => { - app.set_pull_request_files_syncing(false); if app.current_issue_id() == Some(issue_id) { + app.set_pull_request_files_syncing(false); app.set_status(format!("PR files unavailable: {}", message)); } } AppEvent::PullRequestReviewCommentsUpdated { issue_id, comments } => { - app.set_pull_request_review_comments_syncing(false); if app.current_issue_id() == Some(issue_id) { + app.set_pull_request_review_comments_syncing(false); let count = comments.len(); app.set_pull_request_review_comments(comments); app.set_status(format!("Loaded {} review comments", count)); } } AppEvent::PullRequestReviewCommentsFailed { issue_id, message } => { - app.set_pull_request_review_comments_syncing(false); if app.current_issue_id() == Some(issue_id) { + app.set_pull_request_review_comments_syncing(false); app.set_status(format!("PR review comments unavailable: {}", message)); } } @@ -267,10 +304,14 @@ pub(super) fn handle_events( } } AppEvent::LinkedPullRequestResolved { + repo, issue_number, pull_requests, target, } => { + if !repo.is_current(app) { + continue; + } let pull_numbers = pull_requests .iter() .map(|(pull_number, _url)| *pull_number) @@ -368,11 +409,15 @@ pub(super) fn handle_events( )); } AppEvent::LinkedPullRequestLookupFailed { + repo, issue_number, message, target, } => { - app.end_linked_pull_request_lookup(issue_number); + if !repo.is_current(app) { + continue; + } + app.set_linked_pull_requests(issue_number, Vec::new()); if target == LinkedPullRequestTarget::Probe { continue; } @@ -387,10 +432,14 @@ pub(super) fn handle_events( )); } AppEvent::LinkedIssueResolved { + repo, pull_number, issues, target, } => { + if !repo.is_current(app) { + continue; + } let issue_numbers = issues .iter() .map(|(issue_number, _url)| *issue_number) @@ -479,11 +528,15 @@ pub(super) fn handle_events( )); } AppEvent::LinkedIssueLookupFailed { + repo, pull_number, message, target, } => { - app.end_linked_issue_lookup(pull_number); + if !repo.is_current(app) { + continue; + } + app.set_linked_issues_for_pull_request(pull_number, Vec::new()); if target == LinkedIssueTarget::Probe { continue; } @@ -498,20 +551,28 @@ pub(super) fn handle_events( )); } AppEvent::IssueCommentUpdated { + repo, issue_number, comment_id, body, } => { + if !repo.is_current(app) { + continue; + } app.update_comment_body_by_id(comment_id, body.as_str()); app.set_status(format!("#{} comment updated", issue_number)); app.request_comment_sync(); app.request_sync(); } AppEvent::IssueCommentDeleted { + repo, issue_number, comment_id, count, } => { + if !repo.is_current(app) { + continue; + } app.remove_comment_by_id(comment_id); app.update_issue_comments_count_by_number(issue_number, count as i64); app.set_status(format!("#{} comment deleted", issue_number)); @@ -523,10 +584,10 @@ pub(super) fn handle_events( repo, labels, } => { - app.set_repo_labels_syncing(false); if app.current_owner() == Some(owner.as_str()) && app.current_repo() == Some(repo.as_str()) { + app.set_repo_labels_syncing(false); app.merge_repo_label_colors(labels.clone()); if app.view() == View::LabelPicker { let options = labels diff --git a/src/main_linked_actions.rs b/src/main_linked_actions.rs index 396c9ac..f1349e4 100644 --- a/src/main_linked_actions.rs +++ b/src/main_linked_actions.rs @@ -425,10 +425,13 @@ pub(super) fn start_linked_pull_request_lookup( event_tx: Sender, target: LinkedPullRequestTarget, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::LinkedPullRequestLookupFailed { + repo: setup_repo, issue_number, message, target, @@ -444,6 +447,7 @@ pub(super) fn start_linked_pull_request_lookup( match result { Ok(pull_requests) => { let _ = event_tx.send(AppEvent::LinkedPullRequestResolved { + repo: event_repo.clone(), issue_number, pull_requests, target, @@ -451,6 +455,7 @@ pub(super) fn start_linked_pull_request_lookup( } Err(error) => { let _ = event_tx.send(AppEvent::LinkedPullRequestLookupFailed { + repo: event_repo.clone(), issue_number, message: error.to_string(), target, @@ -469,10 +474,13 @@ pub(super) fn start_linked_issue_lookup( event_tx: Sender, target: LinkedIssueTarget, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::LinkedIssueLookupFailed { + repo: setup_repo, pull_number, message, target, @@ -488,6 +496,7 @@ pub(super) fn start_linked_issue_lookup( match result { Ok(issues) => { let _ = event_tx.send(AppEvent::LinkedIssueResolved { + repo: event_repo.clone(), pull_number, issues, target, @@ -495,6 +504,7 @@ pub(super) fn start_linked_issue_lookup( } Err(error) => { let _ = event_tx.send(AppEvent::LinkedIssueLookupFailed { + repo: event_repo.clone(), pull_number, message: error.to_string(), target, diff --git a/src/main_sync/issue_actions.rs b/src/main_sync/issue_actions.rs index ab259a4..9db30e5 100644 --- a/src/main_sync/issue_actions.rs +++ b/src/main_sync/issue_actions.rs @@ -8,10 +8,13 @@ pub(crate) fn start_add_comment( body: String, event_tx: Sender, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::IssueUpdated { + repo: setup_repo, issue_number, message: format!("comment failed: {}", message), }, @@ -26,12 +29,14 @@ pub(crate) fn start_add_comment( match result { Ok(()) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: "commented".to_string(), }); } Err(error) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: format!("comment failed: {}", error), }); @@ -49,10 +54,15 @@ pub(crate) fn start_create_issue( body: Option, event_tx: Sender, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, - move |message| AppEvent::IssueCreateFailed { message }, + move |message| AppEvent::IssueCreateFailed { + repo: setup_repo, + message, + }, move |services, event_tx| { let result = services.runtime.block_on(async { services @@ -75,11 +85,13 @@ pub(crate) fn start_create_issue( } }); let _ = event_tx.send(AppEvent::IssueCreated { + repo: event_repo.clone(), issue_number: issue.number, }); } Err(error) => { let _ = event_tx.send(AppEvent::IssueCreateFailed { + repo: event_repo.clone(), message: error.to_string(), }); } @@ -97,10 +109,13 @@ pub(crate) fn start_update_comment( body: String, event_tx: Sender, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::IssueUpdated { + repo: setup_repo, issue_number, message: format!("comment update failed: {}", message), }, @@ -122,6 +137,7 @@ pub(crate) fn start_update_comment( ); }); let _ = event_tx.send(AppEvent::IssueCommentUpdated { + repo: event_repo.clone(), issue_number, comment_id, body, @@ -129,6 +145,7 @@ pub(crate) fn start_update_comment( } Err(error) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: format!("comment update failed: {}", error), }); @@ -147,10 +164,13 @@ pub(crate) fn start_delete_comment( token: String, event_tx: Sender, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::IssueUpdated { + repo: setup_repo, issue_number, message: format!("comment delete failed: {}", message), }, @@ -173,6 +193,7 @@ pub(crate) fn start_delete_comment( let _ = update_issue_comments_count(conn, issue_id, count as i64); }); let _ = event_tx.send(AppEvent::IssueCommentDeleted { + repo: event_repo.clone(), issue_number, comment_id, count, @@ -180,6 +201,7 @@ pub(crate) fn start_delete_comment( } Err(error) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: format!("comment delete failed: {}", error), }); @@ -198,10 +220,13 @@ pub(crate) fn start_update_labels( event_tx: Sender, labels_display: String, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::IssueUpdated { + repo: setup_repo, issue_number, message: format!("label update failed: {}", message), }, @@ -215,12 +240,14 @@ pub(crate) fn start_update_labels( match result { Ok(()) => { let _ = event_tx.send(AppEvent::IssueLabelsUpdated { + repo: event_repo.clone(), issue_number, labels: labels_display, }); } Err(error) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: format!("label update failed: {}", error), }); @@ -239,10 +266,13 @@ pub(crate) fn start_update_assignees( event_tx: Sender, assignees_display: String, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::IssueUpdated { + repo: setup_repo, issue_number, message: format!("assignee update failed: {}", message), }, @@ -256,12 +286,14 @@ pub(crate) fn start_update_assignees( match result { Ok(()) => { let _ = event_tx.send(AppEvent::IssueAssigneesUpdated { + repo: event_repo.clone(), issue_number, assignees: assignees_display, }); } Err(error) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: format!("assignee update failed: {}", error), }); @@ -278,10 +310,13 @@ pub(crate) fn start_reopen_issue( token: String, event_tx: Sender, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::IssueUpdated { + repo: setup_repo, issue_number, message: format!("reopen failed: {}", message), }, @@ -296,12 +331,14 @@ pub(crate) fn start_reopen_issue( match result { Ok(()) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: "reopened".to_string(), }); } Err(error) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: format!("reopen failed: {}", error), }); @@ -318,10 +355,13 @@ pub(crate) fn start_merge_pull_request( token: String, event_tx: Sender, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::IssueUpdated { + repo: setup_repo, issue_number: pull_number, message: format!("merge failed: {}", message), }, @@ -336,12 +376,14 @@ pub(crate) fn start_merge_pull_request( match result { Ok(()) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number: pull_number, message: "merged".to_string(), }); } Err(error) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number: pull_number, message: format!("merge failed: {}", error), }); @@ -359,15 +401,23 @@ pub(crate) fn start_close_issue( body: Option, event_tx: Sender, ) { + let event_repo = RepoIdentity::new(&owner, &repo); + let setup_repo = event_repo.clone(); spawn_with_services( token, event_tx, move |message| AppEvent::IssueUpdated { + repo: setup_repo, issue_number, message: format!("close failed: {}", message), }, move |services, event_tx| { let result: Result, anyhow::Error> = services.runtime.block_on(async { + services + .client + .close_issue(&owner, &repo, issue_number) + .await?; + let mut comment_error = None; if let Some(body) = body && let Err(error) = services @@ -378,29 +428,27 @@ pub(crate) fn start_close_issue( comment_error = Some(error.to_string()); } - services - .client - .close_issue(&owner, &repo, issue_number) - .await?; - Ok(comment_error) }); match result { Ok(Some(comment_error)) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: format!("closed (comment failed: {})", comment_error), }); } Ok(None) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: "closed".to_string(), }); } Err(error) => { let _ = event_tx.send(AppEvent::IssueUpdated { + repo: event_repo.clone(), issue_number, message: format!("close failed: {}", error), }); diff --git a/src/main_sync/pr_sync.rs b/src/main_sync/pr_sync.rs index 033872d..8fdf32a 100644 --- a/src/main_sync/pr_sync.rs +++ b/src/main_sync/pr_sync.rs @@ -31,15 +31,16 @@ pub(crate) fn start_pull_request_files_sync( } }; - let (pull_request_id, viewed_files) = services - .runtime - .block_on(async { - services - .client - .pull_request_file_view_state(&owner, &repo, issue_number) - .await - }) - .unwrap_or((None, HashSet::new())); + let view_state = services.runtime.block_on(async { + services + .client + .pull_request_file_view_state(&owner, &repo, issue_number) + .await + }); + let (pull_request_id, viewed_files, view_state_error) = match view_state { + Ok((pull_request_id, viewed_files)) => (pull_request_id, viewed_files, None), + Err(error) => (None, HashSet::new(), Some(error.to_string())), + }; let mapped = files .into_iter() @@ -56,6 +57,7 @@ pub(crate) fn start_pull_request_files_sync( files: mapped, pull_request_id, viewed_files, + view_state_error, }); }, ); diff --git a/src/main_sync/repo_sync.rs b/src/main_sync/repo_sync.rs index 9eac104..927d7f6 100644 --- a/src/main_sync/repo_sync.rs +++ b/src/main_sync/repo_sync.rs @@ -82,16 +82,24 @@ pub(crate) fn start_comment_sync( }; let now = comment_now_epoch(); - let mut count = 0usize; - for comment in comments { - let mut row = crate::sync::map_comment_to_row(issue_id, &comment); - row.last_accessed_at = Some(now); - let _ = crate::store::upsert_comment(&ctx.conn, &row); - count += 1; + let rows = comments + .iter() + .map(|comment| { + let mut row = crate::sync::map_comment_to_row(issue_id, comment); + row.last_accessed_at = Some(now); + row + }) + .collect::>(); + let count = rows.len(); + let result = replace_comments_for_issue(&ctx.conn, issue_id, &rows) + .and_then(|()| prune_comments(&ctx.conn, COMMENT_TTL_SECONDS, COMMENT_CAP)); + if let Err(error) = result { + let _ = event_tx.send(AppEvent::CommentsFailed { + issue_id, + message: error.to_string(), + }); + return; } - let _ = update_issue_comments_count(&ctx.conn, issue_id, count as i64); - let _ = touch_comments_for_issue(&ctx.conn, issue_id, now); - let _ = prune_comments(&ctx.conn, COMMENT_TTL_SECONDS, COMMENT_CAP); let _ = event_tx.send(AppEvent::CommentsUpdated { issue_id, count }); }, diff --git a/src/repo_index.rs b/src/repo_index.rs index 219a035..4518086 100644 --- a/src/repo_index.rs +++ b/src/repo_index.rs @@ -1,20 +1,35 @@ +use std::collections::HashSet; use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::Result; +use crate::discovery::is_git_repo; use crate::git::{RemoteInfo, list_github_remotes_at}; -use crate::store::{LocalRepoRow, upsert_local_repo}; +use crate::store::{ + LocalRepoRow, delete_local_repos_at_path, list_local_repos, replace_local_repos_for_path, +}; pub fn index_repo_path(conn: &rusqlite::Connection, path: &Path) -> Result { let remotes = list_github_remotes_at(path)?; let rows = build_local_repo_rows(path, remotes); - for row in &rows { - upsert_local_repo(conn, row)?; - } + replace_local_repos_for_path(conn, path.to_string_lossy().as_ref(), &rows)?; Ok(rows.len()) } +pub fn prune_missing_local_repos(conn: &rusqlite::Connection) -> Result { + let mut removed = 0usize; + let mut checked_paths = HashSet::new(); + for repo in list_local_repos(conn)? { + if !checked_paths.insert(repo.path.clone()) || is_git_repo(Path::new(&repo.path)) { + continue; + } + delete_local_repos_at_path(conn, &repo.path)?; + removed += 1; + } + Ok(removed) +} + fn build_local_repo_rows(path: &Path, remotes: Vec) -> Vec { let now = now_epoch(); remotes @@ -41,9 +56,9 @@ fn now_epoch() -> String { #[cfg(test)] mod tests { - use super::{build_local_repo_rows, index_repo_path}; + use super::{build_local_repo_rows, index_repo_path, prune_missing_local_repos}; use crate::git::{RemoteInfo, RepoSlug}; - use crate::store::{list_local_repos, open_db_at}; + use crate::store::{LocalRepoRow, list_local_repos, open_db_at, upsert_local_repo}; use std::fs; use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -97,6 +112,91 @@ mod tests { let _ = fs::remove_dir_all(&dir); } + #[test] + fn reindexing_replaces_removed_remotes() { + let dir = unique_temp_dir("replace"); + let repo_path = dir.join("repo"); + fs::create_dir_all(&repo_path).expect("create repo"); + init_git_repo(&repo_path); + run_git( + &repo_path, + &[ + "remote", + "add", + "origin", + "https://github.com/acme/blippy.git", + ], + ); + let db_path = dir.join("blippy.db"); + let conn = open_db_at(&db_path).expect("open db"); + index_repo_path(&conn, &repo_path).expect("initial index"); + + run_git(&repo_path, &["remote", "remove", "origin"]); + index_repo_path(&conn, &repo_path).expect("reindex"); + + assert!(list_local_repos(&conn).expect("list repos").is_empty()); + drop(conn); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn failed_reindex_preserves_cached_remotes() { + let dir = unique_temp_dir("failed-reindex"); + let repo_path = dir.join("repo"); + fs::create_dir_all(&repo_path).expect("create repo"); + fs::write( + repo_path.join(".git"), + "gitdir: /definitely/missing/blippy\n", + ) + .expect("write broken git file"); + let db_path = dir.join("blippy.db"); + let conn = open_db_at(&db_path).expect("open db"); + let cached = LocalRepoRow { + path: repo_path.to_string_lossy().to_string(), + remote_name: "origin".to_string(), + owner: "acme".to_string(), + repo: "blippy".to_string(), + url: "https://github.com/acme/blippy.git".to_string(), + last_seen: None, + last_scanned: None, + }; + upsert_local_repo(&conn, &cached).expect("cache remote"); + + assert!(index_repo_path(&conn, &repo_path).is_err()); + assert_eq!(list_local_repos(&conn).expect("list repos"), vec![cached]); + + drop(conn); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn pruning_removes_deleted_repository_paths() { + let dir = unique_temp_dir("prune"); + let repo_path = dir.join("repo"); + fs::create_dir_all(&repo_path).expect("create repo"); + init_git_repo(&repo_path); + run_git( + &repo_path, + &[ + "remote", + "add", + "origin", + "https://github.com/acme/blippy.git", + ], + ); + let db_path = dir.join("blippy.db"); + let conn = open_db_at(&db_path).expect("open db"); + index_repo_path(&conn, &repo_path).expect("index"); + fs::remove_dir_all(&repo_path).expect("remove repo"); + + let removed = prune_missing_local_repos(&conn).expect("prune"); + + assert_eq!(removed, 1); + assert!(list_local_repos(&conn).expect("list repos").is_empty()); + drop(conn); + let _ = fs::remove_dir_all(&dir); + } + fn unique_temp_dir(label: &str) -> PathBuf { let nanos = SystemTime::now() .duration_since(UNIX_EPOCH) diff --git a/src/store.rs b/src/store.rs index 42b5cc9..fa06a1f 100644 --- a/src/store.rs +++ b/src/store.rs @@ -2,7 +2,7 @@ use std::env; use std::path::{Path, PathBuf}; use std::time::Duration; -use anyhow::Result; +use anyhow::{Result, ensure}; use rusqlite::Connection; const DB_FILE_NAME: &str = "blippy.db"; @@ -135,7 +135,6 @@ pub fn upsert_issue(conn: &Connection, issue: &IssueRow) -> Result<()> { ), )?; - index_issue(conn, issue)?; Ok(()) } @@ -161,7 +160,25 @@ pub fn upsert_comment(conn: &Connection, comment: &CommentRow) -> Result<()> { ), )?; - index_comment(conn, comment)?; + Ok(()) +} + +pub fn replace_comments_for_issue( + conn: &Connection, + issue_id: i64, + comments: &[CommentRow], +) -> Result<()> { + ensure!( + comments.iter().all(|comment| comment.issue_id == issue_id), + "comment snapshot contains another issue" + ); + let transaction = conn.unchecked_transaction()?; + transaction.execute("DELETE FROM comments WHERE issue_id = ?1", [issue_id])?; + for comment in comments { + upsert_comment(&transaction, comment)?; + } + update_issue_comments_count(&transaction, issue_id, comments.len() as i64)?; + transaction.commit()?; Ok(()) } @@ -170,19 +187,11 @@ pub fn update_comment_body_by_id(conn: &Connection, comment_id: i64, body: &str) "UPDATE comments SET body = ?1 WHERE id = ?2", (body, comment_id), )?; - conn.execute( - "UPDATE fts_content SET body = ?1 WHERE comment_id = ?2", - (body, comment_id), - )?; Ok(()) } pub fn delete_comment_by_id(conn: &Connection, comment_id: i64) -> Result<()> { conn.execute("DELETE FROM comments WHERE id = ?1", [comment_id])?; - conn.execute( - "DELETE FROM fts_content WHERE comment_id = ?1", - [comment_id], - )?; Ok(()) } @@ -302,6 +311,29 @@ pub fn list_local_repos(conn: &Connection) -> Result> { Ok(repos) } +pub fn replace_local_repos_for_path( + conn: &Connection, + path: &str, + repos: &[LocalRepoRow], +) -> Result<()> { + ensure!( + repos.iter().all(|repo| repo.path == path), + "repository snapshot contains another path" + ); + let transaction = conn.unchecked_transaction()?; + transaction.execute("DELETE FROM local_repos WHERE path = ?1", [path])?; + for repo in repos { + upsert_local_repo(&transaction, repo)?; + } + transaction.commit()?; + Ok(()) +} + +pub fn delete_local_repos_at_path(conn: &Connection, path: &str) -> Result<()> { + conn.execute("DELETE FROM local_repos WHERE path = ?1", [path])?; + Ok(()) +} + pub fn get_repo_by_slug(conn: &Connection, owner: &str, repo: &str) -> Result> { let mut statement = conn.prepare( " @@ -377,41 +409,6 @@ pub fn comment_now_epoch() -> i64 { now as i64 } -fn index_issue(conn: &Connection, issue: &IssueRow) -> Result<()> { - conn.execute( - "DELETE FROM fts_content WHERE issue_id = ?1 AND comment_id IS NULL", - [issue.id], - )?; - conn.execute( - " - INSERT INTO fts_content (issue_id, comment_id, title, body, author) - VALUES (?1, NULL, ?2, ?3, NULL) - ", - (issue.id, issue.title.as_str(), issue.body.as_str()), - )?; - Ok(()) -} - -fn index_comment(conn: &Connection, comment: &CommentRow) -> Result<()> { - conn.execute( - "DELETE FROM fts_content WHERE comment_id = ?1", - [comment.id], - )?; - conn.execute( - " - INSERT INTO fts_content (issue_id, comment_id, title, body, author) - VALUES (?1, ?2, NULL, ?3, ?4) - ", - ( - comment.issue_id, - comment.id, - comment.body.as_str(), - comment.author.as_str(), - ), - )?; - Ok(()) -} - fn data_dir() -> PathBuf { if cfg!(windows) { return windows_data_dir(); @@ -512,14 +509,6 @@ fn apply_migrations(conn: &Connection) -> Result<()> { FOREIGN KEY(issue_id) REFERENCES issues(id) ON DELETE CASCADE ); - CREATE VIRTUAL TABLE IF NOT EXISTS fts_content USING fts5( - issue_id UNINDEXED, - comment_id UNINDEXED, - title, - body, - author - ); - CREATE TABLE IF NOT EXISTS local_repos ( path TEXT NOT NULL, remote_name TEXT NOT NULL, @@ -530,6 +519,8 @@ fn apply_migrations(conn: &Connection) -> Result<()> { last_scanned TEXT, PRIMARY KEY (path, remote_name) ); + + DROP TABLE IF EXISTS fts_content; ", )?; add_comment_accessed_column(conn)?; diff --git a/src/store/tests.rs b/src/store/tests.rs index 30b2b6a..d461948 100644 --- a/src/store/tests.rs +++ b/src/store/tests.rs @@ -1,7 +1,7 @@ use super::{ CommentRow, IssueRow, LocalRepoRow, RepoRow, comments_for_issue, delete_db_at, - get_repo_by_slug, list_issues, list_local_repos, open_db_at, upsert_comment, upsert_issue, - upsert_local_repo, upsert_repo, + get_repo_by_slug, list_issues, list_local_repos, open_db_at, replace_comments_for_issue, + replace_local_repos_for_path, upsert_comment, upsert_issue, upsert_local_repo, upsert_repo, }; use std::fs; use std::path::PathBuf; @@ -52,7 +52,25 @@ fn open_db_creates_tables() { assert!(table_exists(&conn, "repos")); assert!(table_exists(&conn, "issues")); assert!(table_exists(&conn, "comments")); - assert!(table_exists(&conn, "fts_content")); + assert!(!table_exists(&conn, "fts_content")); + drop(conn); + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn open_db_removes_legacy_search_index() { + let dir = unique_temp_dir("legacy-search-index"); + let db_path = dir.join("blippy.db"); + let conn = rusqlite::Connection::open(&db_path).expect("open raw db"); + conn.execute_batch( + "CREATE VIRTUAL TABLE fts_content USING fts5(issue_id, comment_id, title, body, author);", + ) + .expect("create legacy index"); + drop(conn); + + let conn = open_db_at(&db_path).expect("open db"); + + assert!(!table_exists(&conn, "fts_content")); drop(conn); let _ = fs::remove_dir_all(&dir); } @@ -215,6 +233,54 @@ fn comments_are_ordered_oldest_first() { let _ = fs::remove_dir_all(&dir); } +#[test] +fn replace_comments_for_issue_removes_comments_missing_from_snapshot() { + let dir = unique_temp_dir("comment-snapshot"); + let db_path = dir.join("blippy.db"); + let conn = open_db_at(&db_path).expect("open db"); + seed_issue(&conn, 20); + + let old_comment = test_comment(300, 20, "old"); + let kept_comment = test_comment(301, 20, "before"); + upsert_comment(&conn, &old_comment).expect("insert old comment"); + upsert_comment(&conn, &kept_comment).expect("insert kept comment"); + + let refreshed = CommentRow { + body: "after".to_string(), + ..kept_comment + }; + replace_comments_for_issue(&conn, 20, &[refreshed]).expect("replace comments"); + + let comments = comments_for_issue(&conn, 20).expect("list comments"); + assert_eq!(comments.len(), 1); + assert_eq!(comments[0].id, 301); + assert_eq!(comments[0].body, "after"); + + drop(conn); + let _ = fs::remove_dir_all(&dir); +} + +#[test] +fn replace_comments_for_issue_rejects_another_issues_comments() { + let dir = unique_temp_dir("comment-snapshot-mismatch"); + let db_path = dir.join("blippy.db"); + let conn = open_db_at(&db_path).expect("open db"); + seed_issue(&conn, 20); + let old_comment = test_comment(300, 20, "old"); + upsert_comment(&conn, &old_comment).expect("insert old comment"); + let wrong_comment = test_comment(301, 21, "wrong issue"); + + let result = replace_comments_for_issue(&conn, 20, &[wrong_comment]); + + assert!(result.is_err()); + assert_eq!( + comments_for_issue(&conn, 20).expect("list comments"), + vec![old_comment] + ); + drop(conn); + let _ = fs::remove_dir_all(&dir); +} + #[test] fn issues_are_ordered_newest_number_first() { let dir = unique_temp_dir("issue-order"); @@ -300,6 +366,26 @@ fn upsert_local_repo_inserts_and_updates() { let _ = fs::remove_dir_all(&dir); } +#[test] +fn replace_local_repos_for_path_removes_missing_remotes() { + let dir = unique_temp_dir("local-repo-snapshot"); + let db_path = dir.join("blippy.db"); + let conn = open_db_at(&db_path).expect("open db"); + let origin = test_local_repo("/tmp/repo", "origin"); + let upstream = test_local_repo("/tmp/repo", "upstream"); + upsert_local_repo(&conn, &origin).expect("insert origin"); + upsert_local_repo(&conn, &upstream).expect("insert upstream"); + + replace_local_repos_for_path(&conn, "/tmp/repo", &[origin]).expect("replace remotes"); + + let repos = list_local_repos(&conn).expect("list repos"); + assert_eq!(repos.len(), 1); + assert_eq!(repos[0].remote_name, "origin"); + + drop(conn); + let _ = fs::remove_dir_all(&dir); +} + #[test] fn get_repo_by_slug_returns_repo() { let dir = unique_temp_dir("repo-slug"); @@ -365,6 +451,60 @@ fn unique_temp_dir(label: &str) -> PathBuf { dir } +fn seed_issue(conn: &rusqlite::Connection, issue_id: i64) { + upsert_repo( + conn, + &RepoRow { + id: 1, + owner: "acme".to_string(), + name: "blippy".to_string(), + updated_at: None, + etag: None, + }, + ) + .expect("insert repo"); + upsert_issue( + conn, + &IssueRow { + id: issue_id, + repo_id: 1, + number: 1, + state: "open".to_string(), + title: "Issue".to_string(), + body: String::new(), + labels: String::new(), + assignees: String::new(), + comments_count: 0, + updated_at: None, + is_pr: false, + }, + ) + .expect("insert issue"); +} + +fn test_comment(id: i64, issue_id: i64, body: &str) -> CommentRow { + CommentRow { + id, + issue_id, + author: "dev".to_string(), + body: body.to_string(), + created_at: Some("2024-01-02T01:00:00Z".to_string()), + last_accessed_at: Some(1), + } +} + +fn test_local_repo(path: &str, remote_name: &str) -> LocalRepoRow { + LocalRepoRow { + path: path.to_string(), + remote_name: remote_name.to_string(), + owner: "acme".to_string(), + repo: "blippy".to_string(), + url: format!("https://github.com/acme/blippy-{}.git", remote_name), + last_seen: Some("2024-01-05T00:00:00Z".to_string()), + last_scanned: Some("2024-01-05T00:00:00Z".to_string()), + } +} + fn table_exists(conn: &rusqlite::Connection, name: &str) -> bool { let mut statement = conn .prepare("SELECT name FROM sqlite_master WHERE type='table' AND name=?1") diff --git a/src/sync.rs b/src/sync.rs index c5d26e0..4fafe03 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -9,6 +9,7 @@ pub struct SyncStats { pub issues: usize, pub comments: usize, pub not_modified: bool, + pub incomplete_reason: Option, } #[async_trait] @@ -136,10 +137,8 @@ where let mut stats = SyncStats::default(); let mut page = 1u32; let mut fetched_any_page = false; - let mut sync_completed = true; let mut latest_seen_updated_at = previous_cursor.clone(); let mut first_page_etag = None; - const PROGRESS_BATCH: usize = 10; loop { let if_none_match = if page == 1 { previous_etag.as_deref() @@ -166,7 +165,7 @@ where } Err(error) => { if fetched_any_page { - sync_completed = false; + stats.incomplete_reason = Some(error.to_string()); break; } return Err(error); @@ -178,8 +177,7 @@ where if issues.is_empty() { break; } - let mut persisted_since_update = 0usize; - let mut emitted_for_page = false; + let mut rows = Vec::new(); let mut reached_previous_cursor = false; for issue in issues { if let (Some(cursor), Some(issue_updated_at)) = @@ -204,25 +202,23 @@ where } } - crate::store::upsert_issue(_conn, &row)?; - stats.issues += 1; - persisted_since_update += 1; - if persisted_since_update >= PROGRESS_BATCH { - _on_progress(page, &stats); - emitted_for_page = true; - persisted_since_update = 0; - } + rows.push(row); } - if persisted_since_update > 0 || !emitted_for_page { - _on_progress(page, &stats); + + let transaction = _conn.unchecked_transaction()?; + for row in &rows { + crate::store::upsert_issue(&transaction, row)?; + stats.issues += 1; } + transaction.commit()?; + _on_progress(page, &stats); if reached_previous_cursor { break; } page += 1; } - if sync_completed { + if stats.incomplete_reason.is_none() { let next_cursor = latest_seen_updated_at .as_deref() .or(previous_cursor.as_deref()); diff --git a/src/sync/tests.rs b/src/sync/tests.rs index ee0ec60..10483d0 100644 --- a/src/sync/tests.rs +++ b/src/sync/tests.rs @@ -196,6 +196,7 @@ async fn sync_repo_inserts_issues_and_comments() { .await .expect("sync"); assert_eq!(stats.issues, 2); + assert_eq!(stats.incomplete_reason, None); assert_eq!(stats.comments, 0); let rows = list_issues(&conn, 1).expect("list issues"); @@ -354,6 +355,7 @@ async fn sync_repo_persists_partial_when_later_page_fails() { .await .expect("sync"); assert_eq!(stats.issues, 2); + assert_eq!(stats.incomplete_reason.as_deref(), Some("rate limit")); let rows = list_issues(&conn, 1).expect("list issues"); assert_eq!(rows.len(), 2); @@ -660,6 +662,7 @@ async fn sync_repo_does_not_advance_cursor_on_partial_failure() { .await .expect("sync"); assert_eq!(stats.issues, 1); + assert_eq!(stats.incomplete_reason.as_deref(), Some("rate limit")); assert!(!stats.not_modified); let stored_repo = get_repo_by_slug(&conn, "acme", "blippy") @@ -720,6 +723,7 @@ async fn sync_repo_keeps_partial_when_only_pull_requests_seen_before_failure() { .await .expect("sync"); assert_eq!(stats.issues, 1); + assert_eq!(stats.incomplete_reason.as_deref(), Some("rate limit")); drop(conn); let _ = fs::remove_dir_all(&dir); diff --git a/src/ui/ui_pull_request.rs b/src/ui/ui_pull_request.rs index 32a5698..2533f94 100644 --- a/src/ui/ui_pull_request.rs +++ b/src/ui/ui_pull_request.rs @@ -148,16 +148,22 @@ pub(super) fn draw_pull_request_files( "No changed files cached yet. Press r to refresh.", )] } else { + let view_state_loaded = app.pull_request_view_state_loaded(); app.pull_request_files() .iter() .map(|file| { let comment_count = app.pull_request_comments_count_for_path(file.filename.as_str()); let viewed = app.pull_request_file_is_viewed(file.filename.as_str()); + let viewed_marker = match (view_state_loaded, viewed) { + (false, _) => "?", + (true, true) => "✓", + (true, false) => "·", + }; ListItem::new(Line::from(vec![ Span::styled( - if viewed { "✓" } else { "·" }, - if viewed { + viewed_marker, + if view_state_loaded && viewed { Style::default() .fg(theme.accent_success) .add_modifier(Modifier::BOLD)