[UI] Adds CodeMirror as editor

[jpelay]

Description

Adds CodeMirror as editor in Hedy. For now it replaces Ace everywhere, but we're discussing only doing it for languages where Ace is not usable. The functions with //pass in them are temporary, as I work through them, and the same goes for debug console.logs. The style that I'm using is the default Dark Style provided from Code Mirror, but this can be leter edited to mimic Monokai, which is the one we use for Ace.

The editor, as of right now, can be used with the big caveat that it doesn't have syntax highlighting.

Things that I still need to do:

• Properly configure the editor to hande right-to-left languages.
• Add markers to the editor, crucial for highlighting the line that has an error after compiling.
• Integrate the debugger with CodeMirror, for this the previous task needs to be done.
• Investigate about adding syntax highlighting, and how long can it take.

Fixes #4461

Always link the number of the issue or of the discussion that your PR concerns.
Tip, if you use the word fixes before the issue number in this description, the related issue will automatically close then the PR is merged!

How to test

Cypress tests will invariably fail, because the tests for the editor that we've written use Ace native classes and, as such, are really coupled with Ace. I need to work on this once the work with the editor is completed.

As of right now, you can test it by going to the editor and testing its different options!

[jpelay]

This is how the editor looks right now!

[jpelay]

I think is also worth it that I mention here some of the key differences between Ace and CodeMirror. The later uses a functional mindset in its configuration and the way it handles updates in the state of the editor. In Ace you have an instance of AceAjax.Editor and this object has different functions that manipulates it's state. For example, if I want to change the contents of the editor in Ace I can just do this:

this._editor.setValue(content, MOVE_CURSOR_TO_END);

Whereas in CodeMirror you have to make a transaction that will hold the value of the new contents for the editor and then dispatch that transtaction to actually mute the state. That looks like this:

let transaction = this.view.state.update({ changes: { from: 0, to: this.view.state.doc.length, insert: content } });
this.view.dispatch(transaction);

This philosophy extends to things like the style of the editor, or even if it can be edited or not. For example, CodeMirror doesn't take the styles or contents of its HTML parent, so the styles applied to it don't trickle down to the editor, therefore we need to change the pixel value for the style of the theme applied to the editor, and then dispatch that change as a reconfiguration, that look like this:

this.themeStyles['&'].height = `${newHeight}px`;
this.view.dispatch({
    effects: this.theme.reconfigure(EditorView.theme(this.themeStyles))
});

This makes the editor a little bit more difficult to work with, but has the benefits of being more configurable.

[jpelay]

Relevant thing that I just found out about CodeMirror: the Tab key doesn't add indentation. They explain their reasons not to do so here: https://codemirror.net/examples/tab/, but basically it's because of inclusion, due to some users navigating the site without a pointing device, and recurring to tab in order to move between items.

We can change this behavior and make it so that when the user presses Esc and then Tab, it does switches to the next element and then Tab would work like we'd expect from an editor (this is what Ace does), but is an important consideration whether we want to do this or not, what do you guys think?

[jpelay]

Hi @Felienne!

This is my plan to move this PR forward. For us to be able to start working on the markers (these are the decorations in the editor such as highlighting errors, or a debugger line) in this PR, #4508 needs to be merged. Therefore in this PR I will work focus now on including a button to switch between CodeMirror and Ace when the user has set the Arab language.

Since I think that will be relatively easy and I want to keep working on this, I will work on the markers in a branch on my fork: https://github.com/jpelay/hedy/tree/codemiror-markers, based on the improvements to the interface made in #4508. This way I will be able to learn how markers work in CodeMirror without cluttering this PR with unmerged commits.

What do you think about this plan? Is it ok?

[Felienne]

Hi @Felienne!

This is my plan to move this PR forward. For us to be able to start working on the markers (these are the decorations in the editor such as highlighting errors, or a debugger line) in this PR, #4508 needs to be merged. Therefore in this PR I will work focus now on including a button to switch between CodeMirror and Ace when the user has set the Arab language.

Since I think that will be relatively easy and I want to keep working on this, I will work on the markers in a branch on my fork: https://github.com/jpelay/hedy/tree/codemiror-markers, based on the improvements to the interface made in #4508. This way I will be able to learn how markers work in CodeMirror without cluttering this PR with unmerged commits.

What do you think about this plan? Is it ok?

Sounds good! But I also just approved #4508, so if you are ready, you can continue there.

[jpelay]

Advancements with error hihglighting! It does not look pretty with CodeMirror actual theme, so I'll change that :3

error.mp4

[jpelay]

There's a bug right now where sometimes we know the line of the error, but we don't color it, because I think our logic to highlight the word must be flawed. Ace probably fails silently and just does not highlight the error, but in CodeMirror it errors out:
For example in this code:

The error is in line 3, position 9 (the print), but the 9 refers to the last character of the print. I replicated the logic that we use in Ace in CodeMirror and it tells me this:

As you can see in Ace this error is not being highlighted:

I'm going to change make it work here in CodeMirror, and I have the feeling that fix will also work for Ace, and will change that in a separate PR. At least if we can't find color the appropriate word, we should highlight the entire line.

[jpelay]

What do you think about testing the highlighter like this @Felienne? I think in the future I might make a multilevel tester, like the one we have for the Lark gramars.

Also, do you think it's a priority to test the highlighter for other languages, like we do with Lark as well?

[jpelay]

This questions can of course be debated with everyone on our meeting 😄

[Felienne]

This seems similar to what we had but more readable, so great!

And yeah I can imagine we want multi-level testers just to save you work?

[Felienne]

I think is also worth it that I mention here some of the key differences between Ace and CodeMirror. The later uses a functional mindset in its configuration and the way it handles updates in the state of the editor. In Ace you have an instance of AceAjax.Editor and this object has different functions that manipulates it's state. For example, if I want to change the contents of the editor in Ace I can just do this:

this._editor.setValue(content, MOVE_CURSOR_TO_END);

Whereas in CodeMirror you have to make a transaction that will hold the value of the new contents for the editor and then dispatch that transtaction to actually mute the state. That looks like this:

let transaction = this.view.state.update({ changes: { from: 0, to: this.view.state.doc.length, insert: content } });
this.view.dispatch(transaction);

This philosophy extends to things like the style of the editor, or even if it can be edited or not. For example, CodeMirror doesn't take the styles or contents of its HTML parent, so the styles applied to it don't trickle down to the editor, therefore we need to change the pixel value for the style of the theme applied to the editor, and then dispatch that change as a reconfiguration, that look like this:

this.themeStyles['&'].height = `${newHeight}px`;
this.view.dispatch({
    effects: this.theme.reconfigure(EditorView.theme(this.themeStyles))
});

This makes the editor a little bit more difficult to work with, but has the benefits of being more configurable.

Just bumping this here so we remember to add something like this to the docs?

[jpelay]

Just bumping this here so we remember to add something like this to the docs?

We definitely need!

[Felienne]

LET's GOOOOOOOOOO! We can add the rest of the tests later, as discussed today in the meeting

[mergify]

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

[jpelay]

LET's GOOOOOOOOOO! We can add the rest of the tests later, as discussed today in the meeting

AAAAAAAAA I'm free hahaha

[mergify]

⚠️ The sha of the head commit of this PR conflicts with #4755. Mergify cannot evaluate rules on this PR. ⚠️