Monaco #2117

bcolloran · 2019-08-01T07:27:00Z

This may now be working at parity with codemirror, but i need extra eyes and review to kick the tires. I guess the one thing that is for sure not working is the fetch chunk code highlighting mode. i can look at that next.

Pull Request checklist

Documentation: If this feature has or requires documentation, the relevant docs have been updated.
Changelog: This PR updates the changelog with any user-visible changes.
Tests: This PR includes thorough tests or an explanation of why it does not

…ng, webpack need massive cleanup

…naco

(make the monaco component fully controlled)

- indent/dedent - commenting - autoSurround ...

…king

bcolloran · 2019-08-05T23:22:55Z

fetch chunk highlighting now working. last explicit bug i'm now aware of: chunk delim lines without an explicit type don't highlight correctly.

@hamilton @mdboom @wlach we should figure out what the parity-with-codemirror criteria are for Monaco. Getting all the magic of language servers working is still a ways off, but i think i can see a path to some cool stuff that is pretty attainable

mdboom

This is really cool.

Personal preference: I don't know if the overview thing is that helpful -- maybe turn it off to save real estate or make it toggleable, if possible.

The 2 spaces for tabs for Python thing is going to ruffle some feathers. If there's a way to do that on a cell-by-cell basis, that would be awesome, but I couldn't find it in the API docs.

There is some sort of autocompletion going on -- it looks like it pulls words from elsewhere in the document. It's a little weird since it pulls candidates from markdown cells. If it could be limited to looking in cells of the same time (no idea how hard that is) that could be really useful, otherwise, maybe turn it off?

mdboom · 2019-08-06T13:38:41Z

src/editor/state-schemas/language-definitions.js

@@ -19,8 +16,6 @@ const PYODIDE_URL = process.env.USE_LOCAL_PYODIDE
 const pyLanguageDefinition = {
  languageId: "py",
  displayName: "Python",
-  codeMirrorMode: "python",


We'll want to replace this with something for Monaco so that third-party language plugins can get syntax highlighting. Does it look hard to do that dynamically?

i don't think it should be hard, but i think we can address that down the line. there will be a bit more thinking to do on this topic, but not too bad.

wlach

Preliminarily, this seems like a really nice improvement!

I tend to agree that the overview should be skipped. I have a "tabs" sidebar, so every pixel of space in the editor is precious. Here's how it looks for me now:

On a similar note, it would be nice if we used less space for the line numbers -- right now the seem to be taking up 50px of space, most of which is just empty space. I'd rather we just compacted them to just fit the current number of lines, expanding them as needed.

Agree also that the autocompletion doesn't seem to be super useful as is, more often than not the suggestions seem unhelpful / get in the way. I would definitely turn it off until we can make it only pull stuff from the same cell type.

wlach · 2019-08-06T17:58:49Z

webpack.config.js

@@ -55,9 +56,16 @@ module.exports = env => {
    entry: {
      iodide: `${APP_DIR}/editor/index.jsx`,
      "iodide.eval-frame": `${APP_DIR}/eval-frame/index.jsx`,
-      "server.home": `${APP_DIR}/server/index.jsx`
+      "server.home": `${APP_DIR}/server/index.jsx`,
+      // "editor.worker": 'monaco-editor/esm/vs/editor/editor.worker.js'


Leftover from some earlier work?

wlach · 2019-08-06T17:59:09Z

webpack.config.js

+        //   test: /\.css$/,
+        //   include: MONACO_PATH,
+        //   use: [MiniCssExtractPlugin.loader, "css-loader"]
+        // },


Is this a WIP or can we just take it out?

wlach · 2019-08-06T18:00:24Z

src/editor/components/monaco-language-init.js

+  md: [mdLang, mdConf],
+  css: [cssLang, cssConf],
+  py: [pyLang, pyConf]
+}).forEach(lang => {


Do we need to update the language plugin documentation for any new conventions here?

bcolloran · 2019-08-12T23:07:04Z

(oh, also removed the minimap)

wlach · 2019-08-13T03:04:02Z

I deployed this to https://definitely-staging.iodide.io in case anyone wants to give it a whirl. Will give actual feedback tomorrow.

teonbrooks · 2019-08-13T22:28:02Z

so far planning around with the editor, i think the languages shared refs make sense. i understand the inclusion of the fetch cells as well, that is the loaded data gets its variable assignment there, but what i have noticed with this is that it parts of the url are included in the autocomplete list.

bcolloran · 2019-08-14T02:37:03Z

@teonbrooks -- ok i fooled around with it a bit more, and tried to tighten things up a bit. Since I already had the regex handy from something else, i added a filter so that only the variable names in fetch chunks are added to the autocomplete list, and i added a rudimentary heuristic so that only things in css chunks starting with "#" or "." (ids and classes) are added to the list. please take another look when you can and let me know if you see any other merge blockers.

thanks for helping with the QA @teonbrooks !

bcolloran · 2019-08-14T17:55:39Z

webpack.config.js

@@ -141,7 +138,6 @@ module.exports = env => {
        filename: `[name].${APP_VERSION_STRING}.css`
      }),
      new WriteFilePlugin()
-      // Use an external helper script, due to https://github.com/1337programming/webpack-shell-plugin/issues/41


comment for code that was removed

bcolloran · 2019-08-14T17:59:42Z

webpack.config.js

@@ -2,10 +2,8 @@ require("dotenv").config();
 const webpack = require("webpack");
 const path = require("path");
 const fs = require("fs");
-const CreateFileWebpack = require("create-file-webpack");


this and the one below were no longer used. noticed this while cleaning out stuff i'd been doing in this file.

teonbrooks · 2019-08-14T18:31:36Z

@bcolloran that did it for me. looks good!

bcolloran · 2019-08-14T18:41:41Z

note to self: WIP of webpack.config.js with stuff for loading monaco here:
https://gist.github.com/bcolloran/9ad3686b793795bcd03ae285aa24b8e1

wlach

This is working great! I have some minor nits, but in general everything here is looking good.

patches/monaco-editor+0.16.2.patch

src/editor/components/iomd-editor.jsx

src/editor/components/monaco-tools/code-chunk-identifiers.js

src/editor/components/monaco-tools/delim-line-suggestion.js

src/editor/components/iomd-editor.jsx

src/editor/components/monaco-tools/__tests__/code-chunk-identifiers.test.js

src/editor/components/monaco-tools/delim-line-suggestion.js

src/editor/components/monaco-tools/fetch-line-suggestion.js

wlach

Looks great! Thanks for adding more tests and keeping things 💯

bcolloran added 27 commits July 15, 2019 15:37

adding packages

6e4e813

removing codemirror, trying to get monaco to work

7fc0f69

monaco working barely; does not resize when needed, workers not worki…

fba6265

…ng, webpack need massive cleanup

merge fixes 1

3750ddb

more merge conflict fixes

89ab64f

more merge fixes... monaco totally not working

eecb9cf

monaco now loading and auto-sizing, still not hooked up to redux at all

a919a7a

plumbing monaco to redux; changing to 1-based indexing of iomd for Mo…

cc8ded0

…naco

getting selection/multi selection working

7212fd6

language loading working, rudiments of iomd mode coming together

55bba47

patch in mousewheel scroll fix for firefox

5441e8d

update package lock

9797e4e

clean up, extra logging

71d8566

remove forceUpdate from cursor state

1e07986

(make the monaco component fully controlled)

remove codemirror refs added by rebase

be115be

test fix

c348879

removing console.logs

1dbe056

fix word wrap; remove "jsmd" from monarch mode

250b79a

lint fix

bd052b5

iomd mode kind of working, but a lot not working:

363810e

- indent/dedent - commenting - autoSurround ...

Merge branch 'master' into monaco-4

cdc48e6

experimenting with iomd mode conf features

1a9d386

autoIndent, autoSurround, bracket closing/matching all seem to be wor…

45730eb

…king

monaco now basically working at parity with codemirror?...

fe95afe

remove fixme message

5d13597

fix updating of delimLine decorations

c4e6770

basic fetch higlighting working

1f09506

mdboom reviewed Aug 6, 2019

View reviewed changes

wlach reviewed Aug 6, 2019

View reviewed changes

wlach temporarily deployed to iodide-staging August 13, 2019 02:40 Inactive

bcolloran added 2 commits August 13, 2019 13:58

code clean up

56c29dc

more clean up

715b308

reorganized, code clean up, and addressed @teon's issue

f00e162

bcolloran added 2 commits August 13, 2019 19:44

clean up

17700cc

clean up

79de08e

bcolloran commented Aug 14, 2019

View reviewed changes

bcolloran added 2 commits August 14, 2019 11:06

package clean up

a0c981e

remove dead commented code

cff4718

wlach temporarily deployed to iodide-staging August 14, 2019 18:18 Inactive

bcolloran marked this pull request as ready for review August 14, 2019 18:40

wlach suggested changes Aug 14, 2019

View reviewed changes

bcolloran added 2 commits August 14, 2019 15:02

fix wlach's review items

cd4fa01

add tests

48e8359

wlach approved these changes Aug 15, 2019

View reviewed changes

one last review fix

5a8776c

bcolloran merged commit 757bf8f into master Aug 15, 2019

This was referenced Aug 19, 2019

need better find/replace functionality #579

Closed

fix loading of monaco syntax highlighters via language plugin #1279

Open

[bug] dragging Editor Scrollbar not working #1618

Closed

the iodide editor doesn't work with screen readers #1629

Closed

wlach temporarily deployed to iodide-staging December 23, 2019 18:39 Inactive

wlach deleted the monaco-4 branch December 23, 2019 18:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monaco #2117

Monaco #2117

bcolloran commented Aug 1, 2019

bcolloran commented Aug 5, 2019

mdboom left a comment

mdboom Aug 6, 2019

bcolloran Aug 14, 2019

wlach left a comment

wlach Aug 6, 2019

wlach Aug 6, 2019

wlach Aug 6, 2019

bcolloran commented Aug 12, 2019

wlach commented Aug 13, 2019

teonbrooks commented Aug 13, 2019

bcolloran commented Aug 14, 2019

bcolloran Aug 14, 2019

bcolloran Aug 14, 2019

teonbrooks commented Aug 14, 2019

bcolloran commented Aug 14, 2019

wlach left a comment

wlach left a comment

Monaco #2117

Monaco #2117

Conversation

bcolloran commented Aug 1, 2019

Pull Request checklist

bcolloran commented Aug 5, 2019

mdboom left a comment

Choose a reason for hiding this comment

mdboom Aug 6, 2019

Choose a reason for hiding this comment

bcolloran Aug 14, 2019

Choose a reason for hiding this comment

wlach left a comment

Choose a reason for hiding this comment

wlach Aug 6, 2019

Choose a reason for hiding this comment

wlach Aug 6, 2019

Choose a reason for hiding this comment

wlach Aug 6, 2019

Choose a reason for hiding this comment

bcolloran commented Aug 12, 2019

wlach commented Aug 13, 2019

teonbrooks commented Aug 13, 2019

bcolloran commented Aug 14, 2019

bcolloran Aug 14, 2019

Choose a reason for hiding this comment

bcolloran Aug 14, 2019

Choose a reason for hiding this comment

teonbrooks commented Aug 14, 2019

bcolloran commented Aug 14, 2019

wlach left a comment

Choose a reason for hiding this comment

wlach left a comment

Choose a reason for hiding this comment