Upgrade to CodeMirror 6.

[drgrice1]

Some features of this change are improved syntax highlighting and the addition of autocompletion. To see available autocompletions at the current cursor location, you can type Ctrl-Space. In many cases, autocompletions are offered automatically when certain things have been typed.

CodeMirror 6 can not be used via script tags. So a PGProblemEditor package has been created for this. The repository for that is https://github.com/openwebwork/pg-codemirror-editor. An initial package has been published in npm at
https://www.npmjs.com/package/@openwebwork/pg-codemirror-editor. The real work is done in the codemirror-lang-pg package that adds support for the PG language in CodeMirror 6. The repository for that package is https://github.com/openwebwork/codemirror-lang-pg, and the npm package at https://www.npmjs.com/package/@openwebwork/codemirror-lang-pg.

The pg-problem-editor package is designed so that it can be used by webwork2, the standalone renderer, and possible others that want a PG problem editor. It may need a few tweaks to make it work for webwork3 in the future, but should also work for that.

The codemirror-lang-pg package consists of a base PG parser that is largely derived from the codemirror-lang-perl package (at https://github.com/drgrice1/codemirror-lang-perl and https://www.npmjs.com/package/codemirror-lang-perl) with a modifications specific to the perl available in a PG problem. The main differences are that its escape sequence is ~~, much of what can not be used in the safe compartment has been stripped out, and there are special blocks for PGML, PG text, and LaTeX image code. Then there are two other parsers. Those are the PGML and PG text parsers. Those parsers run on the contents of the PGML and PG text blocks found by the base PG parser. Note that the PGML parser is actually a javascript implementation of the parser in PGML.pl with some modifications to track position and things needed to associate the parsed tree with the original code.

Note that the configuration of theme, keymaps, and such are now part of the pg-codemirror-editor, and are shown in a panel of the editor instead of below as before.

Emacs and Vim keymaps are available, but at this point there is not a Sublime keymap for CodeMirror 6.

The themes that are available include a basic default theme (that is part of the pg-problem-editor package and just adds some syntax highlighting that is missing from the CodeMirror 6 default), the OndDark theme that is the only other theme provided by the CodeMirror 6 developers directly, the themes in the thememirror package, and the themes from https://github.com/craftzdog/cm6-themes which are in several separate npm packages. Note that both thememirror and craftzdog/cm6-themes provide a solarized light theme, and that is why there are two of those. They are slightly different though.

I am sure that there is a lot more work to do on this, and there are other things available with CodeMirror 6 that could probably be implemented. In any case, this is a good start.

[somiaj]

I was looking to add functions from the pg macros to the list of highlighted functions, such as PopUp, DropDown, etc. As I was creating a patch for this, it occurred to me, would it be possible to only highlight key words if the associated macro is loaded? Such as PopUp and DropDown will only be included if parserPopUp.pl keyword is found in the code? This might not be 100% accurate (such as if parserPopUp.pl is in a comment, though I guess we could just ignore comments for this case), or if the macros are loaded inside other macros (which I think is nonstandard for most additional things like parserPopUp.pl).

[drgrice1]

No, you can't really do that in a reliable way. Generally CodeMirror 6 is designed to be non-contextual in its parsing.

[drgrice1]

I should point out that since the elements for selecting the theme, keymap, and such are part of the pg-codemirror-editor package, the strings used for those are not translatable unfortunately. That could be done by adding a translation method for that package, but would require translations for that package.

[Alex-Jordan]

GitHub says this has no conflicts, but I just did:

git checkout develop
git pull
git checkout -b cm6-pg-editor
git merge drgrice1/cm6-pg-editor

and I get merge conflicts.

alex.jordan@vmwebworkdevw02:/opt/webwork/webwork2/htdocs $ git merge drgrice1/cm6-pg-editor
Auto-merging templates/HelpFiles/InstructorPGProblemEditor.html.ep
CONFLICT (content): Merge conflict in templates/HelpFiles/InstructorPGProblemEditor.html.ep
Removing templates/HTML/CodeMirrorEditor/controls.html.ep
Auto-merging htdocs/js/PGCodeMirror/pgeditor.js
CONFLICT (content): Merge conflict in htdocs/js/PGCodeMirror/pgeditor.js
Removing htdocs/js/PGCodeMirror/comment.js
Removing htdocs/js/PGCodeMirror/PG.js
Automatic merge failed; fix conflicts and then commit the result.

[Alex-Jordan]

Trying again with just

git checkout -b cm6-pg-editor drgrice1/cm6-pg-editor

and that of course works. I am not sure why my local git says that there is a conflict merging this with develop, but GitHub does not think so.

[Alex-Jordan]

The repository for that is https://github.com/openwebwork/pg-problem-editor.

I don't find anything there.

[Alex-Jordan]

Oh I see it's at https://github.com/openwebwork/pg-codemirror-editor.

[Alex-Jordan]

This is loading and working as far as I can tell. The auto-completion works. The controls for collapsing and expanding blocks work (slightly differently as compared with the hotfix we merged earlier today, as expected). I'm not sure what more to test.

[drgrice1]

Oh I see it's at https://github.com/openwebwork/pg-codemirror-editor.

Sorry for the mistake in my initial comment. I corrected it.

[drgrice1]

For testing, just try opening many problems and checking that syntax highlighting looks right. Test auto completion and such. Note that there is different auto completion depending on where you are. For instance, if you are in the general perl code portion of a problem, versus in a PGML or PG text block. And it is even more fine grained than that. For instance, if you are in a PGML niceTables construct there is different auto completion. If you are inside [# ... #] but not in a [. ... .] then the only auto completion snippet available is for inserting a cell ([. ... .]). But if you are in a cell, then you get the general PGML auto completions.

I think that general testing of this will take time. I have tested with a considerable number of problems of mine as well as problems in the OPL. As more of use start to test and edit problems with this, we will probably see more issues that things need to be tweaked for, as well as potential improvements that can be made.

One thing that I know still needs some work is how indentation works at various points. When you hit Enter (or Return) at the end of the line sometimes indentation is too much or not enough. So the "continued" indent code will still need some tweaking perhaps.

[Alex-Jordan]

I think I must have had a stale version of @drgrice1's branch when I was trying to merge locally before. It's working for me now.

I loaded lots of problems and could see no issues with syntax highlighting.

Auto-completion: I like to use [| ... |]* for verbatim text printed in monospace. Should that appear among auto-completion options? More generally, other starred (and double-starred) variants of things? It could add clutter but also raise awareness of what is possible.

[drgrice1]

I think I must have had a stale version of @drgrice1's branch when I was trying to merge locally before. It's working for me now.

You might have been testing before I updated the branch. There were merge conflicts from @somiaj's pull request #2594.

[drgrice1]

There is an auto completion snippet for [| ... |]. I don't think it is really that beneficial to add a separate snippet for the starred variant because of the way the snippets work.

To see why I don't think it is beneficial, let me first explain auto completion in a little more detail. An auto completion snippet is defined by a template with one or more slots to be filled. When you activate a snippet the cursor starts in the first slot. After typing what you want in that slot you can hit "tab" to switch to the next slot.

Now the snippet for [| ... |] is defined by [|${ }|]${}. Note that ${...} defines a slot wherever it occurs in the definition. This means that if you type [| in a PGML block, then you will be offered the auto completion snippet (you can also not type anything and instead type Ctrl-Spacebar and see a long list of available auto completions). Then if you select the snippet it will inject [| |], and the cursor will be placed after the beginning [|. As soon as you type anything the initial space inside will be replaced with whatever you type. After you have typed what you want inside, then hit "tab", and the cursor will move to after the ending |] since that is where the second slot is in the template. Then you can type the * to get the starred variant.

I designed all of the snippets in the same way with a slot at the end, so you can always tab to the end after typing inside.

If that isn't good enough for you on this, then we can add starred variants for those things that support stars, but I think it could be a bit tedious to have all of those in the general auto completion snippet list.

Also note that the slots can have initial text in them that is replaced as soon as you type in that slot. That is what happens with the space in the first slot above, but I did this in a more meaningful way with the image snippet. It is defined as [!${alt text}!]{${$source}}${}. So when you start that snippet you see [!alt text!]{$source}, and the cursor is placed after the initial [! and before the a in alt text. As soon as you type, the alt text goes away and is replaced with whatever you type. Then when you hit "tab" it moves to after the { and before the $. As soon as you type there, the $source disappears and is replaced with what you type. Then you can hit "tab" again, and now the cursor will be at the end, so you can add more options if needed.

[Alex-Jordan]

I'm good with this at any time. I will continue to make small observations here as long as this PR remains open. These comments do not necessarily belong here as opposed to the other repos associated with code mirror now.

The autocomplete for loadMacros could be more helpful. At a minimum, like loadMacros();. But it might be helpful to go ahead and call PGstandard.pl, PGML.pl, and PGcourse.pl as part of autocompletion. Surely there will be way more times those are wanted than the person will want to delete them. I do understand if, on principles, we don't want to go down the road of assuming the user wants specific arguments.

And I'm not sure how others feel, but lately I've been doing like

loadMacros(qw(
    PGstandard.pl
    PGML.pl
    PGcourse.pl
));

Typing the quotes when you do it the "normal" way got too irritating for me.

More generally, the various PG commands could auto-complete with positions for arguments. For example, random just completes to random, but it could be random(,) or random(start, stop) with "start" and "stop" getting that special treatment where they will be written over once you type.

I noticed some commands that I frequently use from PGauxiliaryFunctions are not in the autocomplete library: random_subset, random_coprime, and more.

Some commands (like DOCUMENT could not only get the () but also a semicolon, since I can't see a reason why you'd not want a semicolon there.

Thanks for adding starred variants. One of them that may be particularly important is the one for layout table. Without that, it would be more likely that people would be using a regular table for layout purposes and go against accessibility principles for that.

[somiaj]

I noticed some commands that I frequently use from PGauxiliaryFunctions are not in the autocomplete library: random_subset, random_coprime, and more.

I think the auto completion is using the same list as syntax highlighting which is incomplete. I've been working on making the list more complete, I forgot about random_coprime, I'll make sure to add that too. I haven't gotten around to making a PR for this yet.

[drgrice1]

Note that I just took the list of PG variables and operators from the old CodeMirror 5 code. We can add whatever we need there. PG has changed quite a bit from when the lists in the CodeMirror 5 code were created.

The auto completion for PG methods is quite different than the auto completion snippets for PGML elements. The PG methods use the completeFromList method which literally just completes words. The PGML elements use snippetCompletion which uses the template that I discussed before. As @somiaj mentioned the set of PG methods doubles for syntax highlighting. I largely did it that way because it was the easy way to go, and in the scope of designing the parsers for syntax highlighting it was much lower priority. Changing the PG methods to snippetCompletion can only be accomplished by not using the syntax highlighting set and instead designing snippet templates for each method. Note that with this approach there would be the additional maintenance of ensuring that the auto completions are in sync with the set for syntax highlighting.

If either of you would like to help, the PG variables and operators are defined in the src/pg-variables.ts file. The general auto completions for the base PG Perl language are defined in the src/pg-parser.ts file.

To fill in other parts of the puzzle, the auto completions (and indentation) for PGML are defined in the src/pgml-language-data.ts file, and the auto completions (and indentation) for PG text blocks are defined in the src/pg-text-language-data.ts file. The PG language as a whole is assembled in src/index.ts which includes the base indentation and code folding definitions. Syntax highlighting is defined for the base PG Perl language in src/pg-highlighting.ts, for PGML near the end of the src/pgml.ts file, and for PG text blocks near the end of src/pg-text.ts. I won't go any deeper, but the remaining files (including the majority of src/pgml.ts and src/pg-text.ts) are the code for the parsers/grammars.

Of course that is all in the codemirror-lang-pg project. Note that the pg-codemirror-editor project is much simpler and just assembles everything into the user interface.

[drgrice1]

I should note that is a bit tedious to bubble up changes to the codemirror-lang-pg project to here. First you have to bump the version of that package, then build and publish it. Then you need to update the dependency on that project in the pg-codemirror-editor project, then bump the pg-codemirror-editor project version, and build and publish that. Then finally, you have to update the dependency here. I tried making those two projects one, but that causes issues with the unit test setup, and is actually more of pain that it was worth. The unit tests are an essential tool for the parsers to guarantee that things work, and continue to work as they should as further development occurs.

Note that the way CodeMirror usually does it, the Lezer parser is usually in one project, and then the language another. If I had gone with that there would be three projects.

[somiaj]

I have already started updating the list of keywords, I just wanted to go though all the pg/macros dir to get them all to start with.

[somiaj]

Here is my start at adding new matching. I got a bit lost in the weeds, there are lots of older out dated macros we probably shouldn't match, but I'm not quite sure which ones are which. I still need to go through the pg/math, pg/misc and pg/ui directoires, though I think I got most the stuff from the other ones.

somiaj/codemirror-lang-pg@b75cab1

[drgrice1]

You are going to need to look in the src/perl-operators.ts file, and remove anything you added in src/pg-variables.ts that is also in that file.

[somiaj]

Thanks, I've updated that.

[drgrice1]

I told you that I took the lists from the current CodeMirror 5 PG.js file, but I did modify the lists some already. One of the things that we don't want to do is to promote usage of deprecated methods and variables. As such I removed the BEGIN_ONE_COLUMN and END_ONE_COLUMN variables that are deprecated, and no longer do anything. I noticed that you added those back. So the next step is to go through the variables and methods you added, and remove any that are deprecated. At a quick glance I don't think I see any others though. So this may just mean remove those.

Also, I noticed that you added Scaffold::Begin and such to the list. Those can't be in the list. The things in these lists need to be pure method names that satisfy Perl's constraints on identifiers. Package names and methods can not be in the list. Note that by package methods here I mean both anything that would be called like PackageName::PackageMethod, and anything that would be called with a package object (like $object->method). Neither of those can be in this list.

I noticed that you added ans_rule to the list, but didn't add ans_array or any of the multitude of other such related methods from PGbasicmacros.pl. There are two ways to go on this. Either add the other related methods, or remove ans_rule. I would vote for removing ans_rule. The thing is that you really shouldn't be using that method anymore. Instead you should be using a MathObject, and calling the ans_rule method of the object (which should not be in this list as mentioned above).

[drgrice1]

I should add that we can add auto completion for Scaffold::Begin and the ilk. They just can't be in the syntax highlighting list.

[drgrice1]

I just noticed that you also added Fun, Label, and Circle for PGgraphmacros.pl. Since those are package names, they also should not be in this list. They may satisfy the constraints on Perl identifier names, but really that isn't the thing. This list just can't contain package names. They don't fit into the grammar in the same way.

[drgrice1]

I am not sure that it is beneficial to add maketext to the list. Although a problem could call it, it won't work to actually perform a translation. The translation string would never be in the po files unless manually added. Generally problem files are not translatable so the method is not useful.

I am not sure that SRAND should be in the list. Although there are valid uses in problems (particularly in Gateway tests), it is a bit dangerous, and must be used correctly. Generally, there is really only one valid usage. That is the case that you call SRAND, do what you want, and then call SRAND with the original problem seed again as in the following code:

SRAND($psvn);
$var = random(0, 2);
SRAND($problemSeed);

[somiaj]

Updated, the last commit on this branch. I have gone though the last few directories. There are lots of macros I'm not familiar with and a lot just seem outdated or specialized so I didn't include what I though clearly fell into that category. I probably included stuff that could be left out as well. I put comments to state which directory I was looking in and pulling macros from.

https://github.com/somiaj/codemirror-lang-pg/tree/update-pg-strings

Please just list any macros you see there that you think shouldn't be, or if you happen to notice anything I missed. I can probably make a PR for this, though want to give time for people to look it over (@drgrice1 You are welcome to just take that commit, modify if you like and apply it directly if you would prefer that over a PR).

[drgrice1]

@somiaj: I merged your branch with the pg operators update. Thanks for working on that.

[drgrice1]

I added auto completions for PG methods like @Alex-Jordan suggested.

By default all of them have the template methodName(${})${}. But that is overridden for the methods below. More of these can be added later.

DOCUMENT and ENDDOCUMENT will be inserted with parentheses and a semicolon.

loadMacros has two variants. One includes the PGstandard.pl, PGML.pl, and PGcourse.pl macros in a qw list. The cursor is at the end of the line after PGML.pl initially so that the author can add more macros if desired. Note that it would not be better to have an empty line between the PGML.pl and PGcourse.pl and place the cursor there as if the author did not want to add more macros then the author must delete the empty line. The other variant is really the default template mentioned above.

list_random has a basic variant of the default list_random(${list})${}.

random has two variants. They are random(${start}, ${end})${} and random(${start}, ${end}, ${increment})${}.

non_zero_random also has two variants that are basically the same as for random other than the method name change.

[Alex-Jordan]

list_random has a basic variant of the default list_random(${list})${}.

Should that be list_random(@{list})${}.?

I've marked this as approved and still so. @somiaj, do you have any hesitation at this point against just merging this?

[drgrice1]

That isn't Perl. That is the format for CodeMirror 6's auto completion snippet template. It should be what it is. That means that if you use the snippet it will show list_random(list), with the cursor after the starting parenthesis, and as soon as you type it replaces the word list.

[Alex-Jordan]

Oh I see. I was thinking the braces alone were the markers for that. But I see that makes no sense given the dollar sigil in other places.

[somiaj]

Well after I merge this, I now notice that italic in PGML doesn't seem to trigger unless there is a bold statement before it.

[drgrice1]

It doesn't seem to be that a bold statement is needed before an italic statement. If an italic statement does not have anything before it on the line, then it works. Somehow, a bold statement before it also makes it work.

I will look into it.