From acc2117baf9eead4f018452197841b299425dd44 Mon Sep 17 00:00:00 2001 From: Fons van der Plas Date: Wed, 2 Sep 2020 18:29:41 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=82=20Offer=20to=20split=20multiline=20ce?= =?UTF-8?q?lls=20(#335)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Project.toml | 2 +- frontend/common/OfflineHTMLExport.js | 2 +- frontend/common/UnicodeTools.js | 17 ++- frontend/components/Cell.js | 7 +- frontend/components/CellInput.js | 4 - frontend/components/CellOutput.js | 3 +- frontend/components/Editor.js | 168 ++++++++++++++++----------- frontend/components/ErrorMessage.js | 44 +++++-- frontend/editor.html | 18 +-- frontend/index.html | 10 +- src/analysis/Parse.jl | 22 +++- src/webserver/Dynamic.jl | 1 + 12 files changed, 197 insertions(+), 101 deletions(-) diff --git a/Project.toml b/Project.toml index feb2a3bf9a..532afd8509 100644 --- a/Project.toml +++ b/Project.toml @@ -2,7 +2,7 @@ name = "Pluto" uuid = "c3e4b0f8-55cb-11ea-2926-15256bba5781" license = "MIT" authors = ["Fons van der Plas ", "Mikołaj Bochenski "] -version = "0.11.10" +version = "0.11.11" [deps] Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f" diff --git a/frontend/common/OfflineHTMLExport.js b/frontend/common/OfflineHTMLExport.js index fc13baddda..60db32d8de 100644 --- a/frontend/common/OfflineHTMLExport.js +++ b/frontend/common/OfflineHTMLExport.js @@ -13,7 +13,7 @@ export const offline_html = ({ pluto_version, body, head }) => { - + diff --git a/frontend/common/UnicodeTools.js b/frontend/common/UnicodeTools.js index 03435eed5e..93c4eb7a20 100644 --- a/frontend/common/UnicodeTools.js +++ b/frontend/common/UnicodeTools.js @@ -5,7 +5,7 @@ export const length_utf8 = (str, startindex_utf16 = 0, endindex_utf16 = undefine export const utf8index_to_ut16index = (str, index_utf8) => td.decode(te.encode(str).slice(0, index_utf8)).length -export const spliceUtf8 = (original, startindex_utf8, endindex_utf8, replacement) => { +export const splice_utf8 = (original, startindex_utf8, endindex_utf8, replacement) => { // JS uses UTF-16 for internal representation of strings, e.g. // "e".length == 1, "é".length == 1, "🐶".length == 2 @@ -28,4 +28,17 @@ export const spliceUtf8 = (original, startindex_utf8, endindex_utf8, replacement return td.decode(result_enc) } -console.assert(spliceUtf8("e é 🐶 is a dog", 5, 9, "hannes ❤") == "e é hannes ❤ is a dog") +export const slice_utf8 = (original, startindex_utf8, endindex_utf8) => { + // JS uses UTF-16 for internal representation of strings, e.g. + // "e".length == 1, "é".length == 1, "🐶".length == 2 + + // Julia uses UTF-8, e.g. + // ncodeunits("e") == 1, ncodeunits("é") == 2, ncodeunits("🐶") == 4 + // length("e") == 1, length("é") == 1, length("🐶") == 1 + + const original_enc = te.encode(original) + return td.decode(original_enc.slice(startindex_utf8, endindex_utf8)) +} + +console.assert(splice_utf8("e é 🐶 is a dog", 5, 9, "hannes ❤") === "e é hannes ❤ is a dog") +console.assert(slice_utf8("e é 🐶 is a dog", 5, 9) === "🐶") diff --git a/frontend/components/Cell.js b/frontend/components/Cell.js index c0a801bb67..194ca78622 100644 --- a/frontend/components/Cell.js +++ b/frontend/components/Cell.js @@ -91,7 +91,12 @@ export const Cell = ({ const focusListener = (e) => { if (e.detail.cell_id === cell_id) { if (e.detail.line != null) { - set_cm_forced_focus([{ line: e.detail.line, ch: 0 }, { line: e.detail.line, ch: Infinity }, { scroll: true }]) + const ch = e.detail.ch + if (ch == null) { + set_cm_forced_focus([{ line: e.detail.line, ch: 0 }, { line: e.detail.line, ch: Infinity }, { scroll: true }]) + } else { + set_cm_forced_focus([{ line: e.detail.line, ch: ch }, { line: e.detail.line, ch: ch }, { scroll: true }]) + } } } } diff --git a/frontend/components/CellInput.js b/frontend/components/CellInput.js index c6a867cedc..4d5faaf9b3 100644 --- a/frontend/components/CellInput.js +++ b/frontend/components/CellInput.js @@ -199,10 +199,6 @@ export const CellInput = ({ change_handler_ref.current(new_value) }) - // cm.on("focus", () => { - // window.dispatchEvent(new CustomEvent("collapse_cell_selection", {})) - // }) - cm.on("blur", () => { // NOT a debounce: setTimeout(() => { diff --git a/frontend/components/CellOutput.js b/frontend/components/CellOutput.js index c77aafeb55..b7c8e59d81 100644 --- a/frontend/components/CellOutput.js +++ b/frontend/components/CellOutput.js @@ -31,6 +31,7 @@ export class CellOutput extends Component { inline_output: !this.props.errored && !!this.props.body && (this.props.mime == "application/vnd.pluto.tree+xml" || this.props.mime == "text/plain"), })} + mime=${this.props.mime} > ${this.props.rootassignee} <${OutputBody} ...${this.props} /> @@ -62,7 +63,7 @@ const OutputBody = ({ mime, body, cell_id, all_completed_promise, requests }) => case "image/bmp": case "image/svg+xml": const src = URL.createObjectURL(new Blob([body], { type: mime })) - return html`
` + return html`
` break case "text/html": case "application/vnd.pluto.tree+xml": diff --git a/frontend/components/Editor.js b/frontend/components/Editor.js index db876711cc..35403b069c 100644 --- a/frontend/components/Editor.js +++ b/frontend/components/Editor.js @@ -14,6 +14,7 @@ import { link_open_path } from "./Welcome.js" import { empty_cell_data, code_differs } from "./Cell.js" import { offline_html } from "../common/OfflineHTMLExport.js" +import { slice_utf8, length_utf8 } from "../common/UnicodeTools.js" const default_path = "..." @@ -35,30 +36,34 @@ export class Editor extends Component { loading: true, } // convenience method - const set_notebook_state = (updater, callback) => { - this.setState((prevstate) => { - return { - notebook: { - ...prevstate.notebook, - ...updater(prevstate.notebook), - }, - } - }, callback) + const set_notebook_state = (updater) => { + return new Promise((resolve) => { + this.setState((prevstate) => { + return { + notebook: { + ...prevstate.notebook, + ...updater(prevstate.notebook), + }, + } + }, resolve) + }) } this.set_notebook_state = set_notebook_state.bind(this) // convenience method - const set_cell_state = (cell_id, new_state_props, callback) => { - this.setState((prevstate) => { - return { - notebook: { - ...prevstate.notebook, - cells: prevstate.notebook.cells.map((c) => { - return c.cell_id == cell_id ? { ...c, ...new_state_props } : c - }), - }, - } - }, callback) + const set_cell_state = (cell_id, new_state_props) => { + return new Promise((resolve) => { + this.setState((prevstate) => { + return { + notebook: { + ...prevstate.notebook, + cells: prevstate.notebook.cells.map((c) => { + return c.cell_id == cell_id ? { ...c, ...new_state_props } : c + }), + }, + } + }, resolve) + }) } this.set_cell_state = set_cell_state.bind(this) @@ -71,8 +76,8 @@ export class Editor extends Component { // these are things that can be done to the local notebook this.actions = { - add_local_cell: (cell, new_index, callback = undefined) => { - set_notebook_state((prevstate) => { + add_local_cell: (cell, new_index) => { + return set_notebook_state((prevstate) => { if (prevstate.cells.some((c) => c.cell_id == cell.cell_id)) { console.warn("Tried to add cell with existing cell_id. Canceled.") console.log(cell) @@ -84,45 +89,40 @@ export class Editor extends Component { return { cells: [...before.slice(0, new_index), cell, ...before.slice(new_index)], } - }, callback) + }) }, - update_local_cell_output: (cell, { output, running, runtime, errored }, callback = undefined) => { + update_local_cell_output: (cell, { output, running, runtime, errored }) => { this.counter_statistics.numRuns++ - set_cell_state( - cell.cell_id, - { - running: running, - runtime: runtime, - errored: errored, - output: { ...output, timestamp: Date.now() }, - }, - callback - ) + return set_cell_state(cell.cell_id, { + running: running, + runtime: runtime, + errored: errored, + output: { ...output, timestamp: Date.now() }, + }) }, - update_local_cell_input: (cell, by_me, code, folded, callback = undefined) => { - set_cell_state( - cell.cell_id, - { - remote_code: { - body: code, - submitted_by_me: by_me, - timestamp: Date.now(), - }, - code_folded: folded, + update_local_cell_input: (cell, by_me, code, folded) => { + return set_cell_state(cell.cell_id, { + remote_code: { + body: code, + submitted_by_me: by_me, + timestamp: Date.now(), }, - callback - ) + local_code: { + body: code, + }, + code_folded: folded, + }) }, - delete_local_cell: (cell, callback = undefined) => { + delete_local_cell: (cell) => { // TODO: event listeners? gc? - set_notebook_state((prevstate) => { + return set_notebook_state((prevstate) => { return { cells: prevstate.cells.filter((c) => c !== cell), } - }, callback) + }) }, - move_local_cells: (cells, new_index, callback = undefined) => { - set_notebook_state((prevstate) => { + move_local_cells: (cells, new_index) => { + return set_notebook_state((prevstate) => { // The set of moved cell can be scatter across the notebook (not necessarily contiguous) // but this action will move all of them to a single cluster // The first cell of that cluster will be at index `new_index`. @@ -134,7 +134,7 @@ export class Editor extends Component { return { cells: [...before, ...cells, ...after], } - }, callback) + }) }, } @@ -363,15 +363,53 @@ export class Editor extends Component { wrap_remote_cell: (cell_id, block = "begin") => { const cell = this.state.notebook.cells.find((c) => c.cell_id == cell_id) const new_code = block + "\n\t" + cell.local_code.body.replace(/\n/g, "\n\t") + "\n" + "end" - set_cell_state(cell_id, { - remote_code: { - body: new_code, - submitted_by_me: false, - timestamp: Date.now(), - }, - }) + this.actions.update_local_cell_input(cell, false, new_code, cell.code_folded) this.requests.change_remote_cell(cell_id, new_code) }, + split_remote_cell: async (cell_id, boundaries, submit = false) => { + const index = this.state.notebook.cells.findIndex((c) => c.cell_id == cell_id) + const cell = this.state.notebook.cells[index] + + const old_code = cell.local_code.body + const padded_boundaries = [0, ...boundaries] + const parts = boundaries.map((b, i) => slice_utf8(old_code, padded_boundaries[i], b).trim()).filter((x) => x !== "") + + const new_ids = [] + + // for loop because we need to wait for each addition to finish before adding the next, otherwise their order would be random + for (const [i, part] of parts.entries()) { + if (i === 0) { + new_ids.push(cell_id) + } else { + const update = await this.requests.add_remote_cell_at(index + i, true) + this.client.on_update(update, true) + new_ids.push(update.cell_id) + } + } + + await Promise.all( + parts.map(async (part, i) => { + const id = new_ids[i] + + // we set the cell's remote_code to force its value + await this.actions.update_local_cell_input({ cell_id: id }, false, part, false) + + // we need to reset the remote_code, otherwise the cell will falsely report that it is in sync with the remote + const new_state = this.state.notebook.cells.find((c) => c.cell_id === id) + await this.set_cell_state(id, { + remote_code: { + ...new_state.remote_code, + body: i === 0 ? old_code : "", + }, + }) + }) + ) + + if (submit) { + const cells = new_ids.map((id) => this.state.notebook.cells.find((c) => c.cell_id == id)) + await this.requests.set_and_run_multiple(cells) + } + }, interrupt_remote: (cell_id) => { set_notebook_state((prevstate) => { return { @@ -425,19 +463,20 @@ export class Editor extends Component { this.requests.add_remote_cell(cell_id, "after") } const index = this.state.notebook.cells.findIndex((c) => c.cell_id == cell_id) + const cell = this.state.notebook.cells[index] this.setState({ recently_deleted: { index: index, body: this.state.notebook.cells[index].local_code.body, }, }) + set_cell_state(cell_id, { running: true, - remote_code: { - body: "", - submitted_by_me: false, - }, + }).then(() => { + this.actions.update_local_cell_input(cell, false, "", true) }) + this.client.send( "delete_cell", {}, @@ -814,7 +853,6 @@ export class Editor extends Component { detail: { cell_id: this.state.notebook.cells[new_i].cell_id, line: delta === -1 ? Infinity : -1, - // ch: delta === -1 ? Infinity : -1, }, }) ) @@ -865,7 +903,7 @@ export class Editor extends Component { on_click=${() => { this.requests.add_remote_cell_at(this.state.recently_deleted.index, true).then((update) => { this.client.on_update(update, true) - this.actions.update_local_cell_input({ cell_id: update.cell_id }, false, this.state.recently_deleted.body, false, () => { + this.actions.update_local_cell_input({ cell_id: update.cell_id }, false, this.state.recently_deleted.body, false).then(() => { this.requests.change_remote_cell(update.cell_id, this.state.recently_deleted.body) }) }) diff --git a/frontend/components/ErrorMessage.js b/frontend/components/ErrorMessage.js index c4bae7900a..5b0d7e8e15 100644 --- a/frontend/components/ErrorMessage.js +++ b/frontend/components/ErrorMessage.js @@ -39,16 +39,40 @@ export const ErrorMessage = ({ msg, stacktrace, cell_id, requests }) => { const rewriters = [ { pattern: /syntax: extra token after end of expression/, - display: () => - html`

Multiple expressions in one cell.

- { - e.preventDefault() - requests.wrap_remote_cell(cell_id, "begin") - }} - >Wrap all code in a begin ... end block.`, + display: (x) => { + console.log(x) + const begin_hint = html` { + e.preventDefault() + requests.wrap_remote_cell(cell_id, "begin") + }} + >Wrap all code in a begin ... end block.` + if (x.includes("\n\nBoundaries: ")) { + const boundaries = JSON.parse(x.split("\n\nBoundaries: ")[1]).map((x) => x - 1) // Julia to JS index + console.log(boundaries) + const split_hint = html`

+ { + e.preventDefault() + requests.split_remote_cell(cell_id, boundaries, true) + }} + >Split this cell into ${boundaries.length} cells, or +

` + return html`

Multiple expressions in one cell.

+

How would you like to fix it?

+
    +
  • ${split_hint}
    • +
  • ${begin_hint}
    • +
` + } else { + return html`

Multiple expressions in one cell.

+

${begin_hint}

` + } + }, }, { pattern: /LoadError: cannot assign a value to variable workspace\d+\..+ from module workspace\d+/, diff --git a/frontend/editor.html b/frontend/editor.html index 3ef6686ab4..3ea7d8e38b 100644 --- a/frontend/editor.html +++ b/frontend/editor.html @@ -19,15 +19,15 @@ - - - - - - - - - + + + + + + + + + diff --git a/frontend/index.html b/frontend/index.html index 484360a2a4..139f303c91 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -19,11 +19,11 @@ - - - - - + + + + + diff --git a/src/analysis/Parse.jl b/src/analysis/Parse.jl index 0767deb941..ca261a24a3 100644 --- a/src/analysis/Parse.jl +++ b/src/analysis/Parse.jl @@ -19,7 +19,7 @@ function parse_custom(notebook::Notebook, cell::Cell)::Expr if (ex isa Expr) && (ex.head == :toplevel) # if there is more than one expression: if count(a -> !(a isa LineNumberNode), ex.args) > 1 - Expr(:error, "extra token after end of expression") + Expr(:error, "extra token after end of expression\n\nBoundaries: $(expression_boundaries(cell.code))") else ex end @@ -35,7 +35,7 @@ function parse_custom(notebook::Notebook, cell::Cell)::Expr # only whitespace or comments after the first expression parsed1 else - Expr(:error, "extra token after end of expression") + Expr(:error, "extra token after end of expression\n\nBoundaries: $(expression_boundaries(cell.code))") end end @@ -51,6 +51,24 @@ function parse_custom(notebook::Notebook, cell::Cell)::Expr Expr(topleveled.head, topleveled.args[1], preprocess_expr(topleveled.args[2])) end +"""Get the list of string indices that denote expression boundaries. + +# Examples + +`expression_boundaries("sqrt(1)") == [ncodeunits("sqrt(1)") + 1]` + +`expression_boundaries("sqrt(1)\n\n123") == [ncodeunits("sqrt(1)\n\n") + 1, ncodeunits("sqrt(1)\n\n123") + 1]` + +""" +function expression_boundaries(code::String, start=1)::Array{<:Integer,1} + expr, next = Meta.parse(code, start, raise=false) + if next <= ncodeunits(code) + [next, expression_boundaries(code, next)...] + else + [next] + end +end + "Make some small adjustments to the `expr` to make it work nicely inside a timed, wrapped expression: 1. If `expr` is a `:toplevel` expression (this is the case iff the expression is a combination of expressions using semicolons, like `a = 1; b` or `123;`), then it gets turned into a `:block` expression. The reason for this transformation is that `:toplevel` does not return/relay the output of its last argument, unlike `begin`, `let`, `if`, etc. (But we want it!) diff --git a/src/webserver/Dynamic.jl b/src/webserver/Dynamic.jl index 157a36aba8..e26b146790 100644 --- a/src/webserver/Dynamic.jl +++ b/src/webserver/Dynamic.jl @@ -48,6 +48,7 @@ end responses[:delete_cell] = (session::ServerSession, body, notebook::Notebook, cell::Cell; initiator::Union{Initiator,Missing}=missing) -> let to_delete = cell + to_delete.code_folded = true # replace the cell's code with "" and do a reactive run change_remote_cellinput!(session, notebook, to_delete, "", initiator=initiator) runtask = update_save_run!(session, notebook, [to_delete]; run_async=true)::Task