diff --git a/captures/ghzinga-issue-88499/large/manifest.json b/captures/ghzinga-issue-88499/large/manifest.json index c1880c09..ca5c5ff2 100644 --- a/captures/ghzinga-issue-88499/large/manifest.json +++ b/captures/ghzinga-issue-88499/large/manifest.json @@ -4,8 +4,8 @@ "title": "openai-responses provider: 404 on previous_response_id when store=false (default)", "mode": "issue", "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5", "config_path": "captures/ghzinga-issue-88499/capture-empty-config.toml", "requested_columns": 160, "requested_rows": 50, diff --git a/captures/ghzinga-issue-88499/manifest.json b/captures/ghzinga-issue-88499/manifest.json index feb7d8f4..701c02d0 100644 --- a/captures/ghzinga-issue-88499/manifest.json +++ b/captures/ghzinga-issue-88499/manifest.json @@ -3,8 +3,8 @@ "title": "openai-responses provider: 404 on previous_response_id when store=false (default)", "mode": "issue", "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5", "config_path": "captures/ghzinga-issue-88499/capture-empty-config.toml", "offline_fixture": "fixtures/issue-88499.json", "offline_resource_fixtures": [], diff --git a/captures/ghzinga-issue-88499/medium/manifest.json b/captures/ghzinga-issue-88499/medium/manifest.json index 35fe9703..966ed4e8 100644 --- a/captures/ghzinga-issue-88499/medium/manifest.json +++ b/captures/ghzinga-issue-88499/medium/manifest.json @@ -4,8 +4,8 @@ "title": "openai-responses provider: 404 on previous_response_id when store=false (default)", "mode": "issue", "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5", "config_path": "captures/ghzinga-issue-88499/capture-empty-config.toml", "requested_columns": 120, "requested_rows": 36, diff --git a/captures/ghzinga-issue-88499/mouse-smoke/manifest.json b/captures/ghzinga-issue-88499/mouse-smoke/manifest.json index ddb9fb99..244cbdae 100644 --- a/captures/ghzinga-issue-88499/mouse-smoke/manifest.json +++ b/captures/ghzinga-issue-88499/mouse-smoke/manifest.json @@ -5,7 +5,7 @@ "fixtures/issue-66943.json" ], "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", "config_path": "captures/ghzinga-issue-88499/mouse-smoke/capture-empty-config.toml", "command": "cd . && TERM=xterm-256color GZG_CONFIG_PATH=./captures/ghzinga-issue-88499/mouse-smoke/capture-empty-config.toml GZG_STATE_HOME=./captures/ghzinga-issue-88499/mouse-smoke/.capture-state GZG_CACHE_HOME=./captures/ghzinga-issue-88499/mouse-smoke/.capture-cache BROWSER=./captures/ghzinga-issue-88499/mouse-smoke/capture-open-url.sh GZG_COPY_COMMAND=./captures/ghzinga-issue-88499/mouse-smoke/capture-copy-url.sh ./target/debug/gzg https://github.com/openclaw/openclaw/issues/88499 --offline-fixture ./captures/ghzinga-issue-88499/mouse-smoke/navigation-fixture.json --offline-resource-fixture ./fixtures/issue-66943.json --no-restore --refresh-seconds 0", "actual_tmux_size": "120x36", @@ -120,5 +120,5 @@ "ansi": "70_mouse_quit_confirm.ansi" } ], - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd" + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5" } diff --git a/captures/ghzinga-issue-88499/narrow/manifest.json b/captures/ghzinga-issue-88499/narrow/manifest.json index 79ae092c..7275f8b2 100644 --- a/captures/ghzinga-issue-88499/narrow/manifest.json +++ b/captures/ghzinga-issue-88499/narrow/manifest.json @@ -4,8 +4,8 @@ "title": "openai-responses provider: 404 on previous_response_id when store=false (default)", "mode": "issue", "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5", "config_path": "captures/ghzinga-issue-88499/capture-empty-config.toml", "requested_columns": 80, "requested_rows": 24, diff --git a/captures/ghzinga-pr-81834/large/manifest.json b/captures/ghzinga-pr-81834/large/manifest.json index 6eada0d3..5b14620d 100644 --- a/captures/ghzinga-pr-81834/large/manifest.json +++ b/captures/ghzinga-pr-81834/large/manifest.json @@ -4,8 +4,8 @@ "title": "feat(senseaudio): add SenseAudio TTS provider", "mode": "pr", "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5", "config_path": "captures/ghzinga-pr-81834/capture-empty-config.toml", "requested_columns": 160, "requested_rows": 50, diff --git a/captures/ghzinga-pr-81834/manifest.json b/captures/ghzinga-pr-81834/manifest.json index 56af5227..5123ee96 100644 --- a/captures/ghzinga-pr-81834/manifest.json +++ b/captures/ghzinga-pr-81834/manifest.json @@ -3,8 +3,8 @@ "title": "feat(senseaudio): add SenseAudio TTS provider", "mode": "pr", "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5", "config_path": "captures/ghzinga-pr-81834/capture-empty-config.toml", "offline_fixture": null, "offline_resource_fixtures": [], diff --git a/captures/ghzinga-pr-81834/medium/manifest.json b/captures/ghzinga-pr-81834/medium/manifest.json index 54b76dc5..9c02c443 100644 --- a/captures/ghzinga-pr-81834/medium/manifest.json +++ b/captures/ghzinga-pr-81834/medium/manifest.json @@ -4,8 +4,8 @@ "title": "feat(senseaudio): add SenseAudio TTS provider", "mode": "pr", "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5", "config_path": "captures/ghzinga-pr-81834/capture-empty-config.toml", "requested_columns": 120, "requested_rows": 36, diff --git a/captures/ghzinga-pr-81834/mouse-smoke/manifest.json b/captures/ghzinga-pr-81834/mouse-smoke/manifest.json index 2f438e05..20f5162b 100644 --- a/captures/ghzinga-pr-81834/mouse-smoke/manifest.json +++ b/captures/ghzinga-pr-81834/mouse-smoke/manifest.json @@ -5,7 +5,7 @@ "fixtures/issue-66943.json" ], "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", "config_path": "captures/ghzinga-pr-81834/mouse-smoke/capture-empty-config.toml", "command": "cd . && TERM=xterm-256color GZG_CONFIG_PATH=./captures/ghzinga-pr-81834/mouse-smoke/capture-empty-config.toml GZG_STATE_HOME=./captures/ghzinga-pr-81834/mouse-smoke/.capture-state GZG_CACHE_HOME=./captures/ghzinga-pr-81834/mouse-smoke/.capture-cache BROWSER=./captures/ghzinga-pr-81834/mouse-smoke/capture-open-url.sh GZG_COPY_COMMAND=./captures/ghzinga-pr-81834/mouse-smoke/capture-copy-url.sh ./target/debug/gzg 'openclaw/openclaw#81834' --offline-fixture ./captures/ghzinga-pr-81834/mouse-smoke/navigation-fixture.json --offline-resource-fixture ./fixtures/issue-66943.json --no-restore --refresh-seconds 0", "actual_tmux_size": "120x36", @@ -240,5 +240,5 @@ ], "load_full_fixture": "captures/ghzinga-pr-81834/mouse-smoke/load-full-fixture.json", "load_full_command": "cd . && TERM=xterm-256color GZG_CONFIG_PATH=./captures/ghzinga-pr-81834/mouse-smoke/capture-empty-config.toml GZG_STATE_HOME=./captures/ghzinga-pr-81834/mouse-smoke/.capture-state GZG_CACHE_HOME=./captures/ghzinga-pr-81834/mouse-smoke/.capture-cache ./target/debug/gzg 'openclaw/openclaw#81834' --offline-fixture ./captures/ghzinga-pr-81834/mouse-smoke/load-full-fixture.json --no-restore --refresh-seconds 0", - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd" + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5" } diff --git a/captures/ghzinga-pr-81834/narrow/manifest.json b/captures/ghzinga-pr-81834/narrow/manifest.json index 0c1490c1..c43b1787 100644 --- a/captures/ghzinga-pr-81834/narrow/manifest.json +++ b/captures/ghzinga-pr-81834/narrow/manifest.json @@ -4,8 +4,8 @@ "title": "feat(senseaudio): add SenseAudio TTS provider", "mode": "pr", "binary": "target/debug/gzg", - "git_commit": "647de6cf9925745eaf082a04dc768699136321cc", - "app_tree_hash": "afdd8d97ceb69f0ceb415ca4f7a5021351f557a780fc6aaf9e865616e58280bd", + "git_commit": "586817dfe1f49e03f66cb4912a6a1a0f07c199f8", + "app_tree_hash": "0fea15dad5b7fe9ed5d986b2bb142246d52f2add690ab0c490d1595b124f8fc5", "config_path": "captures/ghzinga-pr-81834/capture-empty-config.toml", "requested_columns": 80, "requested_rows": 24, diff --git a/src/app/state.rs b/src/app/state.rs index b23690ad..24e1e558 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -2,8 +2,8 @@ use std::{collections::HashSet, fmt, path::PathBuf, str::FromStr}; use crate::app::composer::CommentComposer; use crate::domain::{ - available_actions, ActionKind, MergeMethod, PullRequest, Resource, ResourceAction, ResourceId, - ResourceIdError, ResourceKind, + available_actions, body_edit_target, editable_targets, ActionKind, EditTarget, MergeMethod, + PullRequest, Resource, ResourceAction, ResourceId, ResourceIdError, ResourceKind, }; use crate::input::HitArea; use crate::render::{ @@ -126,6 +126,7 @@ pub struct ResourceTabState { pub last_error: Option, pub status_message: Option, pub comment_composer: Option, + pub composer_target: Option, } impl ResourceTabState { @@ -148,6 +149,7 @@ impl ResourceTabState { last_error: None, status_message: None, comment_composer: None, + composer_target: None, } } @@ -177,6 +179,7 @@ impl ResourceTabState { last_error: None, status_message: None, comment_composer: None, + composer_target: None, } } } @@ -223,11 +226,24 @@ impl ActionConfirmState { .merge_methods .get(self.selected_method) .map(|method| ResourceAction::Merge { method: *method }), - ActionKind::Comment => None, + ActionKind::Comment | ActionKind::Edit => None, } } } +/// One editable object offered by the edit picker. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EditChoice { + pub target: EditTarget, + pub label: String, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct EditPickerState { + pub choices: Vec, + pub selected: usize, +} + #[derive(Debug, Clone)] pub struct AppState { pub resource: Resource, @@ -242,7 +258,11 @@ pub struct AppState { pub resource_link_prompt: Option, pub action_menu: Option, pub action_confirm: Option, + pub edit_picker: Option, pub comment_composer: Option, + /// When set, the open composer edits this object instead of posting a + /// new comment. Tab-scoped together with the composer. + pub composer_target: Option, pub pending_action: Option, /// Post-action refreshes that could not start immediately because a /// fetch was already in flight; drained by the runner once idle. @@ -294,7 +314,9 @@ impl AppState { resource_link_prompt: None, action_menu: None, action_confirm: None, + edit_picker: None, comment_composer: None, + composer_target: None, pending_action: None, pending_action_refreshes: Vec::new(), pending_activity_focus: None, @@ -484,8 +506,80 @@ impl AppState { self.comment_composer = Some(CommentComposer::new()); } + pub fn open_edit_picker(&mut self) { + let choices: Vec = editable_targets(&self.resource) + .into_iter() + .map(|target| EditChoice { + label: self.edit_choice_label(&target), + target, + }) + .collect(); + if choices.is_empty() { + self.status_message = Some("nothing editable on this resource".into()); + return; + } + self.close_all_prompts(); + self.edit_picker = Some(EditPickerState { + choices, + selected: 0, + }); + } + + fn edit_choice_label(&self, target: &EditTarget) -> String { + if body_edit_target(&self.resource).as_ref() == Some(target) { + return "Description".to_string(); + } + self.resource + .activity + .iter() + .find(|entry| entry.edit.as_ref() == Some(target)) + .map(|entry| { + let snippet: String = entry.body.chars().take(40).collect(); + let snippet = snippet.replace('\n', " "); + format!("{}: {snippet}", entry.author) + }) + .unwrap_or_else(|| "comment".to_string()) + } + + pub fn close_edit_picker(&mut self) { + self.edit_picker = None; + } + + pub fn move_edit_picker_selection(&mut self, delta: isize) { + if let Some(picker) = &mut self.edit_picker { + let last = picker.choices.len().saturating_sub(1); + picker.selected = picker.selected.saturating_add_signed(delta).min(last); + } + } + + pub fn select_edit_choice(&mut self, index: usize) { + if let Some(picker) = &mut self.edit_picker { + if index < picker.choices.len() { + picker.selected = index; + } + } + } + + pub fn selected_edit_target(&self) -> Option { + let picker = self.edit_picker.as_ref()?; + picker + .choices + .get(picker.selected) + .map(|choice| choice.target.clone()) + } + + /// Open the composer prefilled with the target's current text. + pub fn open_edit_composer(&mut self, target: EditTarget) { + self.close_all_prompts(); + let mut composer = CommentComposer::new(); + composer.insert_str(&target.current_body); + self.comment_composer = Some(composer); + self.composer_target = Some(target); + } + pub fn close_comment_composer(&mut self) { self.comment_composer = None; + self.composer_target = None; } /// Esc/cancel semantics for the composer: an empty composer closes, @@ -496,6 +590,7 @@ impl AppState { }; if composer.is_empty() || composer.confirm_discard { self.comment_composer = None; + self.composer_target = None; } else { composer.confirm_discard = true; } @@ -508,7 +603,9 @@ impl AppState { self.resource_link_prompt = None; self.action_menu = None; self.action_confirm = None; + self.edit_picker = None; self.comment_composer = None; + self.composer_target = None; self.quit_confirmation = false; self.show_help = false; self.show_settings = false; @@ -524,7 +621,22 @@ impl AppState { /// unrelated draft (another tab, or a replacement) stays editable. pub fn composer_is_posting(&self) -> bool { match (&self.pending_action, &self.comment_composer) { - (Some(ResourceAction::Comment { body }), Some(composer)) => composer.body() == *body, + (Some(action), Some(composer)) => { + self.composer_matches_action(action) + && action + .draft_body() + .is_some_and(|body| composer.body() == body) + } + _ => false, + } + } + + /// An open composer belongs to a pending action only when they agree on + /// what is being written: a new comment, or an edit of the same target. + fn composer_matches_action(&self, action: &ResourceAction) -> bool { + match action { + ResourceAction::Comment { .. } => self.composer_target.is_none(), + ResourceAction::Edit { target, .. } => self.composer_target.as_ref() == Some(target), _ => false, } } @@ -534,15 +646,17 @@ impl AppState { match error { None => { self.status_message = Some(format!("{}, refreshing", action.success_label())); - if let ResourceAction::Comment { body } = action { + if let Some(body) = action.draft_body() { // Close only the composer whose draft was posted; a // replacement draft opened meanwhile must survive. - let holds_posted_draft = self - .comment_composer - .as_ref() - .is_some_and(|composer| composer.body() == *body); + let holds_posted_draft = self.composer_matches_action(action) + && self + .comment_composer + .as_ref() + .is_some_and(|composer| composer.body() == body); if holds_posted_draft { self.comment_composer = None; + self.composer_target = None; } } } @@ -1325,6 +1439,7 @@ impl AppState { tab.last_error = self.last_error.clone(); tab.status_message = self.status_message.clone(); tab.comment_composer = self.comment_composer.clone(); + tab.composer_target = self.composer_target.clone(); } } @@ -1354,6 +1469,7 @@ impl AppState { self.last_error = tab.last_error; self.status_message = tab.status_message; self.comment_composer = tab.comment_composer; + self.composer_target = tab.composer_target; self.hit_areas.clear(); self.scrollbar_drag = None; self.pending_activity_focus = None; @@ -1529,6 +1645,7 @@ mod tests { fn activity_entry(id: &str) -> ActivityEntry { ActivityEntry { + edit: None, id: id.into(), kind: ActivityKind::Comment, author: "alice".into(), @@ -1799,6 +1916,7 @@ mod tests { refreshed.updated_at = "later".into(); refreshed.body = "Changed body".into(); refreshed.activity.push(crate::domain::ActivityEntry { + edit: None, id: "timeline-1".into(), kind: crate::domain::ActivityKind::Timeline, author: "alice".into(), @@ -2189,4 +2307,25 @@ mod tests { assert_eq!(state.status_message.as_deref(), Some("closing\u{2026}")); assert!(state.last_error.is_none()); } + + #[test] + fn select_edit_choice_ignores_out_of_range_indexes() { + let mut resource = crate::test_fixtures::issue_resource(1, "Issue"); + resource.actions.node_id = "I_node".into(); + resource.actions.viewer_can_update = true; + let mut state = AppState::new(resource); + state.open_edit_picker(); + state.select_edit_choice(0); + let count = state + .edit_picker + .as_ref() + .map(|p| p.choices.len()) + .expect("picker open"); + state.select_edit_choice(count); + assert_eq!( + state.edit_picker.as_ref().map(|p| p.selected), + Some(0), + "an index one past the end must not move the selection" + ); + } } diff --git a/src/app/update.rs b/src/app/update.rs index 507a3a8f..60b69861 100644 --- a/src/app/update.rs +++ b/src/app/update.rs @@ -74,6 +74,9 @@ fn apply_key(state: &mut AppState, key: KeyEvent) -> AppIntent { if state.action_confirm.is_some() { return apply_action_confirm_key(state, key); } + if state.edit_picker.is_some() { + return apply_edit_picker_key(state, key); + } if state.action_menu.is_some() { return apply_action_menu_key(state, key); } @@ -423,7 +426,13 @@ fn submit_composer_comment(state: &mut AppState) -> AppIntent { let Some(composer) = &state.comment_composer else { return AppIntent::None; }; - if composer.is_empty() { + // Descriptions may be cleared; comments and reviews need text (the + // server rejects empty ones anyway, this is just the friendlier error). + let empty_allowed = matches!( + state.composer_target.as_ref().map(|target| target.kind), + Some(crate::domain::EditKind::IssueBody) | Some(crate::domain::EditKind::PullRequestBody) + ); + if composer.is_empty() && !empty_allowed { state.status_message = Some("comment is empty".into()); return AppIntent::None; } @@ -431,9 +440,15 @@ fn submit_composer_comment(state: &mut AppState) -> AppIntent { state.status_message = Some("an action is already in flight".into()); return AppIntent::None; } - AppIntent::SubmitAction(ResourceAction::Comment { - body: composer.body(), - }) + let body = composer.body(); + let action = match &state.composer_target { + Some(target) => ResourceAction::Edit { + target: target.clone(), + body, + }, + None => ResourceAction::Comment { body }, + }; + AppIntent::SubmitAction(action) } fn apply_action_menu_key(state: &mut AppState, key: KeyEvent) -> AppIntent { @@ -475,6 +490,7 @@ fn activate_action_by_shortcut(state: &mut AppState, ch: char) -> AppIntent { .unwrap_or_default(); let kind = match ch { 'c' => Some(ActionKind::Comment), + 'e' => Some(ActionKind::Edit), 'm' => Some(ActionKind::Merge), 'x' => Some(ActionKind::Close), 'o' => Some(ActionKind::Reopen), @@ -489,6 +505,7 @@ fn activate_action_by_shortcut(state: &mut AppState, ch: char) -> AppIntent { fn activate_action_kind(state: &mut AppState, kind: ActionKind) -> AppIntent { match kind { ActionKind::Comment => state.open_comment_composer(), + ActionKind::Edit => state.open_edit_picker(), ActionKind::Close | ActionKind::Reopen | ActionKind::Merge => { state.open_action_confirm(kind) } @@ -496,6 +513,53 @@ fn activate_action_kind(state: &mut AppState, kind: ActionKind) -> AppIntent { AppIntent::None } +fn apply_edit_picker_key(state: &mut AppState, key: KeyEvent) -> AppIntent { + match key.code { + KeyCode::Esc | KeyCode::Char('q') if is_plain_shortcut(key) => { + state.close_edit_picker(); + AppIntent::None + } + KeyCode::Char('c') if key.modifiers.contains(KeyModifiers::CONTROL) => { + state.close_edit_picker(); + AppIntent::None + } + KeyCode::Up | KeyCode::Char('k') if is_plain_shortcut(key) => { + state.move_edit_picker_selection(-1); + AppIntent::None + } + KeyCode::Down | KeyCode::Char('j') if is_plain_shortcut(key) => { + state.move_edit_picker_selection(1); + AppIntent::None + } + KeyCode::Char(ch @ '1'..='9') if is_plain_shortcut(key) => { + state.select_edit_choice(ch as usize - '1' as usize); + AppIntent::None + } + KeyCode::Enter if is_plain_shortcut(key) => activate_selected_edit_choice(state), + _ => AppIntent::None, + } +} + +fn activate_selected_edit_choice(state: &mut AppState) -> AppIntent { + if let Some(target) = state.selected_edit_target() { + state.open_edit_composer(target); + } + AppIntent::None +} + +fn activate_edit_for_entry(state: &mut AppState, node_id: &str) -> AppIntent { + let target = state + .resource + .activity + .iter() + .filter_map(|entry| entry.edit.clone()) + .find(|target| target.node_id == node_id); + if let Some(target) = target { + state.open_edit_composer(target); + } + AppIntent::None +} + fn apply_action_confirm_key(state: &mut AppState, key: KeyEvent) -> AppIntent { match key.code { KeyCode::Esc | KeyCode::Char('n') | KeyCode::Char('q') if is_plain_shortcut(key) => { @@ -646,6 +710,7 @@ fn apply_mouse_scroll(state: &mut AppState, direction: ScrollDirection) { || state.resource_link_prompt.is_some() || state.action_menu.is_some() || state.action_confirm.is_some() + || state.edit_picker.is_some() { return; } @@ -739,6 +804,7 @@ fn apply_target(state: &mut AppState, target: HitTarget) -> AppIntent { state.close_quit_confirmation(); state.close_action_menu(); state.close_action_confirm(); + state.close_edit_picker(); AppIntent::None } HitTarget::OpenActionMenu => { @@ -760,6 +826,17 @@ fn apply_target(state: &mut AppState, target: HitTarget) -> AppIntent { state.close_action_confirm(); AppIntent::None } + HitTarget::EditPickerItem(index) => { + state.select_edit_choice(index); + activate_selected_edit_choice(state) + } + HitTarget::EditActivityEntry { node_id } => activate_edit_for_entry(state, &node_id), + HitTarget::EditResourceBody => { + if let Some(target) = crate::domain::body_edit_target(&state.resource) { + state.open_edit_composer(target); + } + AppIntent::None + } HitTarget::SelectMergeMethod(index) => { state.select_merge_method(index); AppIntent::None @@ -954,6 +1031,7 @@ mod tests { fn activity_entry(id: &str, url: &str) -> ActivityEntry { ActivityEntry { + edit: None, id: id.into(), kind: ActivityKind::Comment, author: "alice".into(), @@ -2640,7 +2718,10 @@ mod tests { let intent = press(&mut state, KeyCode::Char('A')); assert_eq!(intent, AppIntent::None); let menu = state.action_menu.as_ref().expect("action menu open"); - assert_eq!(menu.actions, vec![ActionKind::Comment, ActionKind::Close]); + assert_eq!( + menu.actions, + vec![ActionKind::Comment, ActionKind::Edit, ActionKind::Close] + ); } #[test] @@ -2659,6 +2740,7 @@ mod tests { let mut state = actionable_issue_state(); press(&mut state, KeyCode::Char('A')); press(&mut state, KeyCode::Down); + press(&mut state, KeyCode::Down); let intent = press(&mut state, KeyCode::Enter); assert_eq!(intent, AppIntent::None); assert!(state.action_menu.is_none()); @@ -2834,7 +2916,7 @@ mod tests { let mut state = actionable_issue_state(); apply_event(&mut state, AppEvent::Activate(HitTarget::OpenActionMenu)); assert!(state.action_menu.is_some()); - apply_event(&mut state, AppEvent::Activate(HitTarget::ActionMenuItem(1))); + apply_event(&mut state, AppEvent::Activate(HitTarget::ActionMenuItem(2))); assert_eq!( state.action_confirm.as_ref().map(|confirm| confirm.kind), Some(ActionKind::Close) @@ -3336,4 +3418,257 @@ mod tests { assert_eq!(state.scroll, content_scroll); assert_eq!(state.comment_composer.as_ref().map(|c| c.scroll), Some(3)); } + + fn state_with_editable_comment() -> AppState { + let mut state = actionable_issue_state(); + state.resource.body = "original description".into(); + state.resource.activity.push(crate::domain::ActivityEntry { + id: "IC_1".into(), + edit: Some(crate::domain::EditTarget { + node_id: "IC_1".into(), + kind: crate::domain::EditKind::IssueComment, + current_body: "my comment".into(), + }), + kind: crate::domain::ActivityKind::Comment, + author: "me".into(), + body: "my comment".into(), + updated_at: "now".into(), + path: None, + line: None, + url: None, + author_association: None, + reactions: Default::default(), + includes_created_edit: false, + is_minimized: false, + minimized_reason: None, + thread_id: None, + thread_resolved: None, + thread_outdated: None, + }); + state + } + + #[test] + fn menu_edit_shortcut_opens_the_picker() { + let mut state = state_with_editable_comment(); + press(&mut state, KeyCode::Char('A')); + press(&mut state, KeyCode::Char('e')); + let picker = state.edit_picker.as_ref().expect("picker open"); + assert_eq!(picker.choices.len(), 2); + assert_eq!(picker.choices[0].label, "Description"); + assert!(picker.choices[1].label.starts_with("me: my comment")); + } + + #[test] + fn picker_enter_opens_a_prefilled_edit_composer() { + let mut state = state_with_editable_comment(); + state.open_edit_picker(); + press(&mut state, KeyCode::Down); + press(&mut state, KeyCode::Enter); + assert!(state.edit_picker.is_none()); + assert_eq!(composer_body(&state), "my comment"); + assert_eq!( + state.composer_target.as_ref().map(|t| t.node_id.clone()), + Some("IC_1".into()) + ); + } + + #[test] + fn picker_numbers_select_and_modified_keys_are_ignored() { + let mut state = state_with_editable_comment(); + state.open_edit_picker(); + press(&mut state, KeyCode::Char('2')); + assert_eq!(state.edit_picker.as_ref().map(|p| p.selected), Some(1)); + let alt = |code| AppEvent::Key(KeyEvent::new(code, KeyModifiers::ALT)); + apply_event(&mut state, alt(KeyCode::Char('1'))); + assert_eq!( + state.edit_picker.as_ref().map(|p| p.selected), + Some(1), + "alt-1 must not move the picker selection" + ); + apply_event(&mut state, alt(KeyCode::Char('k'))); + assert_eq!(state.edit_picker.as_ref().map(|p| p.selected), Some(1)); + apply_event(&mut state, alt(KeyCode::Enter)); + assert!( + state.edit_picker.is_some(), + "alt-enter must not activate a picker choice" + ); + press(&mut state, KeyCode::Esc); + assert!(state.edit_picker.is_none()); + } + + #[test] + fn edit_composer_submits_an_edit_action() { + let mut state = state_with_editable_comment(); + state.open_edit_picker(); + press(&mut state, KeyCode::Down); + press(&mut state, KeyCode::Enter); + press(&mut state, KeyCode::Char('!')); + let intent = press_ctrl(&mut state, 's'); + assert_eq!( + intent, + AppIntent::SubmitAction(ResourceAction::Edit { + target: crate::domain::EditTarget { + node_id: "IC_1".into(), + kind: crate::domain::EditKind::IssueComment, + current_body: "my comment".into(), + }, + body: "my comment!".into(), + }) + ); + } + + #[test] + fn inline_edit_targets_open_the_right_composer() { + let mut state = state_with_editable_comment(); + apply_event( + &mut state, + AppEvent::Activate(HitTarget::EditActivityEntry { + node_id: "IC_1".into(), + }), + ); + assert_eq!(composer_body(&state), "my comment"); + state.close_comment_composer(); + + apply_event(&mut state, AppEvent::Activate(HitTarget::EditResourceBody)); + assert_eq!(composer_body(&state), "original description"); + assert_eq!( + state.composer_target.as_ref().map(|t| t.kind), + Some(crate::domain::EditKind::IssueBody) + ); + } + + #[test] + fn cancelling_an_edit_clears_the_target() { + let mut state = state_with_editable_comment(); + apply_event(&mut state, AppEvent::Activate(HitTarget::EditResourceBody)); + press(&mut state, KeyCode::Esc); + press(&mut state, KeyCode::Esc); + assert!(state.comment_composer.is_none()); + assert!(state.composer_target.is_none()); + } + + #[test] + fn edit_draft_is_frozen_while_saving() { + let mut state = state_with_editable_comment(); + apply_event(&mut state, AppEvent::Activate(HitTarget::EditResourceBody)); + state.pending_action = Some(ResourceAction::Edit { + target: crate::domain::EditTarget { + node_id: "I_node".into(), + kind: crate::domain::EditKind::IssueBody, + current_body: "original description".into(), + }, + body: "original description".into(), + }); + press(&mut state, KeyCode::Char('!')); + assert_eq!(composer_body(&state), "original description"); + } + + #[test] + fn description_edits_may_be_submitted_empty_but_comments_may_not() { + let mut state = state_with_editable_comment(); + apply_event(&mut state, AppEvent::Activate(HitTarget::EditResourceBody)); + if let Some(composer) = &mut state.comment_composer { + *composer = crate::app::CommentComposer::new(); + } + let intent = press_ctrl(&mut state, 's'); + assert!( + matches!( + intent, + AppIntent::SubmitAction(ResourceAction::Edit { ref body, .. }) if body.is_empty() + ), + "clearing a description is a valid edit: {intent:?}" + ); + + let mut state = state_with_editable_comment(); + state.open_comment_composer(); + let intent = press_ctrl(&mut state, 's'); + assert_eq!(intent, AppIntent::None); + assert_eq!(state.status_message.as_deref(), Some("comment is empty")); + } + + #[test] + fn wheel_does_not_scroll_content_behind_the_edit_picker() { + let mut state = state_with_editable_comment(); + state.scroll_limit = 100; + state.open_edit_picker(); + apply_event( + &mut state, + AppEvent::Mouse(MouseEvent { + kind: MouseEventKind::ScrollDown, + column: 0, + row: 0, + modifiers: KeyModifiers::empty(), + }), + ); + assert_eq!(state.scroll, 0); + } + + #[test] + fn an_identical_edit_elsewhere_does_not_freeze_or_close_this_composer() { + let mut state = state_with_editable_comment(); + state.pending_action = Some(ResourceAction::Edit { + target: crate::domain::EditTarget { + node_id: "OTHER".into(), + kind: crate::domain::EditKind::IssueComment, + current_body: "same text".into(), + }, + body: "same text".into(), + }); + apply_event( + &mut state, + AppEvent::Activate(HitTarget::EditActivityEntry { + node_id: "IC_1".into(), + }), + ); + if let Some(composer) = &mut state.comment_composer { + *composer = crate::app::CommentComposer::new(); + composer.insert_str("same text"); + } + press(&mut state, KeyCode::Char('!')); + assert_eq!( + composer_body(&state), + "same text!", + "a different edit target with identical text must stay editable" + ); + } + + #[test] + fn picker_plain_keys_navigate_and_modified_keys_never_close() { + let mut state = state_with_editable_comment(); + state.open_edit_picker(); + press(&mut state, KeyCode::Char('j')); + assert_eq!(state.edit_picker.as_ref().map(|p| p.selected), Some(1)); + press(&mut state, KeyCode::Char('k')); + assert_eq!(state.edit_picker.as_ref().map(|p| p.selected), Some(0)); + let alt = |code| AppEvent::Key(KeyEvent::new(code, KeyModifiers::ALT)); + apply_event(&mut state, alt(KeyCode::Char('q'))); + assert!( + state.edit_picker.is_some(), + "alt-q must not close the picker" + ); + apply_event(&mut state, alt(KeyCode::Char('j'))); + assert_eq!( + state.edit_picker.as_ref().map(|p| p.selected), + Some(0), + "alt-j must not move the picker selection" + ); + press(&mut state, KeyCode::Char('c')); + assert!( + state.edit_picker.is_some(), + "plain c is not a picker cancel key" + ); + press_ctrl(&mut state, 'c'); + assert!(state.edit_picker.is_none(), "ctrl-c closes the picker"); + } + + #[test] + fn picker_digit_one_selects_the_first_choice() { + let mut state = state_with_editable_comment(); + state.open_edit_picker(); + press(&mut state, KeyCode::Char('2')); + assert_eq!(state.edit_picker.as_ref().map(|p| p.selected), Some(1)); + press(&mut state, KeyCode::Char('1')); + assert_eq!(state.edit_picker.as_ref().map(|p| p.selected), Some(0)); + } } diff --git a/src/domain/action.rs b/src/domain/action.rs index 826a3d16..33d87dc1 100644 --- a/src/domain/action.rs +++ b/src/domain/action.rs @@ -32,10 +32,32 @@ impl fmt::Display for MergeMethod { } } +/// What kind of GitHub object an edit rewrites; picks the update mutation. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum EditKind { + IssueBody, + PullRequestBody, + IssueComment, + Review, + ReviewComment, +} + +/// An editable GitHub object the viewer is allowed to update, per the +/// server's own `viewerCanUpdate`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct EditTarget { + pub node_id: String, + pub kind: EditKind, + /// The object's current raw markdown, used to prefill the composer. + pub current_body: String, +} + /// A menu-level action choice before any payload is collected. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ActionKind { Comment, + Edit, Merge, Close, Reopen, @@ -45,6 +67,7 @@ impl fmt::Display for ActionKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Comment => f.write_str("Comment"), + Self::Edit => f.write_str("Edit"), Self::Merge => f.write_str("Merge"), Self::Close => f.write_str("Close"), Self::Reopen => f.write_str("Reopen"), @@ -56,6 +79,7 @@ impl fmt::Display for ActionKind { #[derive(Debug, Clone, PartialEq, Eq)] pub enum ResourceAction { Comment { body: String }, + Edit { target: EditTarget, body: String }, Close, Reopen, Merge { method: MergeMethod }, @@ -65,6 +89,7 @@ impl ResourceAction { pub fn progress_label(&self) -> &'static str { match self { Self::Comment { .. } => "commenting", + Self::Edit { .. } => "saving edit", Self::Close => "closing", Self::Reopen => "reopening", Self::Merge { .. } => "merging", @@ -74,11 +99,49 @@ impl ResourceAction { pub fn success_label(&self) -> &'static str { match self { Self::Comment { .. } => "comment posted", + Self::Edit { .. } => "edit saved", Self::Close => "closed", Self::Reopen => "reopened", Self::Merge { .. } => "merged", } } + + /// The draft text a composer submitted, when this action carries one. + pub fn draft_body(&self) -> Option<&str> { + match self { + Self::Comment { body } | Self::Edit { body, .. } => Some(body), + _ => None, + } + } +} + +/// The resource description as an edit target, when the viewer may edit it. +pub fn body_edit_target(resource: &Resource) -> Option { + if resource.actions.node_id.is_empty() || !resource.actions.viewer_can_update { + return None; + } + let kind = match resource.kind() { + ResourceKind::Issue => EditKind::IssueBody, + ResourceKind::PullRequest => EditKind::PullRequestBody, + }; + Some(EditTarget { + node_id: resource.actions.node_id.clone(), + kind, + current_body: resource.body.clone(), + }) +} + +/// Everything the viewer may edit right now: the description first, then +/// editable activity entries in display order. +pub fn editable_targets(resource: &Resource) -> Vec { + let mut targets: Vec = body_edit_target(resource).into_iter().collect(); + targets.extend( + resource + .activity + .iter() + .filter_map(|entry| entry.edit.clone()), + ); + targets } /// Actions the viewer can take on this resource right now. Server-side @@ -91,6 +154,9 @@ pub fn available_actions(resource: &Resource) -> Vec { if !resource.actions.locked { actions.push(ActionKind::Comment); } + if !editable_targets(resource).is_empty() { + actions.push(ActionKind::Edit); + } if resource.actions.viewer_can_update { match resource.state.as_str() { "OPEN" => { @@ -146,11 +212,11 @@ mod tests { } #[test] - fn open_issue_offers_comment_and_close() { + fn open_issue_offers_comment_edit_and_close() { let resource = actionable_issue(); assert_eq!( available_actions(&resource), - vec![ActionKind::Comment, ActionKind::Close] + vec![ActionKind::Comment, ActionKind::Edit, ActionKind::Close] ); } @@ -160,16 +226,21 @@ mod tests { resource.state = "CLOSED".into(); assert_eq!( available_actions(&resource), - vec![ActionKind::Comment, ActionKind::Reopen] + vec![ActionKind::Comment, ActionKind::Edit, ActionKind::Reopen] ); } #[test] - fn open_pr_offers_comment_merge_and_close() { + fn open_pr_offers_comment_edit_merge_and_close() { let resource = actionable_pr(); assert_eq!( available_actions(&resource), - vec![ActionKind::Comment, ActionKind::Merge, ActionKind::Close] + vec![ + ActionKind::Comment, + ActionKind::Edit, + ActionKind::Merge, + ActionKind::Close + ] ); } @@ -177,7 +248,10 @@ mod tests { fn merged_pr_offers_only_comment() { let mut resource = actionable_pr(); resource.state = "MERGED".into(); - assert_eq!(available_actions(&resource), vec![ActionKind::Comment]); + assert_eq!( + available_actions(&resource), + vec![ActionKind::Comment, ActionKind::Edit] + ); } #[test] @@ -190,7 +264,7 @@ mod tests { .allowed_merge_methods = Vec::new(); assert_eq!( available_actions(&resource), - vec![ActionKind::Comment, ActionKind::Close] + vec![ActionKind::Comment, ActionKind::Edit, ActionKind::Close] ); } @@ -198,7 +272,10 @@ mod tests { fn locked_resource_hides_comment() { let mut resource = actionable_issue(); resource.actions.locked = true; - assert_eq!(available_actions(&resource), vec![ActionKind::Close]); + assert_eq!( + available_actions(&resource), + vec![ActionKind::Edit, ActionKind::Close] + ); } #[test] @@ -226,4 +303,68 @@ mod tests { assert_eq!(comment.progress_label(), "commenting"); assert_eq!(comment.success_label(), "comment posted"); } + + #[test] + fn body_edit_target_requires_permission() { + let mut resource = actionable_issue(); + let target = body_edit_target(&resource).expect("editable body"); + assert_eq!(target.kind, EditKind::IssueBody); + assert_eq!(target.node_id, "I_node"); + assert_eq!(target.current_body, resource.body); + resource.actions.viewer_can_update = false; + assert!(body_edit_target(&resource).is_none()); + } + + #[test] + fn editable_targets_list_description_then_entries() { + let mut resource = actionable_issue(); + let comment_target = EditTarget { + node_id: "IC_1".into(), + kind: EditKind::IssueComment, + current_body: "a comment".into(), + }; + resource.activity.push(crate::domain::ActivityEntry { + id: "IC_1".into(), + edit: Some(comment_target.clone()), + kind: crate::domain::ActivityKind::Comment, + author: "bob".into(), + body: "a comment".into(), + updated_at: "now".into(), + path: None, + line: None, + url: None, + author_association: None, + reactions: Default::default(), + includes_created_edit: false, + is_minimized: false, + minimized_reason: None, + thread_id: None, + thread_resolved: None, + thread_outdated: None, + }); + let targets = editable_targets(&resource); + assert_eq!(targets.len(), 2); + assert_eq!(targets[0].kind, EditKind::IssueBody); + assert_eq!(targets[1], comment_target); + } + + #[test] + fn draft_body_exists_only_for_text_actions() { + assert_eq!( + ResourceAction::Comment { body: "x".into() }.draft_body(), + Some("x") + ); + let edit = ResourceAction::Edit { + target: EditTarget { + node_id: "n".into(), + kind: EditKind::IssueComment, + current_body: "old".into(), + }, + body: "new".into(), + }; + assert_eq!(edit.draft_body(), Some("new")); + assert_eq!(edit.progress_label(), "saving edit"); + assert_eq!(edit.success_label(), "edit saved"); + assert_eq!(ResourceAction::Close.draft_body(), None); + } } diff --git a/src/domain/mod.rs b/src/domain/mod.rs index afed723a..04bc333e 100644 --- a/src/domain/mod.rs +++ b/src/domain/mod.rs @@ -1,7 +1,10 @@ pub mod action; pub mod resource; -pub use action::{available_actions, ActionKind, MergeMethod, ResourceAction}; +pub use action::{ + available_actions, body_edit_target, editable_targets, ActionKind, EditKind, EditTarget, + MergeMethod, ResourceAction, +}; pub use resource::{ ActionContext, ActivityEntry, ActivityKind, ChangedFile, CheckCounts, CheckRun, CheckStatus, Commit, Deployment, MetadataItem, PullRequest, ReactionCounts, Resource, ResourceId, diff --git a/src/domain/resource.rs b/src/domain/resource.rs index 3e1d249b..40492c52 100644 --- a/src/domain/resource.rs +++ b/src/domain/resource.rs @@ -545,6 +545,9 @@ pub struct PullRequest { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ActivityEntry { pub id: String, + /// Present when the viewer may edit this entry (GraphQL viewerCanUpdate). + #[serde(default)] + pub edit: Option, #[serde(default)] pub kind: ActivityKind, pub author: String, @@ -913,6 +916,7 @@ mod tests { reactions: ReactionCounts::default(), body: "body".into(), activity: vec![ActivityEntry { + edit: None, id: "comment".into(), kind: ActivityKind::Comment, author: "bob".into(), diff --git a/src/fetch.rs b/src/fetch.rs index 774313ae..edefd745 100644 --- a/src/fetch.rs +++ b/src/fetch.rs @@ -1949,4 +1949,71 @@ mod tests { "completion messages must not leak onto the navigated-to resource" ); } + + #[tokio::test] + async fn successful_edit_closes_its_composer_and_refreshes() { + let mut state = actionable_state(); + let target = crate::domain::EditTarget { + node_id: "IC_1".into(), + kind: crate::domain::EditKind::IssueComment, + current_body: "old".into(), + }; + state.open_edit_composer(target.clone()); + if let Some(composer) = &mut state.comment_composer { + composer.insert_str(" text"); + } + let action = ResourceAction::Edit { + target, + body: "old text".into(), + }; + state.begin_action_submission(&action); + let outcome = MutationOutcome { + action, + target: state.resource.id.clone(), + origin_tab_id: state.active_resource_tab_id(), + result: Ok(()), + }; + + let application = deliver_outcome(&mut state, outcome); + + assert!(application.refresh_started); + assert!(state.comment_composer.is_none()); + assert!(state.composer_target.is_none()); + assert_eq!( + state.status_message.as_deref(), + Some("edit saved, refreshing") + ); + } + + #[tokio::test] + async fn failed_edit_keeps_the_draft_and_target() { + let mut state = actionable_state(); + let target = crate::domain::EditTarget { + node_id: "IC_1".into(), + kind: crate::domain::EditKind::IssueComment, + current_body: "old".into(), + }; + state.open_edit_composer(target.clone()); + let action = ResourceAction::Edit { + target, + body: "old".into(), + }; + state.begin_action_submission(&action); + let outcome = MutationOutcome { + action, + target: state.resource.id.clone(), + origin_tab_id: state.active_resource_tab_id(), + result: Err(anyhow::anyhow!("was deleted meanwhile")), + }; + + deliver_outcome(&mut state, outcome); + + assert!(state.comment_composer.is_some(), "draft survives failure"); + assert!(state.composer_target.is_some()); + assert!(state + .last_error + .as_deref() + .expect("error surfaced") + .contains("saving edit failed")); + } } diff --git a/src/github/api.rs b/src/github/api.rs index 241835cf..b9c7c467 100644 --- a/src/github/api.rs +++ b/src/github/api.rs @@ -11,8 +11,8 @@ use serde_json::{json, Value}; use crate::domain::{ ActionContext, ActivityEntry, ActivityKind, ChangedFile, CheckRun, CheckStatus, Commit, - Deployment, MergeMethod, MetadataItem, PullRequest, ReactionCounts, Resource, ResourceId, - ResourceKind, FILE_PATCH_CONTEXT_UNAVAILABLE_WARNING, FULL_DEPTH_WARNING_HINT, + Deployment, EditKind, MergeMethod, MetadataItem, PullRequest, ReactionCounts, Resource, + ResourceId, ResourceKind, FILE_PATCH_CONTEXT_UNAVAILABLE_WARNING, FULL_DEPTH_WARNING_HINT, }; use crate::github::transport::{run_graphql_query, run_rest_get, GITHUB_GRAPHQL_URL}; use crate::github::{ @@ -1153,6 +1153,8 @@ struct TotalCountDto { #[serde(rename_all = "camelCase")] struct CommentDto { id: Option, + #[serde(default)] + viewer_can_update: bool, author: Option, author_association: Option, body: String, @@ -1202,6 +1204,8 @@ struct CommentsConnection { #[serde(rename_all = "camelCase")] struct ReviewDto { id: Option, + #[serde(default)] + viewer_can_update: bool, author: Option, author_association: Option, body: Option, @@ -1340,6 +1344,8 @@ struct CommitCommentThreadCommentsConnection { #[serde(rename_all = "camelCase")] struct ReviewThreadCommentDto { id: Option, + #[serde(default)] + viewer_can_update: bool, author: Option, author_association: Option, body: String, @@ -3045,11 +3051,35 @@ fn reaction_counts_from_value(value: Option<&Value>) -> ReactionCounts { .unwrap_or_default() } +/// Editable objects need a real GraphQL node id and server-side permission; +/// everything else renders read-only. +fn edit_target_for( + node_id: &Option, + viewer_can_update: bool, + kind: EditKind, + current_body: &str, +) -> Option { + match node_id { + Some(id) if viewer_can_update => Some(crate::domain::EditTarget { + node_id: id.clone(), + kind, + current_body: current_body.to_string(), + }), + _ => None, + } +} + fn comments_to_activity(comments: Vec) -> Vec { comments .into_iter() .enumerate() .map(|(index, comment)| ActivityEntry { + edit: edit_target_for( + &comment.id, + comment.viewer_can_update, + EditKind::IssueComment, + &comment.body, + ), id: comment.id.unwrap_or_else(|| format!("comment-{index}")), kind: ActivityKind::Comment, author: display_author(comment.author), @@ -4052,6 +4082,12 @@ fn review_thread_to_activity(thread: ReviewThreadDto) -> Vec { let mut entries = Vec::new(); for comment in thread.comments.nodes { entries.push(ActivityEntry { + edit: edit_target_for( + &comment.id, + comment.viewer_can_update, + EditKind::ReviewComment, + &comment.body, + ), id: comment.id.unwrap_or_else(|| { format!( "review-comment-{}-{}", @@ -4176,6 +4212,7 @@ fn commit_comment_thread_to_activity(index: usize, node: &Value) -> Vec Vec ActivityEntry { ActivityEntry { + edit: None, id: string_field(node, "id") .map(str::to_string) .unwrap_or_else(|| format!("commit-comment-thread-{index}")), @@ -4237,6 +4275,7 @@ fn timeline_node_to_activity(index: usize, node: &Value) -> ActivityEntry { .map(str::to_string) .unwrap_or_else(|| "unknown".to_string()); ActivityEntry { + edit: None, id: string_field(node, "id") .map(str::to_string) .unwrap_or_else(|| synthetic_timeline_id(index, typename, &updated_at, &author, &body)), @@ -4763,13 +4802,19 @@ fn reviews_to_activity(reviews: Vec) -> Vec { .enumerate() .map(|(index, review)| { let state = review.state.unwrap_or_else(|| "REVIEW".to_string()); - let body = review.body.unwrap_or_default(); - let body = if body.trim().is_empty() { + let raw_body = review.body.unwrap_or_default(); + let body = if raw_body.trim().is_empty() { state.clone() } else { - format!("{state}: {body}") + format!("{state}: {raw_body}") }; ActivityEntry { + edit: edit_target_for( + &review.id, + review.viewer_can_update, + EditKind::Review, + &raw_body, + ), id: review.id.unwrap_or_else(|| format!("review-{index}")), kind: ActivityKind::Review, author: display_author(review.author), @@ -6208,6 +6253,7 @@ mod tests { end_cursor: Some("comment-cursor-2".into()), }, nodes: vec![CommentDto { + viewer_can_update: false, id: Some("comment-1".into()), author: Some(UserDto { login: Some("alice".into()), @@ -6262,6 +6308,7 @@ mod tests { end_cursor: Some("review-cursor-2".into()), }, nodes: vec![ReviewDto { + viewer_can_update: false, id: Some("review-1".into()), author: Some(UserDto { login: Some("bob".into()), @@ -6318,6 +6365,7 @@ mod tests { body: "body".into(), activity: vec![ ActivityEntry { + edit: None, id: "old-comment".into(), kind: ActivityKind::Comment, author: "alice".into(), @@ -6336,6 +6384,7 @@ mod tests { thread_outdated: None, }, ActivityEntry { + edit: None, id: "review".into(), kind: ActivityKind::Review, author: "bob".into(), @@ -6363,6 +6412,7 @@ mod tests { replace_comment_activity( &mut resource, vec![ActivityEntry { + edit: None, id: "new-comment".into(), kind: ActivityKind::Comment, author: "carol".into(), @@ -6398,6 +6448,7 @@ mod tests { fn replace_review_activity_keeps_comments_review_comments_and_timeline() { fn activity(id: &str, kind: ActivityKind) -> ActivityEntry { ActivityEntry { + edit: None, id: id.into(), kind, author: "alice".into(), @@ -7664,6 +7715,7 @@ diff --git a/docs/two.md b/docs/two.md\n\ fn pr_activity_includes_reviews_with_state() { let activity = pr_activity( vec![CommentDto { + viewer_can_update: false, id: Some("comment".into()), author: Some(UserDto { login: Some("alice".into()), @@ -7683,6 +7735,7 @@ diff --git a/docs/two.md b/docs/two.md\n\ }], }], vec![ReviewDto { + viewer_can_update: false, id: Some("review".into()), author: Some(UserDto { login: Some("bob".into()), @@ -7736,6 +7789,7 @@ diff --git a/docs/two.md b/docs/two.md\n\ end_cursor: None, }, nodes: vec![ReviewThreadCommentDto { + viewer_can_update: false, id: Some("review-comment".into()), author: Some(UserDto { login: Some("reviewer".into()), @@ -7801,6 +7855,7 @@ diff --git a/docs/two.md b/docs/two.md\n\ end_cursor: None, }, nodes: vec![ReviewThreadCommentDto { + viewer_can_update: false, id: Some("review-comment-2".into()), author: Some(UserDto { login: Some("maintainer".into()), @@ -7847,6 +7902,7 @@ diff --git a/docs/two.md b/docs/two.md\n\ end_cursor: Some("comment-cursor-2".into()), }, nodes: vec![ReviewThreadCommentDto { + viewer_can_update: false, id: Some("review-comment-3".into()), author: Some(UserDto { login: Some("reviewer".into()), @@ -8448,4 +8504,78 @@ diff --git a/docs/two.md b/docs/two.md\n\ let all_off = serde_json::json!({"data": {"repository": {}}}); assert!(allowed_merge_methods_from_repository(&all_off).is_empty()); } + + fn comment_dto_base() -> CommentDto { + CommentDto { + id: None, + viewer_can_update: false, + author: None, + author_association: None, + body: String::new(), + created_at: None, + updated_at: None, + url: None, + includes_created_edit: None, + is_minimized: None, + minimized_reason: None, + reaction_groups: Vec::new(), + } + } + + #[test] + fn comment_edit_targets_require_id_and_permission() { + let editable = CommentDto { + id: Some("IC_1".into()), + viewer_can_update: true, + body: "mine".into(), + ..comment_dto_base() + }; + let readonly = CommentDto { + id: Some("IC_2".into()), + viewer_can_update: false, + body: "theirs".into(), + ..comment_dto_base() + }; + let synthetic = CommentDto { + id: None, + viewer_can_update: true, + body: "orphan".into(), + ..comment_dto_base() + }; + + let entries = comments_to_activity(vec![editable, readonly, synthetic]); + + let edit = entries[0].edit.as_ref().expect("editable comment"); + assert_eq!(edit.node_id, "IC_1"); + assert_eq!(edit.kind, EditKind::IssueComment); + assert_eq!(edit.current_body, "mine"); + assert!(entries[1].edit.is_none(), "no permission, no edit target"); + assert!(entries[2].edit.is_none(), "no node id, no edit target"); + } + + #[test] + fn review_edit_targets_carry_the_raw_body_not_the_display_body() { + let review = ReviewDto { + id: Some("PRR_1".into()), + viewer_can_update: true, + author: None, + author_association: None, + body: Some("looks good".into()), + state: Some("APPROVED".into()), + submitted_at: None, + updated_at: Some("2026-01-01T00:00:00Z".into()), + url: None, + reaction_groups: Vec::new(), + }; + + let entries = reviews_to_activity(vec![review]); + + assert_eq!(entries[0].body, "APPROVED: looks good"); + let edit = entries[0].edit.as_ref().expect("editable review"); + assert_eq!(edit.kind, EditKind::Review); + assert_eq!( + edit.current_body, "looks good", + "editing must start from the raw review body, not the display prefix" + ); + } } diff --git a/src/github/mutations.rs b/src/github/mutations.rs index 691a8bbf..321f09aa 100644 --- a/src/github/mutations.rs +++ b/src/github/mutations.rs @@ -47,6 +47,10 @@ fn action_mutation( "mutation($id: ID!, $body: String!) { addComment(input: {subjectId: $id, body: $body}) { clientMutationId } }", json!({ "id": node_id, "body": body }), ), + ResourceAction::Edit { target, body } => ( + edit_mutation(target.kind), + json!({ "id": target.node_id, "body": body }), + ), ResourceAction::Close => (state_mutation(kind, StateChange::Close), id_variables(node_id)), ResourceAction::Reopen => ( state_mutation(kind, StateChange::Reopen), @@ -86,6 +90,27 @@ fn id_variables(node_id: &str) -> Value { json!({ "id": node_id }) } +fn edit_mutation(kind: crate::domain::EditKind) -> &'static str { + use crate::domain::EditKind; + match kind { + EditKind::IssueBody => { + "mutation($id: ID!, $body: String!) { updateIssue(input: {id: $id, body: $body}) { clientMutationId } }" + } + EditKind::PullRequestBody => { + "mutation($id: ID!, $body: String!) { updatePullRequest(input: {pullRequestId: $id, body: $body}) { clientMutationId } }" + } + EditKind::IssueComment => { + "mutation($id: ID!, $body: String!) { updateIssueComment(input: {id: $id, body: $body}) { clientMutationId } }" + } + EditKind::Review => { + "mutation($id: ID!, $body: String!) { updatePullRequestReview(input: {pullRequestReviewId: $id, body: $body}) { clientMutationId } }" + } + EditKind::ReviewComment => { + "mutation($id: ID!, $body: String!) { updatePullRequestReviewComment(input: {pullRequestReviewCommentId: $id, body: $body}) { clientMutationId } }" + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -133,6 +158,45 @@ mod tests { } } + #[test] + fn edit_mutations_pick_the_kind_specific_update() { + use crate::domain::{EditKind, EditTarget}; + let cases = [ + (EditKind::IssueBody, "updateIssue(input: {id:"), + ( + EditKind::PullRequestBody, + "updatePullRequest(input: {pullRequestId:", + ), + (EditKind::IssueComment, "updateIssueComment(input: {id:"), + ( + EditKind::Review, + "updatePullRequestReview(input: {pullRequestReviewId:", + ), + ( + EditKind::ReviewComment, + "updatePullRequestReviewComment(input: {pullRequestReviewCommentId:", + ), + ]; + for (kind, expected) in cases { + let action = ResourceAction::Edit { + target: EditTarget { + node_id: "NODE".into(), + kind, + current_body: "old text".into(), + }, + body: "updated text".into(), + }; + let (mutation, variables) = + action_mutation("unused-subject", ResourceKind::PullRequest, &action); + assert!( + mutation.contains(expected), + "expected `{expected}` for {kind:?}: {mutation}" + ); + assert_eq!(variables["id"], "NODE"); + assert_eq!(variables["body"], "updated text"); + } + } + #[test] fn merge_mutation_carries_uppercase_method() { let (mutation, variables) = action_mutation( diff --git a/src/github/public_rest.rs b/src/github/public_rest.rs index 69322859..83a33b62 100644 --- a/src/github/public_rest.rs +++ b/src/github/public_rest.rs @@ -1060,6 +1060,7 @@ fn rest_pr_metadata(pr: &RestPullDto) -> Vec { fn rest_comment_activity((index, comment): (usize, RestCommentDto)) -> ActivityEntry { let includes_created_edit = comment.updated_at != comment.created_at; ActivityEntry { + edit: None, id: format!("rest-comment-{}", comment.id), kind: ActivityKind::Comment, author: display_rest_author(comment.user), @@ -1093,6 +1094,7 @@ fn rest_review_activity((index, review): (usize, RestReviewDto)) -> ActivityEntr format!("{state}: {body}") }; ActivityEntry { + edit: None, id: format!("rest-review-{}", review.id), kind: ActivityKind::Review, author: display_rest_author(review.user), @@ -1115,6 +1117,7 @@ fn rest_review_activity((index, review): (usize, RestReviewDto)) -> ActivityEntr fn rest_review_comment_activity((index, comment): (usize, RestReviewCommentDto)) -> ActivityEntry { let includes_created_edit = comment.updated_at != comment.created_at; ActivityEntry { + edit: None, id: format!("rest-review-comment-{}", comment.id), kind: ActivityKind::ReviewComment, author: display_rest_author(comment.user), @@ -1152,6 +1155,7 @@ fn rest_timeline_event_activity( } let body = rest_timeline_event_body(event_name, &event); Some(ActivityEntry { + edit: None, id: format!( "rest-timeline-{}", event diff --git a/src/github/queries.rs b/src/github/queries.rs index 15809533..010ce727 100644 --- a/src/github/queries.rs +++ b/src/github/queries.rs @@ -110,6 +110,7 @@ query($owner: String!, $name: String!, $number: Int!) { pageInfo { hasNextPage } nodes { id + viewerCanUpdate author { login } authorAssociation body @@ -126,6 +127,7 @@ query($owner: String!, $name: String!, $number: Int!) { pageInfo { hasNextPage } nodes { id + viewerCanUpdate author { login } authorAssociation body @@ -189,6 +191,7 @@ query($owner: String!, $name: String!, $number: Int!) { pageInfo { hasNextPage } nodes { id + viewerCanUpdate author { login } authorAssociation body @@ -448,6 +451,7 @@ query($owner: String!, $name: String!, $number: Int!, $after: String) {{ }} nodes {{ id + viewerCanUpdate author {{ login }} authorAssociation body @@ -552,6 +556,7 @@ query($owner: String!, $name: String!, $number: Int!, $after: String) { } nodes { id + viewerCanUpdate author { login } authorAssociation body @@ -589,6 +594,7 @@ query($owner: String!, $name: String!, $number: Int!, $after: String) { } nodes { id + viewerCanUpdate author { login } authorAssociation body @@ -620,6 +626,7 @@ query($threadId: ID!, $after: String) { } nodes { id + viewerCanUpdate author { login } authorAssociation body @@ -655,6 +662,7 @@ query($threadId: ID!, $after: String) { } nodes { id + viewerCanUpdate author { login } authorAssociation body @@ -817,6 +825,7 @@ pub(crate) fn timeline_query(kind: ResourceKind) -> String { } nodes { id + viewerCanUpdate author { login } authorAssociation body @@ -1313,3 +1322,24 @@ fn selector(kind: ResourceKind) -> &'static str { ResourceKind::Issue => "issue", } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn commit_comment_thread_comments_query_selects_the_expected_fields() { + let query = commit_comment_thread_comments_query(); + for expected in [ + "PullRequestCommitCommentThread", + "comments(first: 100, after: $after)", + "viewerCanUpdate", + "reactionGroups", + ] { + assert!( + query.contains(expected), + "commit comment paging query must select `{expected}`" + ); + } + } +} diff --git a/src/input/hit.rs b/src/input/hit.rs index a17ebde6..9f6b9780 100644 --- a/src/input/hit.rs +++ b/src/input/hit.rs @@ -31,6 +31,9 @@ pub enum HitTarget { ActionMenuItem(usize), ConfirmAction, CancelAction, + EditPickerItem(usize), + EditActivityEntry { node_id: String }, + EditResourceBody, SelectMergeMethod(usize), ComposerSubmit, ComposerCancel, @@ -56,6 +59,8 @@ impl HitTarget { matches!( self, Self::ToggleBlock(_) + | Self::EditActivityEntry { .. } + | Self::EditResourceBody | Self::ExpandBlocks(_) | Self::CollapseBlocks(_) | Self::ResourceLink { .. } diff --git a/src/render/resource.rs b/src/render/resource.rs index c84decb9..a759bea9 100644 --- a/src/render/resource.rs +++ b/src/render/resource.rs @@ -245,6 +245,8 @@ pub fn render_app(frame: &mut Frame<'_>, state: &mut AppState) { render_comment_composer_modal(frame, rects.area, state, &palette); } else if state.action_confirm.is_some() { render_action_confirm_modal(frame, rects.area, state, &palette); + } else if state.edit_picker.is_some() { + render_edit_picker_modal(frame, rects.area, state, &palette); } else if state.action_menu.is_some() { render_action_menu_modal(frame, rects.area, state, &palette); } @@ -371,6 +373,7 @@ fn modal_hint_line(text: &str, palette: &Palette) -> Line<'static> { fn action_kind_shortcut(kind: ActionKind) -> char { match kind { ActionKind::Comment => 'c', + ActionKind::Edit => 'e', ActionKind::Merge => 'm', ActionKind::Close => 'x', ActionKind::Reopen => 'o', @@ -429,6 +432,79 @@ fn render_action_menu_modal( .render(inner, frame.buffer_mut()); } +fn render_edit_picker_modal( + frame: &mut Frame<'_>, + area: Rect, + state: &mut AppState, + palette: &Palette, +) { + let Some(picker) = state.edit_picker.clone() else { + return; + }; + let height = (picker.choices.len() as u16).saturating_add(4); + let Some(inner) = open_action_modal_frame( + frame, + area, + state, + palette, + (40, 70), + (height, height), + palette.accent, + ) else { + return; + }; + // The modal may be clamped by the terminal; window the choices so the + // selection is always drawn and every drawn row is clickable. + let visible = usize::from(inner.height.saturating_sub(2)).max(1); + let window_start = picker + .selected + .saturating_add(1) + .saturating_sub(visible) + .min(picker.choices.len().saturating_sub(visible)); + let mut rows = vec![modal_title_line("Edit what?".to_string(), palette)]; + for (offset, (index, choice)) in picker + .choices + .iter() + .enumerate() + .skip(window_start) + .take(visible) + .enumerate() + { + let hint = if index < 9 { + format!("{}", index + 1) + } else { + " ".to_string() + }; + let label = format!(" {:<60}{hint} ", choice.label); + let style = if index == picker.selected { + Style::default() + .fg(palette.panel_bg) + .bg(palette.accent) + .add_modifier(Modifier::BOLD) + } else { + Style::default().fg(palette.text).bg(palette.surface0) + }; + rows.push(Line::from(Span::styled( + fit_label_to_width(&label, inner.width), + style, + ))); + let row_y = inner.y.saturating_add(1).saturating_add(offset as u16); + state.hit_areas.push(HitArea::new( + Rect::new(inner.x, row_y, inner.width, 1), + HitTarget::EditPickerItem(index), + )); + } + let position_hint = format!( + "{}/{} up/down move enter edit esc close", + picker.selected + 1, + picker.choices.len() + ); + rows.push(modal_hint_line(&position_hint, palette)); + Paragraph::new(rows) + .style(Style::default().fg(palette.text).bg(palette.surface0)) + .render(inner, frame.buffer_mut()); +} + fn render_action_confirm_modal( frame: &mut Frame<'_>, area: Rect, @@ -535,7 +611,20 @@ fn render_comment_composer_modal( state: &mut AppState, palette: &Palette, ) { - let title = format!("Comment on {}", state.resource.id.canonical_name()); + let title = match &state.composer_target { + Some(target) + if crate::domain::body_edit_target(&state.resource).as_ref() == Some(target) => + { + format!("Edit description of {}", state.resource.id.canonical_name()) + } + Some(_) => format!("Edit comment on {}", state.resource.id.canonical_name()), + None => format!("Comment on {}", state.resource.id.canonical_name()), + }; + let submit_label = if state.composer_target.is_some() { + "[save ctrl+s]" + } else { + "[comment ctrl+s]" + }; let posting = state.composer_is_posting(); let Some(inner) = open_action_modal_frame( frame, @@ -573,7 +662,7 @@ fn render_comment_composer_modal( rows.push(composer_hint_line(layout.confirm_discard, posting, palette)); let buttons = vec![ ( - "[comment ctrl+s]".to_string(), + submit_label.to_string(), HitTarget::ComposerSubmit, ModalButtonTone::Primary, ), @@ -2511,6 +2600,13 @@ fn push_body_timeline_rows( ), heading_style(palette), )); + if crate::domain::body_edit_target(resource).is_some() { + rows.push(ContentRow::target_styled( + "[edit]", + HitTarget::EditResourceBody, + link_style(palette), + )); + } if resource.body.trim().is_empty() { rows.push(ContentRow::styled( "No description provided.", @@ -2608,6 +2704,9 @@ fn push_activity_timeline_rows( if let Some(url) = &entry.url { rows.push(activity_detail_row(url, resource, palette)); } + if let Some(edit_row) = activity_edit_row(entry, palette) { + rows.push(edit_row); + } if expanded { if let Some(url) = &entry.url { rows.push(linkable_text_row(format!("url: {url}"), resource)); @@ -2647,6 +2746,19 @@ fn activity_detail_row(url: &str, resource: &Resource, palette: &Palette) -> Con ContentRow::target_styled("[details]", target, link_style(palette)) } +/// A standalone clickable row offered on entries the viewer may edit. +fn activity_edit_row(entry: &ActivityEntry, palette: &Palette) -> Option { + entry.edit.as_ref().map(|target| { + ContentRow::target_styled( + "[edit]", + HitTarget::EditActivityEntry { + node_id: target.node_id.clone(), + }, + link_style(palette), + ) + }) +} + fn activity_heading_style(entry: &ActivityEntry, palette: &Palette) -> Style { let color = match entry.kind { crate::domain::ActivityKind::Comment => palette.teal, @@ -2727,6 +2839,9 @@ fn activity_rows(state: &mut AppState, width: usize, palette: &Palette) -> Vec AppState { + let mut state = actionable_pr_state(); + state.resource.body = "the description".into(); + if let Some(entry) = state.resource.activity.first_mut() { + entry.edit = Some(crate::domain::EditTarget { + node_id: "IC_9".into(), + kind: crate::domain::EditKind::IssueComment, + current_body: entry.body.clone(), + }); + } + state + } + + #[test] + fn edit_rows_appear_only_where_the_viewer_may_edit() { + let mut state = AppState::new(pr_resource()); + let content = draw_actionable(&mut state); + assert!(!content.contains("[edit]")); + + let mut state = editable_pr_state(); + draw_actionable(&mut state); + assert!(state + .hit_areas + .iter() + .any(|area| area.target == HitTarget::EditResourceBody)); + let intent = + click_rendered_target(&mut state, |target| *target == HitTarget::EditResourceBody); + assert_eq!(intent, AppIntent::None); + assert_eq!( + state + .comment_composer + .as_ref() + .map(|composer| composer.body()), + Some("the description".into()) + ); + } + + #[test] + fn activity_edit_row_opens_the_entry_composer() { + let mut state = editable_pr_state(); + state.set_tab(crate::app::Tab::Activity); + draw_actionable(&mut state); + let intent = click_rendered_target(&mut state, |target| { + matches!(target, HitTarget::EditActivityEntry { .. }) + }); + assert_eq!(intent, AppIntent::None); + assert!(state.comment_composer.is_some()); + assert_eq!( + state.composer_target.as_ref().map(|t| t.node_id.clone()), + Some("IC_9".into()) + ); + } + + #[test] + fn edit_picker_modal_lists_choices_and_click_edits() { + let mut state = editable_pr_state(); + state.open_edit_picker(); + let content = draw_actionable(&mut state); + assert!(content.contains("Edit what?")); + assert!(content.contains("Description")); + click_rendered_target(&mut state, |target| *target == HitTarget::EditPickerItem(0)); + assert!(state.edit_picker.is_none()); + assert_eq!( + state + .comment_composer + .as_ref() + .map(|composer| composer.body()), + Some("the description".into()) + ); + } + + #[test] + fn edit_composer_shows_edit_title_and_save_button() { + let mut state = editable_pr_state(); + apply_event(&mut state, AppEvent::Activate(HitTarget::EditResourceBody)); + let content = draw_actionable(&mut state); + assert!(content.contains("Edit description of openclaw/openclaw#81834")); + assert!(content.contains("[save ctrl+s]")); + assert!(!content.contains("[comment ctrl+s]")); + } + + #[test] + fn edit_picker_windows_long_lists_around_the_selection() { + let mut state = editable_pr_state(); + for n in 0..30 { + state.resource.activity.push(crate::domain::ActivityEntry { + id: format!("IC_{n}"), + edit: Some(crate::domain::EditTarget { + node_id: format!("IC_{n}"), + kind: crate::domain::EditKind::IssueComment, + current_body: format!("comment {n}"), + }), + kind: crate::domain::ActivityKind::Comment, + author: "me".into(), + body: format!("comment {n}"), + updated_at: "now".into(), + path: None, + line: None, + url: None, + author_association: None, + reactions: Default::default(), + includes_created_edit: false, + is_minimized: false, + minimized_reason: None, + thread_id: None, + thread_resolved: None, + thread_outdated: None, + }); + } + state.open_edit_picker(); + let total = state + .edit_picker + .as_ref() + .map(|picker| picker.choices.len()) + .expect("picker open"); + for _ in 0..total { + state.move_edit_picker_selection(1); + } + let content = draw(&mut state, 100, 20); + assert!( + content.contains(&format!("{total}/{total}")), + "position hint must show the selection" + ); + let last = rendered_target_rect(&state, |target| { + *target == HitTarget::EditPickerItem(total - 1) + }) + .expect("selected choice stays clickable in the window"); + let row = draw_row_text(&mut state, 100, 20, last.y); + assert!( + row.contains("comment 29"), + "windowed row text must sit on its hit area: {row}" + ); + assert!( + !state + .hit_areas + .iter() + .any(|area| area.target == HitTarget::EditPickerItem(0)), + "choices scrolled out of the window are not clickable" + ); + } + + #[test] + fn picker_digit_hints_stop_after_nine_and_selection_is_highlighted() { + let palette = ThemeName::Default.palette(); + let mut state = editable_pr_state(); + for n in 0..10 { + state.resource.activity.push(crate::domain::ActivityEntry { + id: format!("HINT_{n}"), + edit: Some(crate::domain::EditTarget { + node_id: format!("HINT_{n}"), + kind: crate::domain::EditKind::IssueComment, + current_body: format!("hinted {n}"), + }), + kind: crate::domain::ActivityKind::Comment, + author: "me".into(), + body: format!("hinted {n}"), + updated_at: "now".into(), + path: None, + line: None, + url: None, + author_association: None, + reactions: Default::default(), + includes_created_edit: false, + is_minimized: false, + minimized_reason: None, + thread_id: None, + thread_resolved: None, + thread_outdated: None, + }); + } + state.open_edit_picker(); + draw(&mut state, 100, 30); + let ninth = rendered_target_rect(&state, |target| *target == HitTarget::EditPickerItem(8)) + .expect("ninth choice"); + let hint_column = 61; + let ninth_row = draw_row_text(&mut state, 100, 30, ninth.y); + let ninth_inner = ninth_row.rsplit('\u{2502}').nth(1).unwrap_or(""); + assert_eq!( + ninth_inner.chars().nth(hint_column), + Some('9'), + "the ninth choice shows its digit hint: {ninth_row}" + ); + let tenth = rendered_target_rect(&state, |target| *target == HitTarget::EditPickerItem(9)) + .expect("tenth choice"); + let tenth_row = draw_row_text(&mut state, 100, 30, tenth.y); + let tenth_inner = tenth_row.rsplit('\u{2502}').nth(1).unwrap_or(""); + assert_eq!( + tenth_inner.chars().nth(hint_column), + Some(' '), + "choices past nine have no digit hint: {tenth_row}" + ); + assert_eq!( + draw_cell_bg_for_text(&mut state, 100, 30, "Description", 0), + Some(palette.accent), + "the selected picker row is highlighted" + ); + } + + #[test] + fn comment_edits_use_the_comment_title() { + let mut state = editable_pr_state(); + state.set_tab(crate::app::Tab::Activity); + draw_actionable(&mut state); + click_rendered_target(&mut state, |target| { + matches!(target, HitTarget::EditActivityEntry { .. }) + }); + let content = draw_actionable(&mut state); + assert!(content.contains("Edit comment on openclaw/openclaw#81834")); + assert!(!content.contains("Edit description of")); + } }