Skip to content

Conversation

@jeremykawahara
Copy link
Contributor

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
Contributor

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
Contributor

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
Contributor Author

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
Contributor

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
Contributor Author

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

Copy link
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants