`color` in imports shows up as an error in the editor in 4.0 beta 9

[processing-bot]

Created by: benfry

Code still compiles and runs, but is tagged as an error while editing.

To reproduce, see what happens when importing the toxiclibs library.

Possibly related: #519

[processing-bot]

Created by: benfry

[processing-bot]

Created by: bsapozhnikov

@benfry I'm new here so apologies for my ignorance - what's the best way for me to contribute to issues like this one?
I've cloned the repo and played around with this issue; it looks to me like it can be addressed by updating exitColorPrimitiveType in the tree listener to not do the "color" -> "int" replacement when inside an import declaration. I'd be happy to open a PR if that direction sounds reasonable?

[processing-bot]

Created by: benfry

Thanks for looking into it! Paging @sampottinger here since he's been the one working on that part of the code most actively, and can better answer your question.

[processing-bot]

Created by: sampottinger

Hey all! Strange it's an editor error only. Just referencing #240.

[processing-bot]

Created by: sampottinger

See also #246

[processing-bot]

Created by: bsapozhnikov

Yea I also thought that was strange. I did see the issue and fix you linked above and also saw locally that those changes to the .g4 file are important to the actual compilation working properly. But to fix the editor error locally, I needed to make the change described above with existColorPrimitiveType.
I'm very new to the code so this is just a shot in the dark, but it does look like the build and error-checker flows use different logic to preprocess and compile the code (e.g. the error-checker uses the PreprocService while JavaBuild uses the PdePreprocessor directly) so it's possible that they handle edits and/or imports slightly differently? I haven't traced out the code enough to get a better answer though.
Conceptually though it seems like we don't want to do the color -> int conversion inside import declarations, right? Since inside of an import we're not treating color as a primitive the way we do elsewhere.

[processing-bot]

Created by: sampottinger

Hey @bsapozhnikov! Yes that's right though they should hit the same preprocessor eventually which is what is odd. The grammar definition itself should match the import declaration (we technically have a problem if it gets parsed as a color primitive). So, in theory, we shouldn't hit the exit method for the color primitive at all for imports in the current code. I'd like to take look to see what the parser is seeing there. It could be related to #519. Should check that there aren't any other issues.

[processing-bot]

Created by: sampottinger

Thanks for your help @bsapozhnikov.

Cool so the code from the editor is getting in and out of the preprocessor correctly in that the ANTLR output program is always right even when run from the preprocessor server. However, I think we have an issue when we go to generate the EDT edits 😑. Even though it’s the same grammar…

I agree @bsapozhnikov about the exitColorPrimitiveType but I think this could trip up any use of a qualified name for example lib.sublib.color.method().

I’ll have a PR up in a moment. Thank you for your help!

[processing-bot]

Created by: benfry

Great! Incorporated for 4.1.2 or 4.2, whichever comes first.

[processing-bot]

Created by: bsapozhnikov

Cool! Also a general question about contributing - if I dig into any other issues, should I be opening PRs or just continuing to post in the discussion?

[processing-bot]

Created by: benfry

Probably worth checking in w/ the discussion in case it's something that's in progress, or no longer relevant, or just to make sure the problem is clear. This is where most PRs get tripped up, and hopefully we can prevent any wasted time.

Thanks for asking!

[processing-bot]

Created by: sampottinger

Thanks for participating in the community @bsapozhnikov!

[processing-bot]

Created by: github-actions[bot]

This issue has been automatically locked. To avoid confusion with reports that have already been resolved, closed issues are automatically locked 30 days after the last comment. Please open a new issue for related bugs.