Skip to content

Fix py-editor execute code on ctrl-enter#2385

Merged
WebReflection merged 4 commits intopyscript:mainfrom
jeremykawahara:py-editor-enter
Oct 7, 2025
Merged

Fix py-editor execute code on ctrl-enter#2385
WebReflection merged 4 commits intopyscript:mainfrom
jeremykawahara:py-editor-enter

Conversation

@jeremykawahara
Copy link
Copy Markdown
Contributor

@jeremykawahara jeremykawahara commented Oct 7, 2025

Description

Really enjoying trying out PyScript! Thanks for this making available!

Currently, py-editor with CodeMirror does not execute code on shift-enter or ctrl-enter, and instead only adds a new line.

I believe the problem is that defaultKeymap overrides the custom key mapping.

From my understanding, defaultKeymap is not necessary as the default command bindings are already included in the basicSetup.

After removing defaultKeymap, the code is executed; however, it will also insert a newline.

We can prevent the newline by having the listener return true.

Changes

  • Removed defaultKeymap
  • listener returns true
  • Updated paths to dist folder in tests/manual

Checklist

  • I have checked make build works locally.
  • I have created / updated documentation for this change (if applicable).

@WebReflection
Copy link
Copy Markdown
Contributor

WebReflection commented Oct 7, 2025

I believe the problem is that defaultKeymap overrides the custom key mapping.

In JS, if you have { ...options, explicit: 123 } and options = { explicit: 456 } the result is { explicit: 123 } ... the last part after literal spreading matters more than keys spread before ... so I'd like to be sure something is off but, most importantly, I think this was tested and working as expected?

@WebReflection
Copy link
Copy Markdown
Contributor

WebReflection commented Oct 7, 2025

OK @jeremykawahara ... I gave it a go and indeed the precedence is somehow misleading because it's not a literal spread, it's an array of literals, which is different ... but because there is a reason I have put defaultKeyMap in there in the first place, unless that's really superfluous as you stated, this is the only code I had to change and it works:

    const listener = () => !runButton.click();
    const editor = new EditorView({
        extensions: [
            indentUnit.of(indentation),
            new Compartment().of(python()),
            keymap.of([
                { key: "Ctrl-Enter", run: listener, preventDefault: true },
                { key: "Cmd-Enter", run: listener, preventDefault: true },
                { key: "Shift-Enter", run: listener, preventDefault: true },
                ...defaultKeymap,
                // @see https://codemirror.net/examples/tab/
                indentWithTab,
            ]),
            basicSetup,
        ],
        foldGutter: true,
        gutters: ["CodeMirror-linenumbers", "CodeMirror-foldgutter"],
        parent,
        doc,
    });

would this work good enough? we really would like not to break previous expectations, this way Ctrl-Enter works as expected though ... what do you think?

@jeremykawahara
Copy link
Copy Markdown
Contributor Author

jeremykawahara commented Oct 7, 2025

Yep this works as well! Moving defaultKeymap below the custom key bindings seems to give precedence to the custom bindings and the !runButton.click() returns true on undefined fixing the adding newline issue.

This looks good to me! I do believe that defaultKeymap may be redundant since basicSetup seems to include it (in my tests commands like Alt-ArrowUp will still work with defaultKeymap removed). But keeping it in I think is fine too!

I think this was tested and working as expected?

FYI it seemed to work in 2024.9.2 but then break in 2024.10.1

<html lang="en">
<head>
    <!-- Shift-Enter executes code -->
    <link rel="stylesheet" href="https://pyscript.net/releases/2024.9.2/core.css" />
    <script type="module" src="https://pyscript.net/releases/2024.9.2/core.js"></script>

    <!-- Shift-Enter does not execute code and only adds a new line -->
    <!-- <link rel="stylesheet" href="https://pyscript.net/releases/2024.10.1/core.css" />
    <script type="module" src="https://pyscript.net/releases/2024.10.1/core.js"></script> -->
</head>

<body>
    <script type="mpy-editor">
        print("Hello!")
    </script>
</body>
</html>

@WebReflection
Copy link
Copy Markdown
Contributor

WebReflection commented Oct 7, 2025

heh, I guess that's due library changes behind the scene ... I am good with my proposed changes so, please:

  • just use the code I've used, plus add a comment before defaultKeyMaps that it's likely redundant, so we can just remove it later on
  • rebased on latest main as there are interesting and important changes that should not even conflict with this MR

Then I'll run the whole thing and approve, once green, thank you!

@jeremykawahara
Copy link
Copy Markdown
Contributor Author

jeremykawahara commented Oct 7, 2025

@WebReflection thanks - updated! Hope I did the rebase correctly.

WebReflection
WebReflection approved these changes Oct 7, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@WebReflection WebReflection left a comment

Choose a reason for hiding this comment

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

Thank you 🙏

@WebReflection WebReflection merged commit b609b60 into pyscript:main Oct 7, 2025
2 checks passed
@jeremykawahara jeremykawahara deleted the py-editor-enter branch October 8, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WebReflection WebReflection WebReflection approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jeremykawahara @WebReflection