Skip to content

Comments

Upgrade CodeMirror5 to CodeMirror6#210

Open
fredsa wants to merge 63 commits intosehugg:masterfrom
fredsa:cm5-cm6
Open

Upgrade CodeMirror5 to CodeMirror6#210
fredsa wants to merge 63 commits intosehugg:masterfrom
fredsa:cm5-cm6

Conversation

@fredsa
Copy link

@fredsa fredsa commented Feb 15, 2026

Upgrade to CodeMirror6, supports multi-cursor editing, and other modern conveniences.

Removed CodeMirror5 implementation:

  • codemirror submodule
  • src/codemirror/*
  • css/codemirror.css

Installed CodeMirror6 npm packages:

  • @codemirror/commands
  • @codemirror/view
  • @codemirror/lang-cpp
  • @codemirror/lang-markdown

Filed changed:

  • src/ide/views/editors.ts Significant changes; bulk of migration effort
  • css/ui.css Removed no longer used styles
  • `index.html' Removed CodeMirror5 references

New files:

  • src/ide/views/filters.ts Text transform filter (for BASIC uppercase support)
  • src/ide/views/gutter.ts Gutter decorations and widgets
  • src/ide/views/visuals.ts Main editor decorations and widgets
  • src/parser/lang-*.ts StreamParser based languages, based on original CM5 implementations
  • src/themes/mbo.ts migrated mbo theme
  • src/themes/cobalt.ts migrated cobalt theme
  • src/themes/editorTheme.ts CSS for main editor
  • src/themes/disassemblyTheme.ts CSS for disassembly view

Migration notes:

  • src/codemirror/ecs.js Removed, but not migrated; appears unused
  • src/codemirror/vasm.js Removed, but not migrated; appears unused
  • @codemirror/lang-javascript Not installed; javascript platform appears unused

TODO

  • Migrated src/parser/lang-*.ts stream parsers may require a few final color / style tweaks
  • Migrated src/themes/mbo.ts and src/themes/cobalt.ts themes may require a few final color / style tweaks
  • Consider migrating src/parser/lang-*.ts stream parser to Lezer parsers for languages were this is reasonable

@fredsa
Copy link
Author

fredsa commented Feb 19, 2026

A few more commits, to fix issues accumulated during migration:

  • flaky undo support due to missing historyKeymap
  • insertText/undoStep still using CM5 code
  • polish up text transform filter; not strictly needed given toUppercase() is the only use case, but it's not hopefully a better example and more future proof
  • couple of color changes to cobalt theme to make multi-line + multi-select easier to see
  • fix a handful of line number off-by-one bugs
  • set parsers for a few modes that had parsers, but weren't being used
  • removed unused code in editors.ts

@sehugg
Copy link
Owner

sehugg commented Feb 20, 2026

After 1.5 cups of coffee I can say it looks pretty good! Some comments:

I like the new breakpoint marker, and having a visual indicator when you Run To Line. Maybe the breakpoint and current PC gutters could be adjacent for a bigger hit target? And maybe a cursor change or highlight when you move the pointer over the gutter.

I noticed that the gutters expand and push the editor to the right after the first compilation completes. Actually, the gutters expand when you scroll down into a region that has gutter elements. It's a little distracting on first load but I can't think of an easy solution, that seems to just be the way CM6 works.

I wish there was a way to make error tooltips show up on pointer move, but I don't think CM6 has much to do with this, since the marker just uses the title attribute.

The Asset Editor is a complete mess but it works by modifying text in the editor directly via project.updateFile(). It also expects projectWindows.undoStep() to undo the last updateFile() operation, because CM doesn't have a global undo history. At least I think that's how it works ... I see this is broken in the CM5 version also, so I need to investigate.

I still want to test on mobile devices and look at performance, but I hope to merge this soon! Nice job!

@fredsa
Copy link
Author

fredsa commented Feb 20, 2026

After 1.5 cups of coffee I can say it looks pretty good! Some comments:
Great to hear. I'm working through a couple of improvements now that I understand CM6 a bit better.

  • I'm focused on the gutter right now which has some unnecessary excess churn in DOM element creation as you move around the document on scroll. I'm just figuring out the best way to address it.
  • The PR current has the gutter keeping track of line numbers. Turns out this is not ideal as this causes a bit of lag when say inserting a line above an error marker or breakpoint before the marker moves to the correct line. The right way to this is apparently managing ranges, which CM can automatically update as the document is mutated. Again, just looking into the best way to do this.

For both I should have something shortly.

I like the new breakpoint marker, and having a visual indicator when you Run To Line. Maybe the breakpoint and current PC gutters could be adjacent for a bigger hit target? And maybe a cursor change or highlight when you move the pointer over the gutter.

Agreed that some improvement is needed there. Having the CurrentPC gutter be clickable was a bit of a temporary implementation as I was imaging that the user would focus on toggle (multiple) breakpoints.

There's also a question of how to treat error marker, current PC marker and other info markers. Question being which of these can/should share the same physical gutter column. Does a priority system make sense? Want to avoid taking up too much horizontal space, while also avoiding a confusing experience. The approach I've been taking for the migration is a) get all the functionality working, using separate gutter columns to keep things simple, b) once that's all settled, and you're happy with it, revisit what you want to CM6 experience to be. A key enhancement would be multiple breakpoints, assuming the different runtimes can easily support them.

I noticed that the gutters expand and push the editor to the right after the first compilation completes. Actually, the gutters expand when you scroll down into a region that has gutter elements. It's a little distracting on first load but I can't think of an easy solution, that seems to just be the way CM6 works.

Yep, this needs to be fixed. This should be addressable through initialSpacer. The one thing that might need to get plumbed through (perhaps it already is) is the maximum number of bytes for each line, or at least the default number of bytes.

Are 3 bytes (opcode + operand) always enough? And, still okay with the gutter surpressing bytes? For example, https://8bitworkshop.com/v3.12.1/?platform=c64&file=hello.dasm contains byte "HELLO`WORLDa", but the gutter only shows the first 3 bytes. We can keep that, and can definitely keep the gutter to min/max 3 bytes (or a different number of bytes for different platforms). Adding an ellipsis to the bytes gutter that shows all the values would be nice. Another cool feature would be to allow user highlighting of byte "HELLO`WORLDa" and see the value below, the same way that variable values are shown when highlighted.

Oh and variable highlights: currently they show up on an extra line. In the production version you can copy/paste that value. In the CM6 version you currently can't. I think probably we can fix this, but I wonder if you think of this as an important feature or just a random useless thing that happens to be possible.

Have variables values show up as tooltips might be a more familiar way to handle things. That should be easy.

In editors.ts, try setting debugHighlightTags = true, then hover over different parts of the code to see the underlying style.

I wish there was a way to make error tooltips show up on pointer move, but I don't think CM6 has much to do with this, since the marker just uses the title attribute.

I think this is possible via the gutter's domEventHandlers.

The Asset Editor is a complete mess but it works by modifying text in the editor directly via project.updateFile(). It also expects projectWindows.undoStep() to undo the last updateFile() operation, because CM doesn't have a global undo history. At least I think that's how it works ... I see this is broken in the CM5 version also, so I need to investigate.

I noticed that undo in the asset editor (in both CM5 and CM6 version) lets mod+z undo the underlying file (i.e. the running program would revert to the old sprites), but not undo the asset visualization.

The CM6 undo history is quite capable and has all sorts of capabilities in terms of supporting undo barriers, and having ways to have multiple changes be combined. All that functionality does come we some requirements to call the APIs correctly. (Sometimes naive attempts appear to work at first, but prove flaky.) Also, I was thrown for a loop for a bit when CMD-Z was all wonky. Turned out not having historyKeymap in my list of extensions meant that undo was being handled by the O/S instead of CM6. Once I finally figured that out (and updated the docs for help future self https://github.com/codemirror/basic-setup/pull/5/changes) everything got way better.

I still want to test on mobile devices and look at performance, but I hope to merge this soon! Nice job!
Sounds good. Let me know how that goes. I expect performance will be fine. I'm also hopeful that some of the gutter changes I'm looking at will nibble away at some waste in terms of objection and DOM churn.

- state fields use RangeSet<GutterMarker> instead of simple maps/sets
- lineMarker / lineMarkerChange replaced with gutter() configs and markers
- implement eq() in markers to avoid unnecessary churn
- BREAKPOINT_MARKER CURRENT_PC_MARKER reusable across all lines
- removed the unsused shownErrorLinesField
Add gutter `initialSpacer`s
value now renders after the end of
the line, instead of breaking it up
@fredsa
Copy link
Author

fredsa commented Feb 21, 2026

  • I'm focused on the gutter right now which has some unnecessary excess churn in DOM element creation as you move around the document on scroll. I'm just figuring out the best way to address it.
  • The PR current has the gutter keeping track of line numbers. Turns out this is not ideal as this causes a bit of lag when say inserting a line above an error marker or breakpoint before the marker moves to the correct line. The right way to this is apparently managing ranges, which CM can automatically update as the document is mutated. Again, just looking into the best way to do this.

For both I should have something shortly.

Made improvements to gutter around performance, and leaning into CM6 approach.

I like the new breakpoint marker, and having a visual indicator when you Run To Line. Maybe the breakpoint and current PC gutters could be adjacent for a bigger hit target? And maybe a cursor change or highlight when you move the pointer over the gutter.

I had the error gutter in between the breakpoint gutter and currentpc gutter, which made finding the right place to click feel bad. I merged error and breakpoint gutter (breakpoints hide while there are errors).

Potential breakpoints now appear on hover as red dot with 50% opacity.

TODO: consider whether hover and setting of breakpoints is limited to lines that have an offset (i.e. valid breakpoint)

I noticed that the gutters expand and push the editor to the right after the first compilation completes. Actually, the gutters expand when you scroll down into a region that has gutter elements. It's a little distracting on first load but I can't think of an easy solution, that seems to just be the way CM6 works.

Should be fixed. Initial space bytes is hardcoded to "00 00 00", offset to "0000" and clock to "00". Wider values will automaticall force a wider gutter. If there are platforms that need more bytes (or fewer), it shouldn't be too hard to plumb those values through. I haven't looked closely enough to know the answer, but I seem to recall one system where at least the disassembly view had offsets > two bytes.

TODO I did not address display of more than three bytes, so the current behavior of just showing the first three bytes is untouched.

To maintain consistent status column width, I used "ⓧ" as the initialSpacer, since even with a monospace font, "ⓧ" is wider than "●".

I wish there was a way to make error tooltips show up on pointer move, but I don't think CM6 has much to do with this, since the marker just uses the title attribute.

The native browser tooltip does show up, but it's only after 1000ms or so. I also thought it was not showing, so I'm clearly not patient enough.

A custom styled tooltip that appears immediately is possible.

Unrelated, filed #212 (the issue is more easily recreated in CodeMirror 6) as a TODO.

@fredsa
Copy link
Author

fredsa commented Feb 21, 2026

Implemented multiple breakpoints, except for MAME

In trying to implement MAME…

TBH, I can't tell if MAME breakpoints are entirely broken (both CM5 and CM6 versions). Whether I single step or set a specific breakpoint, MAME rarely stops where I ask it to. It does run for a bit and stop somewhere. I can't identify a
pattern. Perhaps there are limitations I'm not understanding. Or maybe it's broken for some simple reason such as
this code not passing the value correctly.

function mamedbg.runTo(addr)
  debugger:command(string.format("g %x", addr))
  mamedbg.start()
end

I tried %X and %d as quick sanity checks, but didn't change much.

Not sure about the right docs, but I'm guessing https://docs.mamedev.org/debugger/execution.html is relevant.
If so, looks like https://docs.mamedev.org/debugger/breakpoint.html offers multiple breakpoints.

Thoughts?

@fredsa
Copy link
Author

fredsa commented Feb 21, 2026

Other than the couple of TODO I mentioned, I think it's all looking pretty good.

Let me know if you have a short list of "must fix before accepting the PR" and I/we can focus on that.

@sehugg
Copy link
Owner

sehugg commented Feb 21, 2026

Agreed that some improvement is needed there. Having the CurrentPC gutter be clickable was a bit of a temporary implementation as I was imaging that the user would focus on toggle (multiple) breakpoints.

The clickable breakpoint gutter is not too bad now that it has the cursor affordance. And it's cool how you implemented breakpoints with runToPC()!

Even though the breakpoints are useful I am finding that I miss the old click-the-offset-run-to-PC method which was good for quickly iterating through loops. Can they co-exist?

I wonder if we can lose line numbers to free up gutter space? I like the aesthetics, but I don't know if they're useful since we have error markers.

Are 3 bytes (opcode + operand) always enough? And, still okay with the gutter surpressing bytes? For example, https://8bitworkshop.com/v3.12.1/?platform=c64&file=hello.dasm contains byte "HELLO```WORLDa", but the gutter only shows the first 3 bytes. We can keep that, and can definitely keep the gutter to min/max 3 bytes (or a different number of bytes for different platforms). Adding an ellipsis to the bytes gutter that shows all the values would be nice. Another cool feature would be to allow user highlighting of byte "HELLO```WORLDa" and see the value below, the same way that variable values are shown when highlighted.

3 bytes is enough for 6502 instructions, the Z80 sometimes uses 4 bytes but the spaces are removed so they take up the same number of chars.

I like the ellipsis idea, I can't guarantee the assembler listing files are parsed properly so that it'll always work -- sometimes it splits across multiple lines or just omits bytes.

Oh and variable highlights: currently they show up on an extra line. In the production version you can copy/paste that value. In the CM6 version you currently can't. I think probably we can fix this, but I wonder if you think of this as an important feature or just a random useless thing that happens to be possible.

I don't think that's a big deal. It'd be nice to show live variables in a tooltip or overlay or something, there's no type information though so you have to guess what format to display. I end up using the Memory Browser for that kind of thing.

@sehugg
Copy link
Owner

sehugg commented Feb 21, 2026

TBH, I can't tell if MAME breakpoints are entirely broken (both CM5 and CM6 versions). Whether I single step or set a specific breakpoint, MAME rarely stops where I ask it to.

I was never able to get MAME debugging working properly, apparently it goes into a separate event loop when debugging and that causes all kinds of problems. I hear someone got it working by using an Emscripten option that invokes the Relooper algorithm or something, but I ran out of patience with it. I was considering removing debugging support from MAME altogether.

New `node_modules` target, ensures `npm install`
has run before attempting to build grammars with
`./node_modules/.bin/lezer-generator`
New `submodules` target that ensures
`git submodule update --init --recursive` runs before `buildtsc`
Avoids spurious error messages when `tsweb` is run after fresh
checkout by depending on `submodules` and `buildgrammars`.
This ensures both are fully finished before spawn several child
processes that will race each other.
@fredsa
Copy link
Author

fredsa commented Feb 22, 2026

  • MAME: added setting of multiple breakpoint, though it's still quite buggy; have spent several hours, so I might also give up; leaving code here for just a bit longer in case there's a breakthrough
  • Makefile node_modules and submodules target, used as new deps
  • Gutter current PC pointer is now clickable and acts like runToLine

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