Skip to content

Commit af1d93b

Browse files
committed
fix: numpad Enter submit and AUR after system update
- fix: treat KeyCode::Char('\n'|\r') as submit in password prompt and modals. - fix: treat numpad Enter as submit in search, recent panel, install flow. - fix: queue AUR update after pacman succeeds in system update flow. - test: add numpad Enter tests for modals (alert, confirm, help, news, etc.). - test: add handle_tick test for AUR queue after system update success. - chore: add PR description for fix/numpad-enter (dev/PR/).
1 parent 6552d3f commit af1d93b

11 files changed

Lines changed: 306 additions & 63 deletions

File tree

dev/PR/PR_fix-numpad-enter.md

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
<!-- Thank you for contributing to Pacsea!
2+
3+
**Important references:**
4+
- [CONTRIBUTING.md](../CONTRIBUTING.md) — Full contribution guidelines and PR process
5+
- [PR_DESCRIPTION.md](../Documents/PR_DESCRIPTION.md) — Detailed PR description template
6+
- [Development Wiki](https://github.com/Firstp1ck/Pacsea/wiki/Development) — Development tools and debugging
7+
8+
Please ensure you've reviewed these before submitting your PR.
9+
-->
10+
11+
## Summary
12+
13+
- **Problem:** Numpad Enter did not submit the password prompt; it could insert a character or do nothing. On some terminals (e.g. Wayland/Alacritty), numpad Enter is reported as `KeyCode::Char('\r')` or `KeyCode::Char('\n')` instead of `KeyCode::Enter`, and only main Enter was handled as submit.
14+
- **Fix:** Treat `KeyCode::Char('\n')` and `KeyCode::Char('\r')` as submit alongside `KeyCode::Enter` in the password prompt and extend the same behaviour for consistency across search, modals (Alert, Help, Confirm, optional deps, import, system update, etc.), recent panel, and install flow.
15+
- **Implementation:** In `src/events/modals/password.rs`, submit arm is `KeyCode::Enter | KeyCode::Char('\n' | '\r')`. Same “Enter = submit” semantics applied in search (normal/insert), modal handlers, recent, and install. PostSummary excluded-keys includes `Char('\n')` and `Char('\r')` for restore logic.
16+
- **Tests:** Unit tests for password handler (numpad Enter as `\r`/`\n`, main Enter, regression, edge cases) and modal numpad Enter tests (Alert, Help, Confirm, PostSummary, GnomeTerminalPrompt, ImportHelp, VirusTotalSetup, News).
17+
18+
- **Problem (Update system):** When using all selections in the "Update system" modal (mirrors + pacman + AUR + cache), AUR packages were not updated. The AUR command was stored separately and only run when pacman failed (ConfirmAurUpdate); it was never run when pacman succeeded.
19+
- **Fix (Update system):** Store password and header_chips when submitting the Update password so they are available after the first batch. In the tick handler, when `PreflightExec` shows success with empty items (system update) and `pending_aur_update_command` is set, queue the AUR command; AUR then runs automatically after pacman succeeds.
20+
- **Implementation (Update system):** Handlers: set `pending_executor_password` and `pending_exec_header_chips` on Update submit. Tick handler: new `maybe_queue_aur_after_system_update_success()`; call it from `handle_tick`.
21+
- **Tests (Update system):** `handle_tick_queues_aur_after_system_update_success` verifies AUR is queued after system update success when pacman + AUR were selected.
22+
23+
## Type of change
24+
25+
- [ ] feat (new feature)
26+
- [x] fix (bug fix)
27+
- [ ] docs (documentation only)
28+
- [ ] refactor (no functional change)
29+
- [ ] perf (performance)
30+
- [x] test (add/update tests)
31+
- [ ] chore (build/infra/CI)
32+
- [ ] ui (visual/interaction changes)
33+
- [ ] breaking change (incompatible behavior)
34+
35+
## Related issues
36+
37+
Closes #119
38+
39+
## How to test
40+
41+
1. Run quality checks:
42+
```bash
43+
cargo fmt --all
44+
cargo clippy --all-targets --all-features -- -D warnings
45+
cargo check
46+
cargo test -- --test-threads=1
47+
```
48+
49+
2. Manual: trigger a password prompt (e.g. install a package that prompts for sudo password), type password, press **numpad Enter** — password should submit and flow continue (same as main Enter).
50+
51+
3. Manual: in search, modals (confirm, alert, help, etc.), recent panel, and install flow, verify numpad Enter acts as submit/confirm where main Enter does.
52+
53+
## Screenshots / recordings (if UI changes)
54+
55+
N/A — key handling only; no visual change.
56+
57+
## Checklist
58+
59+
**Code Quality:**
60+
- [ ] Code compiles locally (`cargo check`)
61+
- [ ] `cargo fmt --all` ran without changes
62+
- [ ] `cargo clippy --all-targets --all-features -- -D warnings` is clean
63+
- [ ] `cargo test -- --test-threads=1` passes
64+
- [ ] Complexity checks pass for new code (`cargo test complexity -- --nocapture`)
65+
- [ ] All new functions/methods have rustdoc comments (What, Inputs, Output, Details)
66+
- [ ] No `unwrap()` or `expect()` in non-test code
67+
68+
**Testing:**
69+
- [ ] Added or updated tests where it makes sense
70+
- [ ] For bug fixes: created failing tests first, then fixed the issue
71+
- [ ] Tests are meaningful and cover the functionality
72+
73+
**Documentation:**
74+
- [ ] Updated README if behavior, options, or keybinds changed (keep high-level, reference wiki)
75+
- [ ] Updated relevant wiki pages if needed
76+
- [ ] Updated config examples in `config/` directory if config keys changed
77+
- [ ] For UI changes: included screenshots and updated `Images/` if applicable
78+
79+
**Compatibility:**
80+
- [ ] Changes respect `--dry-run` flag
81+
- [ ] Code degrades gracefully if `pacman`/`paru`/`yay` are unavailable
82+
- [ ] No breaking changes (or clearly documented if intentional)
83+
84+
**Other:**
85+
- [ ] Not a packaging change for AUR (otherwise propose in `pacsea-bin` or `pacsea-git` repos)
86+
87+
## Notes for reviewers
88+
89+
- Minimal behavioural change: only additional key codes (`Char('\n')`, `Char('\r')`) are treated as submit; main Enter and existing behaviour unchanged.
90+
- `\n` and `\r` are control characters, so they were already not inserted into the password buffer; this change only makes them trigger submit.
91+
- Consistency pass: same Enter semantics applied in search, modals, recent, and install so numpad Enter behaves uniformly.
92+
- Design/analysis is documented in `dev/IMPROVEMENTS/ISSUE_119_NUMPAD_ENTER_PASSWORD_PROMPT.md`.
93+
94+
## Breaking changes
95+
96+
None.
97+
98+
## Additional context
99+
100+
- Issue: [Firstp1ck/Pacsea#119](https://github.com/Firstp1ck/Pacsea/issues/119)
101+
- Crossterm has no `NumpadEnter`; numpad Enter is reported as `KeyCode::Enter` or `KeyCode::Char('\r')` / `KeyCode::Char('\n')` depending on terminal/OS.

src/app/runtime/tick_handler.rs

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,47 @@ use super::super::persist::{
1616
maybe_flush_services_cache,
1717
};
1818
use super::super::recent::{maybe_save_news_recent, maybe_save_recent};
19+
use crate::install::ExecutorRequest;
20+
21+
/// What: Queue AUR update after system update success when pacman + AUR were both selected.
22+
///
23+
/// Inputs:
24+
/// - `app`: Mutable application state
25+
///
26+
/// Output: None
27+
///
28+
/// Details:
29+
/// - When `PreflightExec` shows success, items are empty (system update), and pending AUR command
30+
/// is set, queues the AUR command and updates the modal log. Called from tick to avoid borrow
31+
/// conflict in `event_loop`.
32+
fn maybe_queue_aur_after_system_update_success(app: &mut AppState) {
33+
let (success_is_true, items_empty) =
34+
if let crate::state::Modal::PreflightExec { success, items, .. } = &app.modal {
35+
(success == &Some(true), items.is_empty())
36+
} else {
37+
(false, false)
38+
};
39+
if !success_is_true || !items_empty || app.pending_aur_update_command.is_none() {
40+
return;
41+
}
42+
let aur_cmd = app.pending_aur_update_command.take();
43+
let password = app.pending_executor_password.take();
44+
if let (Some(aur_cmd), Some(password)) = (aur_cmd, password) {
45+
app.pending_executor_request = Some(ExecutorRequest::Update {
46+
commands: vec![aur_cmd],
47+
password: Some(password),
48+
dry_run: app.dry_run,
49+
});
50+
if let crate::state::Modal::PreflightExec {
51+
log_lines, success, ..
52+
} = &mut app.modal
53+
{
54+
log_lines.push(String::new());
55+
log_lines.push("Running AUR update...".to_string());
56+
*success = None;
57+
}
58+
}
59+
}
1960

2061
/// What: Handle PKGBUILD result event.
2162
///
@@ -649,6 +690,8 @@ pub fn handle_tick(
649690
summary_req_tx,
650691
);
651692

693+
maybe_queue_aur_after_system_update_success(app);
694+
652695
// Send pending executor request if PreflightExec modal is active
653696
if let Some(request) = app.pending_executor_request.take()
654697
&& matches!(app.modal, crate::state::Modal::PreflightExec { .. })
@@ -794,6 +837,9 @@ pub fn handle_status(app: &mut AppState, txt: &str, color: ArchStatusColor) {
794837
#[cfg(test)]
795838
mod tests {
796839
use super::*;
840+
use crate::install::ExecutorRequest;
841+
use crate::state::Modal;
842+
use crate::state::modal::{PreflightAction, PreflightTab};
797843

798844
/// What: Provide a baseline `AppState` for tick handler tests.
799845
///
@@ -927,6 +973,95 @@ mod tests {
927973
assert!(app.preflight_sandbox_items.is_none());
928974
}
929975

976+
#[test]
977+
/// What: Verify AUR update is queued after system update success when pacman + AUR were selected.
978+
///
979+
/// Inputs:
980+
/// - `AppState` with `PreflightExec` (success, empty items), `pending_aur_update_command`,
981+
/// and `pending_executor_password` set; tick handler called.
982+
///
983+
/// Output:
984+
/// - `pending_executor_request` is `Some(Update { commands: [aur_cmd], ... })`,
985+
/// `pending_aur_update_command` is `None`, modal log contains "Running AUR update...".
986+
///
987+
/// Details:
988+
/// - Ensures "Update system" with all selections (including AUR) runs AUR after pacman succeeds.
989+
fn handle_tick_queues_aur_after_system_update_success() {
990+
let mut app = new_app();
991+
let aur_cmd = "paru -Sua --noconfirm".to_string();
992+
app.modal = Modal::PreflightExec {
993+
items: vec![],
994+
action: PreflightAction::Install,
995+
tab: PreflightTab::Summary,
996+
verbose: false,
997+
log_lines: vec!["Done.".to_string()],
998+
abortable: false,
999+
header_chips: crate::state::modal::PreflightHeaderChips::default(),
1000+
success: Some(true),
1001+
};
1002+
app.pending_aur_update_command = Some(aur_cmd.clone());
1003+
app.pending_executor_password = Some("pass".to_string());
1004+
1005+
let (query_tx, _query_rx) = mpsc::unbounded_channel();
1006+
let (details_tx, _details_rx) = mpsc::unbounded_channel();
1007+
let (pkgb_tx, _pkgb_rx) = mpsc::unbounded_channel();
1008+
let (deps_tx, _deps_rx) = mpsc::unbounded_channel();
1009+
let (files_tx, _files_rx) = mpsc::unbounded_channel();
1010+
let (services_tx, _services_rx) = mpsc::unbounded_channel();
1011+
let (sandbox_tx, _sandbox_rx) = mpsc::unbounded_channel();
1012+
let (summary_tx, _summary_rx) = mpsc::unbounded_channel();
1013+
let (updates_tx, _updates_rx) = mpsc::unbounded_channel();
1014+
let (executor_req_tx, mut executor_req_rx) = mpsc::unbounded_channel();
1015+
let (post_summary_tx, _post_summary_rx) = mpsc::unbounded_channel();
1016+
let (news_content_tx, _news_content_rx) = mpsc::unbounded_channel();
1017+
1018+
handle_tick(
1019+
&mut app,
1020+
&query_tx,
1021+
&details_tx,
1022+
&pkgb_tx,
1023+
&deps_tx,
1024+
&files_tx,
1025+
&services_tx,
1026+
&sandbox_tx,
1027+
&summary_tx,
1028+
&updates_tx,
1029+
&executor_req_tx,
1030+
&post_summary_tx,
1031+
&news_content_tx,
1032+
);
1033+
1034+
assert!(
1035+
app.pending_aur_update_command.is_none(),
1036+
"pending_aur_update_command should be taken when AUR is queued"
1037+
);
1038+
let request = executor_req_rx
1039+
.try_recv()
1040+
.expect("executor should receive Update request with AUR command");
1041+
match &request {
1042+
ExecutorRequest::Update {
1043+
commands, password, ..
1044+
} => {
1045+
assert_eq!(commands.len(), 1);
1046+
assert_eq!(commands[0], aur_cmd);
1047+
assert_eq!(password.as_deref(), Some("pass"));
1048+
}
1049+
_ => panic!("expected ExecutorRequest::Update, got {request:?}"),
1050+
}
1051+
if let Modal::PreflightExec {
1052+
log_lines, success, ..
1053+
} = &app.modal
1054+
{
1055+
assert!(
1056+
log_lines.iter().any(|l| l == "Running AUR update..."),
1057+
"modal log should contain 'Running AUR update...'"
1058+
);
1059+
assert!(success.is_none(), "success should be cleared for AUR run");
1060+
} else {
1061+
panic!("modal should still be PreflightExec");
1062+
}
1063+
}
1064+
9301065
#[test]
9311066
/// What: Verify that `handle_tick` processes `PKGBUILD` reload debouncing.
9321067
///

src/events/modals/handlers.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,6 +1060,15 @@ pub(super) fn handle_password_prompt_modal(
10601060
// Handle Update purpose with pending_update_commands
10611061
if matches!(purpose, crate::state::modal::PasswordPurpose::Update) {
10621062
if let Some(commands) = app.pending_update_commands.take() {
1063+
// Store password and header_chips so AUR update can run after pacman succeeds,
1064+
// or so ConfirmAurUpdate can run AUR if pacman fails
1065+
match (&mut app.pending_executor_password, &password) {
1066+
(Some(p), Some(pass)) => p.clone_from(pass),
1067+
(None, Some(pass)) => app.pending_executor_password = Some(pass.clone()),
1068+
(_, None) => app.pending_executor_password = None,
1069+
}
1070+
app.pending_exec_header_chips = Some(header_chips.clone());
1071+
10631072
// Transition to PreflightExec for system update
10641073
app.modal = Modal::PreflightExec {
10651074
items: Vec::new(), // System update doesn't have package items

src/events/modals/tests/alert.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ fn alert_enter_closes_modal() {
6565
///
6666
/// Inputs:
6767
/// - Alert modal with message
68-
/// - KeyCode::Char('\r') (numpad Enter on some terminals)
68+
/// - `KeyCode::Char`('\r') (numpad Enter on some terminals)
6969
///
7070
/// Output:
7171
/// - Modal is set to None
@@ -88,7 +88,7 @@ fn alert_numpad_enter_carriage_return_closes_modal() {
8888
///
8989
/// Inputs:
9090
/// - Alert modal with message
91-
/// - KeyCode::Char('\n') (numpad Enter on some terminals)
91+
/// - `KeyCode::Char`('\n') (numpad Enter on some terminals)
9292
///
9393
/// Output:
9494
/// - Modal is set to None

src/events/modals/tests/confirm.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,17 +114,17 @@ fn confirm_remove_enter_closes_modal() {
114114
}
115115

116116
#[test]
117-
/// What: Verify numpad Enter (carriage return) closes ConfirmRemove modal like main Enter.
117+
/// What: Verify numpad Enter (carriage return) closes `ConfirmRemove` modal like main Enter.
118118
///
119119
/// Inputs:
120-
/// - ConfirmRemove modal
121-
/// - KeyCode::Char('\r')
120+
/// - `ConfirmRemove` modal
121+
/// - `KeyCode::Char`('\r')
122122
///
123123
/// Output:
124124
/// - Modal is set to None
125125
///
126126
/// Details:
127-
/// - Ensures numpad Enter handling does not break ConfirmRemove; same outcome as main Enter
127+
/// - Ensures numpad Enter handling does not break `ConfirmRemove`; same outcome as main Enter
128128
fn confirm_remove_numpad_enter_carriage_return_closes_modal() {
129129
let mut app = new_app();
130130
app.modal = crate::state::Modal::ConfirmRemove {
@@ -145,17 +145,17 @@ fn confirm_remove_numpad_enter_carriage_return_closes_modal() {
145145
}
146146

147147
#[test]
148-
/// What: Verify numpad Enter (newline) closes ConfirmRemove modal like main Enter.
148+
/// What: Verify numpad Enter (newline) closes `ConfirmRemove` modal like main Enter.
149149
///
150150
/// Inputs:
151-
/// - ConfirmRemove modal
152-
/// - KeyCode::Char('\n')
151+
/// - `ConfirmRemove` modal
152+
/// - `KeyCode::Char`('\n')
153153
///
154154
/// Output:
155155
/// - Modal is set to None
156156
///
157157
/// Details:
158-
/// - Ensures numpad Enter handling does not break ConfirmRemove; same outcome as main Enter
158+
/// - Ensures numpad Enter handling does not break `ConfirmRemove`; same outcome as main Enter
159159
fn confirm_remove_numpad_enter_newline_closes_modal() {
160160
let mut app = new_app();
161161
app.modal = crate::state::Modal::ConfirmRemove {

src/events/modals/tests/help.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ fn help_enter_closes_modal() {
6161
///
6262
/// Inputs:
6363
/// - Help modal
64-
/// - KeyCode::Char('\r')
64+
/// - `KeyCode::Char`('\r')
6565
///
6666
/// Output:
6767
/// - Modal is set to None
@@ -82,7 +82,7 @@ fn help_numpad_enter_carriage_return_closes_modal() {
8282
///
8383
/// Inputs:
8484
/// - Help modal
85-
/// - KeyCode::Char('\n')
85+
/// - `KeyCode::Char`('\n')
8686
///
8787
/// Output:
8888
/// - Modal is set to None

src/events/modals/tests/news.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ fn news_enter_preserves_modal() {
211211
///
212212
/// Inputs:
213213
/// - News modal with item
214-
/// - KeyCode::Char('\r')
214+
/// - `KeyCode::Char`('\r')
215215
///
216216
/// Output:
217217
/// - Modal remains News, selected unchanged
@@ -231,7 +231,7 @@ fn news_numpad_enter_carriage_return_keeps_modal_open() {
231231
}];
232232
let mut app = new_app();
233233
app.modal = crate::state::Modal::News {
234-
items: items.clone(),
234+
items,
235235
selected: 0,
236236
scroll: 0,
237237
};
@@ -249,7 +249,7 @@ fn news_numpad_enter_carriage_return_keeps_modal_open() {
249249
///
250250
/// Inputs:
251251
/// - News modal with item
252-
/// - KeyCode::Char('\n')
252+
/// - `KeyCode::Char`('\n')
253253
///
254254
/// Output:
255255
/// - Modal remains News, selected unchanged
@@ -269,7 +269,7 @@ fn news_numpad_enter_newline_keeps_modal_open() {
269269
}];
270270
let mut app = new_app();
271271
app.modal = crate::state::Modal::News {
272-
items: items.clone(),
272+
items,
273273
selected: 0,
274274
scroll: 0,
275275
};

0 commit comments

Comments
 (0)