F0rce / ace

Ace Editor for Vaadin 14 & 23
MIT License
27 stars 13 forks source link

Regex in ext-language_tools.js wrong #32

Closed Rob-Rocket closed 2 years ago

Rob-Rocket commented 2 years ago

The changed regex (from the original ace-repository) cause problems witin our application - point (".") added var ID_REGEX = /[a-zA-Z_0-9\$-\u00A2-\u2000\u2070-\uFFFF.]/;

1) Why is that point "." added within that regex on line 1367?

2) Should'nt the "." in the regex be escaped?

F0rce commented 2 years ago

Hello @Rob-Rocket,

the point was added, that when using the dynamicAutocompletion you could use a "." as seperator, for example when re-creating a class like autocompletion:

Class A with 2 methods:
a.method1
a.method2
...

What problems is it causing and which version of Ace are you using, as all of my test's went through completely fine (and the language_tools.js change was released with 2.0.0 as far as I remember).

F0rce commented 2 years ago

Refernce where people used that code aswell (official Ace Repo)

https://github.com/ajaxorg/ace/issues/1670

askpythia commented 2 years ago

Hello F0rce, I've the same problem as Rob-Rocket, the additional point in the ID_REGEX causes problems with all higher level languages using ace editor. We have implemented an IDE using Ace with an Xtext based JVM integrated language behind. With this change the code completion broke because deriving the correct context is more complex than just looking for point separated junks. Would it be possible to make the ID_REGEX defineable? Regards, Pythia

F0rce commented 2 years ago

Hello @askpythia,

I will tomorrow (as it is super late in Germany) change the regex to escape the .. If it does not break current logic I will build a hotfix asap.

I'll keep you updated.

Good Night, David

askpythia commented 2 years ago

... or is it possible to use lit-ace together with the latest official ajaxorg/ace-builds together with your Vaadin Component? It seems that the ajaxorg builds have the correct ID_REGEX in ext-language_tools.js ...

askpythia commented 2 years ago

Just saw your answer, thank you (from Vienna, also late here :-)

askpythia commented 2 years ago

be careful, not escaping but removing. The ID_REGEX should match Identifiers (which don't contain "." in their names) not calls

askpythia commented 2 years ago

Any news?

F0rce commented 2 years ago

Sorry @askpythia,

I had my finals exams, and was not being able to test everything out. As a new version of ace was released I will try use some time migrating to the new version and testing out the regex.

Will keep you updated. Really hope it won't take that long...

F0rce commented 2 years ago

Hello @askpythia,

as I was investigating this, I came to the conclusion that the "." in the regex makes no difference.

https://regexr.com/6m3f7

As you can see here, it matches the "." character and not every character as you thought. So to keep the current logic and as it is not breaking anything, I will mark this ticket as closed and resolved.

Sorry for the long awaited answer.

askpythia commented 2 years ago

Sorry for the late answer, but it is the point itself which causes the scope to consume all expressins on the left side including points. In the code lookup itself high level languages navigate from expression to expression (called via ".") by themself. The point at the end causes that a callstack of expressions is handled like one big and that's definitely wrong. We patched the regexp in node_modules after install (just remove the point) to get it work, but this is not the right way... If you take the last Version of Ace, please don't add the point again ...

F0rce commented 2 years ago

Hei @askpythia,

could you please provide some more information on that (especially on your use-cases) ? I am likely to keep the point in the regex, as in many cases I was able to create a class based autocompletion model (using the dynamic autocompletion I created for ace). And if the seperator is something else then a "." it just seems odd for an IDE.