[rubberduck-vba/Rubberduck] ScottDennison commented on development/java/Rubberduck.Parsing/Grammar/build.gradle of pull request #4815: Reduce target language dependence of grammar

@Duga ugh

[rubberduck-vba/Rubberduck] ScottDennison commented on development/java/Rubberduck.Parsing/Grammar/build.gradle of pull request #4815: Reduce target language dependence of grammar

[rubberduck-vba/Rubberduck] Vogel612 commented on development/java/Rubberduck.Parsing/Grammar/build.gradle of pull request #4815: Reduce target language dependence of grammar

[rubberduck-vba/Rubberduck] ScottDennison synchronized pull request #4815: Reduce target language dependence of grammar

[rubberduck-vba/Rubberduck] build for commit 24b67447 on unknown branch: AppVeyor build succeeded

Hmm. Official ANTLR version now has a nuget package: github.com/antlr/antlr4/pull/2216

OTOH, I understood that Sam's version diverges somewhat from the official version.... He had some performance enhancements that aren't part of the official that might be working in our favor.

esp. for long Boolean expressions with many operators - with 4.3 that was killing parse performance big time

was that specifically Sam's enhancement?

IDK

but I'm 99.99999999% sure that everything will crumble if we try to use the "official" Antlr

@MathieuGuindon I tried. It wasn't particularly easy, and you have to have Java installed to generate the files, and all.

I got past the first issues, but kind of got lost after that.

Sam's version is much easier to use.

@Hosch250 given that the official now has a nuget, it might be now a moot point.

The other issue with the divergences in syntax/features between Sam's and the official, however, is more pressing.

It had the NuGet then too.

oh.... then I don't follow.

Why a nuget if you gonna build in java?

that just feels wrong.

I think when we last checked only Sam's version supported c# build tasks to generate the files within the build of the solution.

I think the NuGet is the runtime, not the generator.

here's the thing - if we're now building java for the grammar, why not just do it w/ the official version?

because the generated API is massively different and none of us need that headache?

I do not know how well that integrates into the build of the solution.

^^

I definitely don't want to build java locally.

Looks like I opened a can of worms with that suggestion. I only envisioned adding the class files.

I like my Java in a mug next to the compuer, not in the computer.

> A new Java version is available!

runs installer after dismissing it for 2 months ... reboots ... "A new Java version is available!"

^

I only meant that it'd be manually built for the occasional debugging of the grammar

yeah, that's totally fine

ironic. I got an email just now: > Java 12 to be released next month

It's your fault, Mat.

pastebin.com/uaGci43J

My Magic the Gathering parser is slowly coming along ^^

[rubberduck-vba/Rubberduck] build for commit 1f074c34 on unknown branch: AppVeyor build succeeded

@Duga This seems to have worked.

so ...we're good with building the Java target with Antlr 4.7?

we're technically still using 4.6.4 in our C#

I guess the java is building w/ 4.7?

IDK.

I'm fine.

hey @ScottDennison!

hi @ScottDennison you've been our man of the hour!

thanks again for the awesome work!

^

Hope I haven't opened a can of worms with the Java stuff.

nah. I'm the official worm can opener here.

lol

I was the one who suggested it, so no worries. I do see why we would want to build the java targets.

However, as you see, the devil's in the details - we're using 4.6.4 of Sam's fork, so I'm not sure how that works out with the official version of 4.7.2.

theverge.com/circuitbreaker/2019/2/21/18234615/…

Whoops, someone put a bug in their smart shoes ^^

@this for analyzing & debugging the grammar, it's probably fine

pretty sure if you ever want 5.2K broken unit tests and 75K lines of code to fix, swapping 4.6.4 (Sam's) for 4.7.2 (official) is a good way :)

I think so, too. I'm more worried about the divergences between official and Sam's (of which I have no clue about) causing build failures.

but that might be moot given that the most recent build succeeded.

I don't remember, honestly. might have been something as crazy as "oh, it doesn't have the tokenstreamrewriter stuff"

or some members we're using that don't exist in IParseTree or something

so, we're good to merge #4815 I think now

who wants to press the green button?

My thoughts on the difference between the antlr version for the java gradle project and the main c# project:

Damn. Pressed enter early

eh, SE chat takes some getting used-to :)

public abstract class VBABaseLexer extends Lexer {
    public VBABaseLexer() {
        super();
    }

so just like that, we have a rock-solid argument (other than, well, consistency) to enforce brace style across the C# code :)

[rubberduck-vba/Rubberduck] retailcoder merged pull request #4815: Reduce target language dependence of grammar from ScottDennison/Rubberduck/next into rubberduck-vba/Rubberduck/next

[rubberduck-vba/Rubberduck] Unrecognized author pushed commit 13087a48 to next: Modify grammar and introduce base class to reduce target language dependence of grammar.

[rubberduck-vba/Rubberduck] Unrecognized author pushed commit 67125e99 to next: [PR-4815] Review feedback. Also commited java files, making it so that checking the java builds OK is part of the build.

[rubberduck-vba/Rubberduck] Unrecognized author pushed commit 6accbd36 to next: [PR-4815] Correct rouge tabs into spaces, and a missing newline from the end of .gitignore

[rubberduck-vba/Rubberduck] Unrecognized author pushed commit 81630d77 to next: [PR-4815] Attempt to cache gradle wrapper binaries.

[rubberduck-vba/Rubberduck] Unrecognized author pushed commit 24b67447 to next: [PR-4815] Correct build.gradle typo, and hopefully fix failing appveyor build.

[rubberduck-vba/Rubberduck] Unrecognized author pushed commit 1f074c34 to next: [PR-4815] Make antlr version consistent (unfortunately not on version 4.6 due to errors.). Also use appveyor version numbering.

[rubberduck-vba/Rubberduck] web-flow pushed commit 56662a55 to next

@Duga unrecognized author??

lol, Java doesn't even show up :/

@ScottDennison can't wait to hear them :)

*read

Currently, the java/gradle project is not used within rubberduck (and is unlikely to be used in a permanent manner in the future). So unless a breaking change between antlr java and antlr-cs-sam regarding the grammer itself, there is unlikely to be a problem.

I decided to make it part of the build process so that if that time ever comes where they do diverge, it won't sit there unknown, and instead someone has to do something about it, even if it is making a concious decision to leave well alone. And because of this, a slight difference in version numbers does not matter. If the full project was being side written in Java, it would be a ahole different story

That took a long time to write on an android keyboard.

oh lol

that is good reasoning.

Wonder why I am classified as unrecognized author by Duga

Also what did you mean @MathieuGuindon regarding the brace style? Did I muck up the braces, or was it just java conventions having the brace on the same line and c# conventions the opposite? (or at least the conventions I am aware of)

Duga is just our chatbot, I'm thinking it's GitHub

@ScottDennison there was a guy that once wrote a certain feature in C#-that-looks-like-Java

you didn't muck up anything :)

looks like the commits are correctly attributed, all is good

Hi guys

I compiled, but the "OK" button does not work

image

No change applies

yeah there was a bug reported about that. I broke the xml persistence. :(

you can hand-edit your XML file to use spanish

@this Where do I find it?

%APPDATA%\Rubberduck\config.xml

[rubberduck-vba/Rubberduck] rubberduck-vba-releasebot published prerelease Prerelease-v2.4.0.4563

[rubberduck-vba/Rubberduck] rubberduck-vba-releasebot created tag Prerelease-v2.4.0.4563

[rubberduck-vba/Rubberduck] build for commit 56662a55 on next: AppVeyor build succeeded

@this Is not found

Can it be configured by default when compiling?

I forogt that it's not created until you change the default

IOW settings are fully borked rn?

you can create it by hand:

github.com/rubberduck-vba/Rubberduck/wiki/…

@MathieuGuindon IDK, we just don't create it at install/build

have to actually edit the setting to update the file

I know - but then if the settings won't save...

I thought it was just the import/export that broke

no, it was saving, too

it was in the issue this AM, too

[rubberduck-vba/Rubberduck] retailcoder added label critical to issue #4816: Won't apply settings to newly formatted computer

sue me harder?

lol

And if we first solve the problem of the "Ok" button.

<?xml version="1.0" encoding="utf-8"?>
<Configuration>
  <GeneralSettings>
    <Language Code="es-ES" />
    <CanShowSplash>true</CanShowSplash>
    <CanCheckVersion>true</CanCheckVersion>
    <CompileBeforeParse>true</CompileBeforeParse>
    <IsSmartIndenterPrompted>false</IsSmartIndenterPrompted>
    <IsAutoSaveEnabled>false</IsAutoSaveEnabled>
    <AutoSavePeriod>10</AutoSavePeriod>
    <UserEditedLogLevel>false</UserEditedLogLevel>
    <MinimumLogLevel>6</MinimumLogLevel>
    <EnableExperimentalFeatures />

^ that should do it

^ save as %APPDATA%\Rubberduck.config.xml

Actually, based on the log file, <Configuration /> might do it.

I think that should get loaded. hopefully.

<Language Code="es-ES" />

if it doesn't, then sue me with extreme prejudice.

^ but you want that bit in there to test the translated UI

No, extreme prejudice means we can't sue you again if we win.

hmm good point.

so, sue me with slightly-less-than-extreme prejudice, then?

I think it's just getting a null path if the file doesn't exist.

It did not work

@ScottDennison Since you intend to use our grammar within one of your own projects, you might want to have a look at what we do in the preprocessing step to hide precompiler directives and dead code from the parser, too.

Settings hot-fix inbound.

[rubberduck-vba/Rubberduck] comintern created pull request #4817: Settings hot-fix. to merge comintern/Rubberduck/next into rubberduck-vba/Rubberduck/next

> Ensure created config file has root node. Closes #4816

[rubberduck-vba/Rubberduck] comintern review_requested pull request #4817: Settings hot-fix.

let's see swhat I did wrong....

No just you. I reviewed it.

the darned thing is that I thought I did test it but ?

It only manifests itself if the config file doesn't exist.

The root node just wasn't getting added on the early return.

hmmm. also, the original version was kind of stupid since we had 2 different lines returning a new XDocument

I think I followed R#'s suggestions a bit too blindly there

Yeah, I think it probably missed the .Add

yep

this looks much better anyway. Thanks for covering!

NP - I was kind of stumped when I saw it because I was assuming the NRE was on the file name.

but what about this....

shouldn't that have had worked?

One sec, let me try the cut-down file.

Well, the settings UI fails to load if it can't find the language.

That's unrelated though (and has apparently always been broken).

kind of tempted to write integration tests

can't have settings going bust (and that has happened more than few times) :(

Might not be a bad idea.

just dummy up a XML file, load it w/ a simplified implementation, make sure service is behaving sanely

When I originally wrote the non-AV compatible ones, I was doing round trip testing.

I.e., serialize settings, then deserialize and assert that they matched.

[rubberduck-vba/Rubberduck] build for commit abbf1531 on unknown branch: AppVeyor build succeeded

#4814 has me completely stumped though. The only way I can see that happening is if the parse tree either isn't complete when the listener runs or it's stale.

Oh crap, I know what happened there.