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

Make CodeMirror the default editor #3560

Closed
4 tasks done
laurent22 opened this issue Jul 26, 2020 · 9 comments
Closed
4 tasks done

Make CodeMirror the default editor #3560

laurent22 opened this issue Jul 26, 2020 · 9 comments
Labels
desktop All desktop platforms enhancement Feature requests and code enhancements high High priority issues

Comments

@laurent22
Copy link
Owner

laurent22 commented Jul 26, 2020

This issue is to track the few remaining issues with CodeMirror that would need to be fixed before we can make it the default Markdown editor.

@laurent22 laurent22 added desktop All desktop platforms enhancement Feature requests and code enhancements high High priority issues labels Jul 26, 2020
@laurent22
Copy link
Owner Author

@CalebJohn, those are the only three "blocking" issues I could think of. If you can think of any other issue that should be fixed before making CodeMirror the default, please let me know.

@CalebJohn
Copy link
Collaborator

I have a small list of known issues that I don't consider to be blocking.

  • multicursor doesn't work for the extended functionality that I added
  • Shift+Tab doesn't work properly with - [ ] lists
  • Currently extra css is loaded in index.html, but it should be loaded by CodeMirror.tsx

One maybe more serious issue is that $ breaks the syntax highlighting (this is because it starts a katex block). The same thing happens with ``` but that's not as serious because you wouldn't use it in normal notes (for example a user writing about money would be annoyed that $20 breaks highlighting).

@laurent22
Copy link
Owner Author

I agree the three issues you've listed would be nice to fix but not required to make the editor the default. The multicursor in particular is not really something we need to support. In fact would it make sense to disable the feature for now?

One maybe more serious issue is that $ breaks the syntax highlighting (this is because it starts a katex block). The same thing happens with ``` but that's not as serious because you wouldn't use it in normal notes (for example a user writing about money would be annoyed that $20 breaks highlighting).

Do you have an example of Markdown code that would break?

@CalebJohn
Copy link
Collaborator

CalebJohn commented Jul 30, 2020

Remember to pay $10 for field trip

The rest of this not should use markdown **formatting** but it no longer works

# not even headers

image

It's only broken in the codemirror editor of course, not the viewer.

In fact would it make sense to disable the feature for now?

As far as the list indenting (and other plugins) is concerned I'll probably just disable it since I'm sure users don't rely on it.

@CalebJohn
Copy link
Collaborator

@laurent22 All of these issues have now been addressed. What's the path forward? Should we wait for 1 more release of beta? I can make a pull swapping the default whenever we're ready for that.

@PhiLhoSoft
Copy link

What is the motivation behind using CodeMirror? What does that solve?

I can see some pluses (moving with Ctrl+left/right arrow is more logical), but some minuses as well, like lack of shortcut for duplicating a line (I suppose you can add it easily), breaking some userchrome.css, the issues above, and so on.
Just curious of the reason for this change.

Playing a bit with the editor, I see it manages Unicode characters better: I have a page of emoji followed by a tab followed by the meaning, Ace has issues moving between these, CM behaves better.
And it has a different syntax highlighting, but it isn't compelling.

@CalebJohn
Copy link
Collaborator

@PhiLhoSoft Good question!
I would say the primary benefit (from my perspective) is improved unicode support. There were a large number of issues with the Ace editor when using non latin languages (notably Korean, Russian, and Chinese). Which ranged from minor annoyances to being completely unusable (#113 #1351 #3134). This also means users won't be limited to a monospace font.

Other major benefits include:

  1. Codemirror will actually support spellcheck (ace had some issues which meant it would be a major task to add spellcheck)
  2. Ace editor didn't easily support searching within the contents of a note, but it was a comparatively simpler task in codemirror (Desktop: Enable searching in editor rather than the webview for codemirror #3360 is already merged and available)

Overall Codemirror is easier to extend than ace (this is in part because we were relying on the react-ace wrapper). Looking into the future the Joplin project is planning on adding a plugin system which means it will be especially important to have an editor that is easier to configure/extend. Adding a shortcut for duplicating a line is already very easy with Codemirror and will be equally easy to add as a Joplin plugin (once those are available).

You can see a bit of the discussion about the change in the original pull request

@PhiLhoSoft
Copy link

PhiLhoSoft commented Aug 26, 2020

Many thanks for detailing this. I didn't find an issue for this, so I hijacked this one…

Unicode support is compelling for non-roman language users, indeed. And as I pointed out, Ace has issues with emoji, at least.
And spellcheck is what I miss a bit from coming from Simplenote (which added it only recently, and alas doesn't allow to switch languages on note basis -- I write in English and in French, depending on note nature).

So it is a good move.

@laurent22
Copy link
Owner Author

@laurent22 All of these issues have now been addressed. What's the path forward? Should we wait for 1 more release of beta? I can make a pull swapping the default whenever we're ready for that.

Yes looks like it's ready now so if you could make a pull request to make it default that would be great.

We can keep the Ace Editor code in there for now, just in case, but I expect we'll delete it in a few versions. We need to switch to CodeMirror before the UI update as it means less components to update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop All desktop platforms enhancement Feature requests and code enhancements high High priority issues
Projects
None yet
Development

No branches or pull requests

3 participants