Skip to content
This repository has been archived by the owner on Jun 15, 2022. It is now read-only.

Missing languages #2

Closed
moughxyz opened this issue Mar 12, 2018 · 6 comments
Closed

Missing languages #2

moughxyz opened this issue Mar 12, 2018 · 6 comments

Comments

@moughxyz
Copy link
Member

  • Java
  • C#
@gshakhn
Copy link
Contributor

gshakhn commented Jun 4, 2018

I recently started using StandardNotes and hit this issue for Scala. Some thoughts are below. Assuming the below works for you, I can put together a PR if you want.

The clike languages aren't working since the individual language definitions are based on the mime type, which isn't stored on the note as far as I can tell. One fix would be to store the CodeMirror name on the note instead of the mode. This would require changing the dropdown list to a list of names and storing it in a name attribute (or codeMirrorModeName to avoid confusion with the title of the note).

The fun part would be migrating existing notes. If the existing note has a codeMirrorModeName field, we're in the new state and can do CodeMirror.findModeByName. If the existing note has a mode field, we're in the old state. The CodeMirror API doesn't expose a findModeByMode method, so we'll have to filter CodeMirror.modeInfo ourselves. The latter is documented, so it should be safe to use.

Some questions:

  1. If the old mode is on the note, we could either save the new codeMirrorModeName immediately (and I'm guessing indirectly modify the modified time), or wait for the user to change the note before saving. Former is easier, but latter is probably better UX. Thoughts?

  2. The existing code tries to do findModeByExtension and findModeByMIME if it thinks the mode is an extension or mime type. With the current code, when would either of those be the case? Am I missing some context when either of those would be saved as the mode?

@moughxyz
Copy link
Member Author

moughxyz commented Jun 4, 2018

Hey @gshakhn, thanks for looking into this. Is it possible to just create a different mode for each clike language? https://github.com/sn-extensions/code-editor/tree/master/vendor/modes/clike

Duplication is fine, if it means not having to tinker with the architecture too much. It looks like we have some if/else to handle the different mime types. Any idea if it would be possible to isolate each one into a separate mode file?

@gshakhn
Copy link
Contributor

gshakhn commented Jun 5, 2018

I was able to hack something together by:

  1. Copying the clike directory and renaming it to scala.
  2. Renaming clike.js to scala.js.
  3. Changing line 49 of scala.js from CodeMirror.defineMode("clike",... to CodeMirror.defineMode("scala",...
  4. Changing line 489 of scala.js from:
  def("text/x-scala", {
    name: "clike",

to

  def("scala", {
    name: "scala",
  1. Changing {name: "Scala", mime: "text/x-scala", mode: "clike", ext: ["scala"]}, to {name: "Scala", mime: "text/x-scala", mode: "scala", ext: ["scala"]}, (I'm not sure if this was actually necessary. Did this early before tweaking the stuff below)

In theory it's possible. In practice, I wouldn't recommend this approach since this is fighting against how CodeMirror wants to work and will make upgrades more painful. Plus you'd be changing the mime type from text/x-scala to scala.

@gshakhn
Copy link
Contributor

gshakhn commented Jun 5, 2018

Alternatively, you could also have a specific branch for each of the clike languages. e.g.

In changeMode:

      const actualMode = mode === 'scala' ? 'clike' : mode;
      const actualSpec = mode === 'scala' ? 'text/x-scala' : spec;
      editor.setOption("mode", actualSpec);
      CodeMirror.autoLoadMode(editor, actualMode);
      if (clientData) {
        clientData.mode = mode;
      }
      document.getElementById("select").selectedIndex = modes.indexOf(mode);

When a new clike language is added, you would have to add an extra branch in addition to adding it to the modes array. Given how often that happens, that's probably easier than my first suggested approach of migrating to storing the name.

@moughxyz
Copy link
Member Author

moughxyz commented Jun 5, 2018

Interesting. I don't have the capacity for this now, so I'll either take a look in the future, or feel free to submit a PR :)

@gshakhn
Copy link
Contributor

gshakhn commented Jun 5, 2018

Submitted #3

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants