feat: improve format painter ux to match ms word#3266
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e36a99669
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| result = result[cmd.command](cmd.argument); | ||
| }); | ||
|
|
||
| if (!shouldStayActive) clearFormatPainterStorage(storage); |
There was a problem hiding this comment.
Double-clear bug in applyStoredFormat
clearFormatPainterStorage is called both inside the try block and unconditionally in finally. The try-body calls are dead code — finally always fires on return. This means the event listener is removed twice (a minor waste) and the logic is confusing to read.
// try block:
if (!shouldStayActive) clearFormatPainterStorage(storage); // ← redundant
return result;
// finally block:
if (!shouldStayActive) clearFormatPainterStorage(storage); // ← the only one needed
Fix: remove the clearFormatPainterStorage calls from inside the try body and keep only the finally.
There was a problem hiding this comment.
removed redundant line
artem-harbour
left a comment
There was a problem hiding this comment.
Added my comments.
3e36a99 to
272998b
Compare
No description provided.