Skip to content

Commit

Permalink
Fixes multiple issues with the Jupyter LabExtension for MATLAB.
Browse files Browse the repository at this point in the history
* Fixing bug where chained transposes would be highlighted incorrectly, by swapping ? (zero or one) with * (any number). Also adding test for same bug.
* Fixing bug with building on some Windows systems. In the Lezer build, we need to copy some files, but only on Windows - we attempt to copy, but fall back to || true on Linux, which does nothing but does not fail the build.
* In comment parsing, using a regex to match whitespace, instead of just matching tabs and spaces.
* Adding highlighting for system commands, choosing meta as the most appropriate tag: "Metadata or meta-instruction."
* Adding tests for parsing system commands.
  • Loading branch information
philipc-mw authored and Prabhakar Kumar committed Sep 5, 2024
1 parent d6be9b6 commit 1a1ced6
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 29 deletions.
1 change: 1 addition & 0 deletions .github/workflows/run-unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ on:
# Reusable workflow
# Trigger on workflow call
workflow_call:
workflow_dispatch:

jobs:
matlab_unit_tests:
Expand Down
2 changes: 1 addition & 1 deletion src/jupyter_matlab_labextension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"build:labextension": "jupyter labextension build .",
"build:labextension:dev": "jupyter labextension build --development True .",
"build:lib": "jlpm build:lezer && tsc",
"build:lezer": "cd src/lezer-matlab && npm run build",
"build:lezer": "cd src/lezer-matlab && npm install",
"clean": "jlpm clean:lib",
"clean:lib": "rimraf lib tsconfig.tsbuildinfo",
"clean:lintcache": "rimraf .eslintcache .stylelintcache",
Expand Down
11 changes: 5 additions & 6 deletions src/jupyter_matlab_labextension/src/lezer-matlab/package.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

{
"name": "@lezer/matlab",
"version": "1.0.0",
Expand All @@ -18,19 +17,19 @@
"license": "SEE LICENSE IN LICENSE.md",
"devDependencies": {
"@lezer/generator": "^1.0.0",
"@rollup/plugin-node-resolve": "^9.0.0",
"mocha": "^10.2.0",
"rollup": "^2.52.2",
"@rollup/plugin-node-resolve": "^9.0.0"
"rollup": "^2.52.2"
},
"dependencies": {
"@lezer/common": "^1.2.0",
"@lezer/lr": "^1.0.0",
"@lezer/highlight": "^1.0.0"
"@lezer/highlight": "^1.0.0",
"@lezer/lr": "^1.0.0"
},
"scripts": {
"build": "lezer-generator src/matlab.grammar -o src/parser && rollup -c && npm run copy-lezer-files-to-build-on-windows",
"build-debug": "lezer-generator src/matlab.grammar --names -o src/parser && rollup -c && npm run copy-lezer-files-to-build-on-windows",
"copy-lezer-files-to-build-on-windows": "cp src/*.js dist/",
"copy-lezer-files-to-build-on-windows": "copy src\\*.js dist\\ || true",
"prepare": "npm run build",
"test": "mocha test/test-*.js"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import { styleTags, tags as t } from '@lezer/highlight';
// https://lezer.codemirror.net/docs/ref/#highlight.styleTags
export const matlabHighlighting = styleTags({
Keyword: t.keyword,
VariableName: t.variableName,
Identifier: t.variableName,
LineComment: t.comment,
MultilineComment: t.comment,
SystemCommand: t.meta,
String: t.string,
'( )': t.paren,
'[ ]': t.squareBracket,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
@top Script { expression* }

expression {
VariableName |
Identifier |
String |
MultilineComment |
keyword |
Expand All @@ -14,26 +14,26 @@ expression {
// See https://lezer.codemirror.net/docs/guide/ for documentation on syntax
// specific to @tokens blocks, and how it differs from regular expression syntax.
@tokens {
VariableName { $[a-zA-Z0-9_]+ $[a-zA-Z0-9_']? }
Identifier { $[a-zA-Z0-9_]+ $[a-zA-Z0-9_']* }
charVector { '"' (!["\n])* '"' }
stringArray { "'" (!['\n])* "'" }
SystemCommand { "!" (![\n])* }
Symbol { "+" | "-" | "*" | "=" | ";" | ":" | "(" | ")" | "{" | "}" | "[" | "]" }
space { @whitespace+ }
@precedence { SystemCommand, VariableName }
@precedence { SystemCommand, Identifier }
@precedence { SystemCommand, space }
@precedence { VariableName, charVector }
@precedence { VariableName, stringArray }
@precedence { Identifier, charVector }
@precedence { Identifier, stringArray }
}

String { charVector | stringArray }

// Once a string has been parsed and found to be a VariableName, it will then
// Once a string has been parsed and found to be a Identifier, it will then
// be tested against its specialize table, to test if it is a keyword.
// The keyword node name is "Keyword".
// https://lezer.codemirror.net/docs/guide/#token-specialization
keyword {
@specialize[@name=Keyword]<VariableName, "break" | "case" | "classdef" | "continue" | "global" | "otherwise" | "persistent" | "return" | "spmd" | "arguments" | "enumeration" | "events" | "for" | "function" | "if" | "methods" | "parfor" | "properties" | "try" | "while" | "elseif" | "else" | "end" | "switch" | "catch">
@specialize[@name=Keyword]<Identifier, "break" | "case" | "classdef" | "continue" | "global" | "otherwise" | "persistent" | "return" | "spmd" | "arguments" | "enumeration" | "events" | "for" | "function" | "if" | "methods" | "parfor" | "properties" | "try" | "while" | "elseif" | "else" | "end" | "switch" | "catch">
}

@skip { space | LineComment }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,19 @@ const fileStart = -1;
const fileEnd = -1;
const newline = '\n'.charCodeAt(0);
const carriageReturn = '\r'.charCodeAt(0);
const space = ' '.charCodeAt(0);
const tab = '\t'.charCodeAt(0);

const whitespaceArray = [tab, space];
const lineEndArray = [newline, carriageReturn, fileEnd, fileStart];

const isWhitespace = (char) => /\s/.test(char);

const precededByWhitespaceOnly = (input) => {
// Scan from current position to start of line.
// Return False if non-whitespace found.
// Always return input back to where it started.
const startPos = input.pos;
let onlyWhitespace = true;
while (!lineEndArray.includes(input.peek(-1))) {
if (whitespaceArray.includes(input.peek(-1))) {
if (isWhitespace(input.peek(-1))) {
input.advance(-1);
} else {
onlyWhitespace = false;
Expand All @@ -42,7 +41,7 @@ const followedByWhitespaceOnly = (input) => {
const startPos = input.pos;
let onlyWhitespace = true;
while (!lineEndArray.includes(input.peek(0))) {
if (whitespaceArray.includes(input.peek(0))) {
if (isWhitespace(input.peek(0))) {
input.advance(1);
} else {
onlyWhitespace = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,36 @@ ver %{ Invalid as preceded by non-whitespace.

==>

Script(LineComment,String,LineComment,String,VariableName,LineComment,String,MultilineComment)
Script(LineComment,String,LineComment,String,Identifier,LineComment,String,MultilineComment)

# Transpose is not a string delimiter

A1' + A2' + B1

==>

Script(VariableName,Symbol,VariableName,Symbol,VariableName)
Script(Identifier,Symbol,Identifier,Symbol,Identifier)

# Character vectors can be preceded by non whitespace

A1" + A2" + B1

==>

Script(VariableName,String,Symbol,VariableName)
Script(Identifier,String,Symbol,Identifier)

# Transposed transpose

A'' + B'

==>

Script(Identifier,Symbol,Identifier)

# System command with no preceding whitespace

test!test

==>

Script(Identifier,SystemCommand)
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,20 @@ A1'

==>

Script(VariableName)
Script(Identifier)

# Symbol

A + B

==>

Script(VariableName,Symbol,VariableName)
Script(Identifier,Symbol,Identifier)

# System command

test ! echo "Test system command"

==>

Script(Identifier,SystemCommand)
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Keyword in VariableName
# Keyword in Identifier

forest

==>

Script(VariableName)
Script(Identifier)

# String in comment

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import {fileTests} from "@lezer/generator/dist/test"

let N = 10000

let long_file_spec = `Script(${"Keyword,VariableName,Symbol,VariableName,Symbol,VariableName,VariableName,Symbol,VariableName,Symbol,Symbol,Keyword,".repeat(N)}LineComment)`
let long_file_spec = `Script(${"Keyword,Identifier,Symbol,Identifier,Symbol,Identifier,Identifier,Symbol,Identifier,Symbol,Symbol,Keyword,".repeat(N)}LineComment)`
let long_file_input = `
${"for c = 1:100\n\tdisp(c);\nend\n".repeat(N)}
% Long file
`

let long_line_spec = `Script(${"Keyword,VariableName,Symbol,VariableName,Symbol,VariableName,Symbol,VariableName,Symbol,VariableName,Symbol,Symbol,Keyword,Symbol,".repeat(N)}LineComment)`
let long_line_spec = `Script(${"Keyword,Identifier,Symbol,Identifier,Symbol,Identifier,Symbol,Identifier,Symbol,Identifier,Symbol,Symbol,Keyword,Symbol,".repeat(N)}LineComment)`
let long_line_input = `
${"for c = 1:100;\tdisp(c);end;".repeat(N)}
% Long line
Expand Down

0 comments on commit 1a1ced6

Please sign in to comment.