[jgregoriorincon]

Summary

Holding Ctrl/⌘+Enter in the SQL pane ran the query again and again: key_pressed(Enter) counts key-repeat events, and go only skipped them while a query was running, so a repeat arriving after the preview finished ran it again.

The run key is now read by run_pressed: an Enter press with Ctrl/⌘ (matches_logically(COMMAND)) and repeat: false, the rule the Script pane uses since #773. egui sets repeat itself for every press after the first while a key is held. The key is still not consumed, as before.

The Labeling pane's SQL box (sqlbox) is left as is: it takes the repeats too, but Apply only acts on a changed clause and the pane runs it in the same frame, so the repeats find nothing to apply (see the correction on #845).

Testing

• cargo xtask ci passes.
• New test sqlpane::tests::a_held_ctrl_enter_runs_once: a press runs, a repeat doesn't, a press among repeats does, Enter alone doesn't, Shift is ignored.
• Manual (desktop, DuckDB installed): holding Ctrl/⌘+Enter on SELECT 42 for 2 s runs it once; releasing and pressing again runs it again; Enter alone breaks the line; Ctrl/⌘+Enter with the editor unfocused runs nothing; the Run button still works.

Related issue

Closes #845

Summary by CodeRabbit

• Bug Fixes
  • Ctrl/⌘+Enter runs a query only once per key press. Holding the keys no longer triggers repeated runs, while Enter without Ctrl/⌘ continues to insert a line break.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: cd3c9b69-ce69-49c8-b75b-72b03de8f940

Commits

Reviewing files that changed from the base of the PR and between ebbe8a5 and 2f42c6b.

Files selected for processing: 1

• crates/ui-egui/src/sqlpane.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

The SQL pane now runs a query on a non-repeated Enter press with the logical Command modifier. Enter without Command continues to insert a line break. Shift can also be held.

Changes: SQL pane shortcut

Layer / File(s)	Summary
SQL shortcut repeat handling crates/ui-egui/src/sqlpane.rs	A helper detects non-repeated Command+Enter events. The editor uses it to decide when to run a query. Tests cover repeated and non-repeated events, Enter without Command, and Command+Shift+Enter.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

• [Feature]: Run the Script pane with Ctrl+Enter (⌘+Enter on macOS) #773 — The PR applies the non-repeat shortcut rule already used in the Script pane.

Possibly related PRs

• opengeos/RustGIS#783 — This PR adds the Script pane shortcut behavior that the SQL pane change follows.

Suggested reviewers: giswqs

Merge Risk: Minimal · up to 2f42c

The requested shortcut behavior is implemented without introducing a current merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title names the change and its cause: the SQL pane's query runs once per Ctrl+Enter press instead of on each key repeat. The changeset matches this. The sqlpane.rs summary shows Ctrl/Cmd+Enter now…
Description check		The description follows the repository template. It has Summary, Testing, and Related issue sections. The Summary explains the root cause (key_pressed counts key-repeat events, and go only skipped the…
Linked Issues check		The changes satisfy the coding requirements in [#845]. sqlpane::run_pressed accepts only a pressed Enter event with repeat: false and logical Command modifiers. The SQL pane keeps the focus chec…
Out of Scope Changes check		The whole diff is within [#845]. The new run_pressed helper changes the SQL pane shortcut behavior. The integration change uses that helper. The unit test verifies the required key-event cases. No u…

Full details: Title check

Explanation

The title names the change and its cause: the SQL pane's query runs once per Ctrl+Enter press instead of on each key repeat. The changeset matches this. The sqlpane.rs summary shows Ctrl/Cmd+Enter now runs only on a non-repeated Enter press with logical Command modifiers, and the new test covers held-key repeats. The title is in the conventional commit form and is a single sentence under 80 characters. It names a specific change, so it is not vague.

Full details: Description check

Explanation

The description follows the repository template. It has Summary, Testing, and Related issue sections. The Summary explains the root cause (key_pressed counts key-repeat events, and go only skipped them while a query ran), the fix (run_pressed checks for an Enter press with COMMAND and repeat false), and why sqlbox was left unchanged. The Testing section lists cargo xtask ci, a new named test with the cases it covers, and manual desktop steps. The Related issue section links Closes #845. The description is complete.

Full details: Linked Issues check

Explanation

The changes satisfy the coding requirements in [#845]. sqlpane::run_pressed accepts only a pressed Enter event with repeat: false and logical Command modifiers. The SQL pane keeps the focus check and the existing running-query guard. The new test covers initial presses, repeats, presses among repeats, Enter without Command, and Shift. The Labeling pane already prevents repeat applies: sqlbox::editor requires changed && reads.is_ok(), and labelpane::sql applies the result in the same frame. A repeat after the first apply therefore finds no changed clause.

Full details: Out of Scope Changes check

Explanation

The whole diff is within [#845]. The new run_pressed helper changes the SQL pane shortcut behavior. The integration change uses that helper. The unit test verifies the required key-event cases. No unrelated production behavior or unrelated files changed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit taps the Enter key,
One press runs the query free.
Repeats now wait and do not go,
Shift may join, the lines still flow.
The SQL pane rests in row.}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Code review

This PR makes the SQL pane run its query once per Ctrl+Enter press instead of once for each key repeat (#845). I found no bugs. I read the diff only and did not build the code or run the tests.

Bugs

• No blocking issues.
• Low confidence (about 30%), minor behaviour change: matches_logically(COMMAND) requires Alt to be unpressed. The old modifiers.command && key_pressed(Enter) ignored Alt, so Ctrl+Alt+Enter ran the query before and probably won't now. I think this is harmless, and it matches how the Script pane's shortcut works. If you want the old behaviour exactly, test modifiers.command directly. The new test covers Shift but not Alt.
• Not a regression: on macOS, command maps to ⌘ and Ctrl+Enter doesn't match. This was already the case.

Security

• Nothing to flag.

Performance

• Nothing to flag. run_pressed scans the frame's events once and stops at the first match.

Quality

• run_pressed is small, pure and easy to test. Its doc comment explains the reason, and the regression test covers the main cases: a press, a repeat, a press among repeats, Enter alone and Shift held. This is good.
• Optional nit: the new code line matches!(e, egui::Event::Key { … } if …) is long. cargo fmt should already handle it, since cargo xtask ci runs it.

AGENTS.md

• No unwrap, panic or unsafe is added.
• The change touches only the egui layer (L6), so layering and wasm builds are unaffected.
• No user-visible strings are added, so tl! doesn't apply.
• It includes a regression test for the fix, as the repo asks.
• No new dependencies or assets.

Change size

Compared: the whole PR against main (merge base ebbe8a5 to PR head 2f42c6b)

Metric	Value
Files changed	1 (added 0, modified 1, removed 0, renamed 0)
Size before	19.0 KB
Size after	20.3 KB
Total size change	+1.2 KB

File	Before	After	Change
crates/ui-egui/src/sqlpane.rs	19.0 KB	20.3 KB	+1.2 KB

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-848/
Web app	https://preview.rustgis.app/pr-848/app/
Commit	2f42c6b

GitHub Pages takes a minute or two to publish a new commit. Newer commits from the fork aren't published until a maintainer adds the preview label (once per commit).

[github-actions]

Code review

I reviewed the one-file change to crates/ui-egui/src/sqlpane.rs: the new run_pressed helper, its call site in body, and the new test. I found nothing worth raising as an inline comment.

Bugs

• None found. The new check reads Enter-press events and requires pressed: true, repeat: false and Ctrl/⌘ via matches_logically(COMMAND). That fixes the repeat re-run described in [Bug]: Holding Ctrl/⌘+Enter runs a SQL query or applies a clause again for every key repeat #845.
• The change only reads the events, so the key is still not consumed, as before. run_key is still gated on focused.
• The new test covers a press, a repeat, a press among repeats, Enter alone and Shift. The Shift case only passes if matches_logically ignores extra Shift. I couldn't run the tests here, and the PR says cargo xtask ci passes. Confidence: high.

Security

• None. The change only reads input events.

Performance

• None. The helper scans the frame's event list, which is tiny.

Quality

• Minor, low confidence: run_pressed copies the rule the Script pane already uses ([Feature]: Run the Script pane with Ctrl+Enter (⌘+Enter on macOS) #773). If both panes share the same rule, a shared helper would stop them drifting apart. This is optional and not blocking.

AGENTS.md

• No unwrap, expect or unsafe, and the change uses egui only in ui-egui (L6), so layering is respected.
• No new user-visible strings and no new dependencies.
• The PR adds a regression test for the bug fix, as the repo expects.

Change size

Compared: the whole PR against main (merge base ebbe8a5 to PR head 2f42c6b)

Metric	Value
Files changed	1 (added 0, modified 1, removed 0, renamed 0)
Size before	19.0 KB
Size after	20.3 KB
Total size change	+1.2 KB

File	Before	After	Change
crates/ui-egui/src/sqlpane.rs	19.0 KB	20.3 KB	+1.2 KB