Skip to content

Confirm viewport cancellation and restore source focus - #460

Merged
tninja merged 1 commit into
tninja:mainfrom
cxa:feat/viewport-cancel-confirmation
Jul 27, 2026
Merged

Confirm viewport cancellation and restore source focus#460
tninja merged 1 commit into
tninja:mainfrom
cxa:feat/viewport-cancel-confirmation

Conversation

@cxa

@cxa cxa commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • prompt for confirmation before canceling a viewport containing nonblank content
  • treat whitespace-only viewport content as empty
  • keep the viewport open when discard confirmation is declined
  • guard alternate recursive-edit exit and abort paths before viewport cleanup
  • restore focus to the original source input window after cancellation when it is still available
  • avoid forcing window-layout changes when the source window is no longer available

Testing

  • added regression coverage for confirmed and declined cancellation
  • added coverage for whitespace-only and narrowed viewport content
  • added full-lifecycle coverage for alternate exit, abort, and source-focus restoration
  • passed all 1,271 tests with 0 unexpected failures and 13 existing skips
  • passed byte compilation, checkdoc, and git diff --check

@tninja tninja left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution—this is a thoughtful improvement to viewport cancellation safety and source-focus restoration. I left one blocking lifecycle comment and one explicitly non-blocking defense-in-depth suggestion.

Comment thread ai-code-editor-viewport.el
Comment thread ai-code-editor-viewport.el
@cxa

cxa commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@tninja Thanks for the review. I addressed the blocking lifecycle issue and the defense-in-depth remaps in 00f59a0, replied on both threads, and reran the full suite (1,271 tests, 0 unexpected failures). Could you take another look when convenient?

@cxa
cxa force-pushed the feat/viewport-cancel-confirmation branch from 4629ff4 to 00f59a0 Compare July 27, 2026 16:19
@cxa
cxa requested a review from tninja July 27, 2026 16:51
@tninja

tninja commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Thank for the updates! Will review by the end of day.

@tninja tninja left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the feedback so thoroughly. The lifecycle guard and recursive-exit remaps close the data-loss path, and the updated coverage looks appropriate. Approved—thank you for the contribution!

@tninja
tninja merged commit 8471c1a into tninja:main Jul 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants