fix: scope CM6 Tidy keymap per file state (#4093)#4097
Open
dimssu wants to merge 1 commit intoprocessing:develop-codemirror-v6from
Open
fix: scope CM6 Tidy keymap per file state (#4093)#4097dimssu wants to merge 1 commit intoprocessing:develop-codemirror-v6from
dimssu wants to merge 1 commit intoprocessing:develop-codemirror-v6from
Conversation
createNewFileState mutated a module-scoped extraKeymaps array by pushing a Shift-Mod-F binding that closed over the current file's mode. Because the array was shared across every file's state, later-created files accumulated multiple tidy handlers — Cmd+Shift+F on a CSS/HTML file ran prettier with the JS parser first, either throwing or mangling output. Move the tidy binding into a per-state local array and return true from the handler so CM6 consumes the shortcut. Add a regression test that creates JS/CSS/HTML states in order and verifies each tidies with its own mode.
Contributor
|
@dimssu Thank you for the PR. Please share the video preview for the fix |
|
Hi @dimssu, I think we can't raise a PR for an issue with the "Awaiting Maintainer Approval" label. I think you should first let a maintainer approve the issue, only then raise a PR. Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue:
Fixes #4093
In the CM6 editor (
develop-codemirror-v6), the Tidy shortcut (Cmd/Ctrl+Shift+F) works on the first file but misbehaves on subsequent files — sometimes nothing happens, sometimes the file is reformatted as "one big block."Root cause:
createNewFileStateinclient/modules/IDE/components/Editor/stateUtils.jspushed a Shift-Mod-F binding into a module-scopedextraKeymapsarray, closing over the current file's mode. Because later-created file states snapshot that shared array, files created after the first end up with multiple tidy handlers (one per previously-initialized file's mode). On a CSS file, the JS-mode tidy runs first → prettier's babel parser throws on CSS, or (for files it can parse) mangles the content before the correct-mode tidy ever runs.The Edit → Tidy Code menu click path was unaffected because it computes the mode from
file.nameat call time.Changes:
stateUtils.js: stop mutating the module-levelextraKeymaps. Build the tidy binding as a per-state local array (fileTidyKeymap) and include it in the state's keymaps. Handler now returnstrueso CM6 consumes the shortcut.stateUtils.test.js: new regression test. Creates JS, CSS, and HTML file states in order, dispatchesShift-Ctrl-FviarunScopeHandlers, and asserts each file is tidied with its own mode. Test fails on the previous code (prettier throws parsing CSS as JS) and passes on the fix.Testing:
Manually verified that pressing
Cmd+Shift+Fnow tidies correctly insketch.js,style.css, andindex.htmlregardless of which file is opened first.I have verified that this pull request:
npm run lint)npm run test— Editor suite; 9/9 pass)npm run typecheck)