AzureMarker / intellij-lalrpop

Jetbrains plugin for the LALRPOP parser-generator
MIT License
16 stars 2 forks source link

Resolve replace for parentheses #45

Closed ghost closed 3 years ago

ghost commented 3 years ago

Hi,

Syntax for replacing just about any character is correctly highlighted with this plugin, e.g.:

Bool: String = <r"(true|false);"> => <>.replace(";", "").to_string();

But if you try to replace a parentheses or (curly) bracket it does not correctly highlight:

Bool: String = <r"\((true|false)"> => <>.replace("(", "").to_string();
AzureMarker commented 3 years ago

The plugin is trying to figure out where the Rust code ends, and the algorithm isn't 100% accurate (it doesn't understand that parentheses in strings aren't structural). If you replace the snippet with the following, it should highlight fine:

Bool: String = <r"\((true|false)"> => { <>.replace("(", "").to_string() };

The algorithm can be improved, though reaching 100% accuracy is difficult :slightly_smiling_face:.

ghost commented 3 years ago

Thanks @AzureMarker for the quick response. Unfortunately your suggestion also doesn't highlight syntax for me, just installed the plugin last week on IntelliJ:

IntelliJ IDEA 2020.3.2 (Community Edition) Build #IC-203.7148.57, built on January 26, 2021 Runtime version: 11.0.9.1+11-b1145.77 amd64 VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o. Linux 5.4.0-81-generic GC: ParNew, ConcurrentMarkSweep Memory: 1979M Cores: 12 Non-Bundled Plugins: String Manipulation, org.toml.lang, org.rust.lang, com.mdrobnak.intellij-lalrpop, org.intellij.scala, slinky.core.intellij Current Desktop: Undefined

AzureMarker commented 3 years ago

I'll look at this again using 2020.3.4. Are you using the latest compatible versions of all your plugins?

ghost commented 3 years ago

LALRPOP: 0.2.3 Rust 0.4.148.3911-203

AzureMarker commented 3 years ago

Ok, I reproduced it and adding the braces still doesn't fix the parentheses issue. I'll look into improving the algorithm.

FYI, there's also an ongoing issue getting full support from the Rust plugin, which may be impacting you: https://github.com/intellij-rust/intellij-rust/issues/7655

AzureMarker commented 3 years ago

I've fixed the issue and will push a commit to master. I'm planning on making a release soon, but the release will only be for 2021 IDE versions. You'll have to update, but that should be easy since you're on the Community edition (free). image

AzureMarker commented 3 years ago

I've submitted v0.2.4 to the plugin marketplace, and it should be available in a few days (once it's approved).

ghost commented 3 years ago

@AzureMarker can we please reopen? Replacing a single character works, but as soon as there's two or more it is again the same problem:

e.g. to avoid ambiguity, the simplest way forward is to include "pin (" in the terminal, but then I must replace it (a very common occurrence - so common that I could never use this plugin)

pub Pin: Box<Pin> =
  <n:r"pin \([a-zA-Z0-9_]+"> <range:PinRange?> ") {"
    <attrs:Attr*>
  "}" => Box::new(Pin{name: n.replace("pin (", "").to_string(), range, attrs});

I'm sorry if this looks like perfectionism, but replace is such a critical function in lalrpop grammar since it's often times necessary to define highly distinct terminals that intentionally contain characters you don't want just to avoid grammar ambiguities, then requiring you to perform a replace in the action code.

I'm working with a terribly inconsistent text file format that relies on distinct prefixes in my terminals I must keep, else everything is just a \w+ terminal and I cannot parse intricate details very well. So I have a lot of replace being used.

ghost commented 3 years ago

and this is occurring with the latest intellij 2021.2.1

IntelliJ IDEA 2021.2.1 (Community Edition) Build #IC-212.5080.55, built on August 24, 2021 Runtime version: 11.0.11+9-b1504.16 amd64 VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o. Linux 5.4.0-81-generic GC: ParNew, ConcurrentMarkSweep Memory: 1979M Cores: 12 Registry: scala.erase.compiler.process.jdk.once=false Non-Bundled Plugins: org.toml.lang (0.2.153.4056-212), String Manipulation (8.15.203.000.3), org.rust.lang (0.4.153.4056-212), com.mdrobnak.intellij-lalrpop (0.2.4), org.intellij.scala (2021.2.17), slinky.core.intellij (0.6.7) Kotlin: 212-1.5.30-release-409-IJ4638.7 Current Desktop: Undefined

AzureMarker commented 3 years ago

Hi, I'll reopen and look into this. The fix I added shouldn't be specific to any one method; it just ignores parentheses (and brackets and braces) in strings. I'll see why it's not working in this case.

AzureMarker commented 3 years ago

I found the issue and will push a release: image

AzureMarker commented 3 years ago

I've submitted 0.2.5 to the plugin marketplace.