update ace syntax to fix lua 5.3 bitwise op parse errors

[tehn]

seems to be using an earlier version of lua. for example, sees bit shifting as an error.

[jlmitch5]

I think we are up-to-date in terms of what Ace has for lua syntax.

Looks to me like this would need to be fixed at the ace level (it doesn't seem like there's a scanPunctuator(>>) assuming that's what you mean by bitshifting (Nor is there <<)

https://github.com/ajaxorg/ace/blob/master/lib/ace/mode/lua/luaparse.js#L545

[ngwese]

well that is a bummer. i wonder how much effort it would be to roll our own build (with some enhancements) and then submit the changes back to the Ace repo...

[jlmitch5]

It looks like the react-ace component uses brace which embeds a version of ace (there are a bunch of commits with messages like "update to using ace 1.2.9". So I went to look there and that's how I happened upon the line above in the ace lib's lua mode files. brace seems to have its own (different?) parsing stuff...which also doesn't mention the shifting ops >> and <<

https://github.com/thlorenz/brace/blob/master/mode/lua.js#L139

--

Both what we'd change and how we'd roll our own is a little unclear on this one, and I feel like this one might be a bit of a rabbit-hole. Also could be totally possible I'm making this harder than it actually is.

I think some further investigation could be good, but unfortunately, I don't think this one is going to be as easy as bump the package.json version :/

[pq]

doing a bit more digging, i think the bit we want to see update is luaparse.js; specifically, we'd want to add some new case logic around here:

https://github.com/ajaxorg/ace/blob/master/lib/ace/mode/lua/luaparse.js#L544-L546

(EDIT: @jlmitch5 already noted this... consider it doubly confirmed! 😀)

[pq]

i think the fix is actually easy; the tough part is integrating it. (i'm looking at #78 which is harder than you'd think 😬; in this case, we'd have to clobber the custom worker that does the annotating and replace it with our own that uses a modified parser. investigating.)

[Dewb]

Another 5.3 feature that Ace doesn't handle correctly is the // floor-division operator. It reports a red-x error with <expression> expected near '/'.

(Mainly mentioning this here so if someone else searches for this error, this issue is more likely to come up.)

[ngwese]

At this point I’d be happy to just maintain a local fork of the lua syntax if someone could figure out how to get ace to load it instead of the built in.

[tlubke]

I'm wondering if this would be a solution, found from the Ace website:

Configure dynamic loading of modes and themes

By default ace detcts the url for dynamic loading by finding the script node for ace.js. This doesn't work if ace.js is not loaded with a separate script tag, and in this case it is required to set url explicitely

ace.config.set("basePath", "https://url.to.a/folder/that/contains-ace-modes");

Maybe we could create a fork on github like suggested above, and just point the config there?

[pq]

hey @tlubke. i would love if this turned out to be possible. TBH i can't recall if i tried but it sure feels promising.

as for the syntax fixes proper, i'm happy to help shepherd them in (and really we should just upstream them while we're at it).

cheers!

[pq]

(oh wait, i'm remembering now: maybe this is not as straight-forward because ace is bundled in a react wrapper?)

[ngwese]

...oldest ticket on the list, back from the dead. on a whim i looked to see if the react-ace package has been updated. currently maiden uses v5.1.0 and the latest version is (cough) v9.2.1. after some fairly trivial edits it looks like the newer version might be more or less a drop in. on an encouraging note testing so far has shown that the new version:

• fixes parsing of << and >> operators so they are no longer flagged as bad
• correctly de-indents end keywords when completing a block

i'm going to push forward and see if i can get the maiden's dark mode working as well.