Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port eval-region to squint #44

Merged
merged 31 commits into from
Nov 22, 2023
Merged

Port eval-region to squint #44

merged 31 commits into from
Nov 22, 2023

Conversation

borkdude
Copy link
Contributor

See https://snapshots.nextjournal.com/clojure-mode/build/9a448e92b43fc0f6b3e1724d8ac1b1d7ca1000b3/squint/dist/index.html for a demo.

@zampino Note that I also updated the keys in the HTML since they weren't accurate.

@borkdude borkdude requested review from mk and zampino November 21, 2023 16:59
Copy link
Collaborator

@zampino zampino left a comment

Choose a reason for hiding this comment

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

👍 Looks good! Apart from the fact that Alt + Enter in the squint demo also breaks the line. To avoid that I think you have to return true from the key handler.

@borkdude
Copy link
Contributor Author

Apart from the fact that Alt + Enter in the squint demo also breaks the line

What do you mean with breaks the line? I'm not sure if I can see what you mean

@zampino
Copy link
Collaborator

zampino commented Nov 21, 2023

Apart from the fact that Alt + Enter in the squint demo also breaks the line

What do you mean with breaks the line? I'm not sure if I can see what you mean

alt + enter does evaluate the code in the editor but also enters a new line at the cursor, at least in safari. Chrome doesn't reproduce this issue.

@borkdude
Copy link
Contributor Author

@zampino Should be fixed now.

@mk @zampino Note that I added this to package.json:

"./extensions/eval_region": "./dist/nextjournal/clojure_mode/extensions/eval_region.mjs"

so you can write import * as evalRegion from '@nextjournal/clojure-mode/extensions/eval_region'

This should actually also be done if you want to import the other extensions explicitly but we can address that in a later PR/version. My point is more about: is this style the way we want it.

@zampino zampino merged commit 0bcc17f into main Nov 22, 2023
3 checks passed
@zampino zampino deleted the eval-region-squint branch November 22, 2023 14:15
@zampino zampino restored the eval-region-squint branch February 8, 2024 09:37
@zampino zampino deleted the eval-region-squint branch February 8, 2024 09:42
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