VBA Rubberducking

Duga

00:00

RELOAD!

[rubberduck-vba/Rubberduck] 2 commits. 1 closed issue. 3 issue comments. 22 additions. 238 deletions.

[Zomis/Server2] 1 commit. 14 additions. 13 deletions.

IvenBach

00:16

Rebooting is the GoTo place to start for all things tech.

Mathieu Guindon

Agreed. I never use Option Base 1 myself and honestly don't see much use for it beyond obfuscating one code sheet among others in a project that will still default to Option Base 0. However, I know others that do use it and prefer it so there it is for those that prefer to complicate things unnecessarily. — user11087823 2 mins ago

lol!

IvenBach

It's taken a long time but I have started disliking several quirks of VBA. Too easy to write code that works when you don't know why.

Comintern

That's not unique to VBA.

IvenBach

00:33

Certainly not unique bet a lot easier than other languages.

this

I see you haven't written JS yet.

Comintern

Sometimes the JS doesn't even know why it works.

IvenBach

Range("A1") = "foo" has lots of ~~Help~~ hindering that it's doing without new coders realizing. I was one of them, until RD enlightened me.

I have null experience with JS.

Comintern

As opposed to PHP, which doesn't work but thinks it does.

this

PHP + JS + CSS = 3 stooges

IvenBach

00:45

What's CSS done wrong?

Had to work with it briefly a long time ago. I'm inclined to think differently now with my additional tech knowledge though.

Comintern

A Google image search for "css done wrong" wasn't nearly as amusing as I thought it would be.

this

of the 3, CSS is the most benign. To be fair, it's not a programming language anyway

IvenBach

As I remember CSS was a BIG step up from in-lining everything.

this

Well yes, but how it does it is.... interesting.

IvenBach

What'cha mean?

this

00:50

For an example of that, google on what it takes to vertically center text using CSS

there's several ways, all which don't quite work.

Comintern

blog.codinghorror.com/whats-wrong-with-css

Mathieu Guindon

@IvenBach depending on how JS interprets it, that might already be more than you think 😂

Comintern

lol

Mathieu Guindon

argh, OleTools beat us to 750

Comintern

I should stop writing malware.

Mathieu Guindon

01:04

@jphenow @Amy_Hupe @NickColley JSON is pronounced J-A-M-E-S

Tweeted by terrajobst on February 20, 2019 at 7:55 PM in reply to jphenow

IvenBach

@MathieuGuindon hence my comment.

There was a meme about JS and null where <= and >= treated it at true. Will be driving and can’t search for it.

@MathieuGuindon oh jeez...

this

@MathieuGuindon I wonder if the people who stars are starring VBA repos as they go by the list?

Mathieu Guindon

Good question. Surely a bunch of people must be searching for "vba" on GitHub

this

TBH, I didn't think that malware analysis, particularly for office documents would be that a big deal.

I would hope(!) that by now, majority of people know better than to download any attachments from weird emails.

Comintern

IDK, we had an incident last week at work.

Mathieu Guindon

01:15

@this you can hope...

Dair

@this You would also be wrong.

Comintern

You'd think the majority of people who work at an f'n software company would know better, but that's apparently not the case.

this

:( you guys destroy my (little of what's left) faith in humanity.

Mathieu Guindon

18 mins ago, by Comintern

I should stop writing malware.

Hmm

Comintern

Shhhh....

TBH, I'm the victim of code that I write in the vast majority of cases.

Dair

01:17

Just because I studied computer science doesn't mean that I know not to open attachments from suspicious emails.

Duga

[rubberduck-vba/Rubberduck] bclothier synchronized pull request #4784: Misc bug fixes

Comintern

Question.

Why IQuickFixProvider instead of having the IInspectionResult expose its own quickfixes?

It looks like there is a good reason for it, but in practice it is coupling the results with the VM.

I'm not sure there's a great way to unwind that though...

It's acting like a service provider, but the classes that it provides services for don't access it directly.

Mathieu Guindon

@Comintern good question. the inspection mechanics have been revisited a time or two, but changing/improving it just feels.. daunting, I guess -- and the number of types affected isn't shrinking either!

Comintern

Daunting is a bit of an understatement.

Mathieu Guindon

^

Comintern

01:29

The problem I keep running into with the stupid context menus for the IR is that it's incredibly difficult to simply bind them to grid rows.

this

FWIW: chat.stackexchange.com/transcript/14929?m=36614344#36614344

@Comintern If it's incredibly difficult, then it should be easy, per the rules of WPF, no?

Dair

@this This is what Dijkstra was warning us about.

Comintern

@this lol

this

it's hard because the grid contents are dynamic?

Comintern

All the angles I have completely step outside of MVVM.

It's hard because the inspections don't know how to fix themselves.

this

01:34

Could we sic CW on that?

Comintern

Maybe?

The problem is that the results aren't injected, so there's no clear way to just give them all a provider instance.

this

no, we don't need to inject those. CW has to trawl through the assembly for registration, right?

so when it finds the types, it builds up a map of what QFs are allowed for the given IR

Duga

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

this

then we just have the CW provide that map as a class or something.

@Duga @MathieuGuindon here ya go

Then now you have your nice little binding.... right?

Comintern

How does that work if all the instances come via 'new'?

this

01:37

we're talking about types, not instances, right?

a QF says I can fix type A, B, C.

we only need to know that we have a instance that's a type B, then we know that QF applies.

Comintern

No, the provider should still be responsible for that.

The inspection results themselves want a reference to it though.

this

Right but it's going ot end up reflecting over all the IRs/QFs

If that's what we end up, might as well just use CW since it's already doing the work of reflecting

Mathieu Guindon

Sounds like a good case for a GH "project", no? "Inspections Overhaul", with its own branches (Inspections-Next, Inspections-Dev), and we could collaboratively, progressively, ...*eat the elephant*, and merge into next once Inspections-Next is in a mergeable state?

Comintern

@this This is the root issue:

    return Listener.Contexts
        .Where(result => !IsIgnoringInspectionResultFor(result.ModuleName, result.Context.Start.Line))
        .Select(result => new QualifiedContextInspectionResult(this,
                                                InspectionResults.EmptyElseBlockInspection,
                                                result));

Every one of those really wants the provider.

I suppose it could be injected into the inspection base, but that's a crapload of manual argument passing to hack in.

this

I'm confused. Why does QF need to be there?

Comintern

01:41

(and probably a couple K of borked test setup).

So the InspectionResult has access to it's quick fixes without being coupled to anything else.

this

but, InspectionResult should be just data

Mathieu Guindon

^

this

the moment we couple it to any QF, it's no longer just data.

Comintern

It doesn't want to be though.

What happens when we want to put them in an Avalon margin?

Does the code pane need the QF provider now too?

this

I'm not understanding why that matters - that's a provider's responsiblity.

Comintern

01:43

I guess I'm coming from the standpoint of "a quick-fix is not a UI concern".

this

we agree.

we haven't started to talk about the UI yet.

the provider can just map QFs to IR

then the UI stuff takes that provider

Comintern

Except structurally, the provider is owned by elements of the UI.

@this Right - that's what I'm saying. That couples the QFs to the UI.

An intermediary solution would be to implement an InspectionResultModel, but that would basically consist of something else tacking the IQuickFixProvider onto an IInspectionResult.

this

hmm, maybe it's backward.

we don't want a QF provider

we want a IR provider

that would then provide the QFs relevant for that.

I'm not sure if provider (of either forms) have to be coupled to the UI. I still can see them existing just fine on their own.

Comintern

We have one of those too.

this

evidently we don't have enough providers then.

Comintern

01:58

lol

this

PROVIDE ALL THE THINGS!

Comintern

IProviderProvider

this

If that goes into RD, then we can safely call RD "enterprise-grade"

Back to the problem, so what is it that's coupling the provider to the UI?

Comintern

That might be the least painful solution though - have the IInspector perform the compositing.

Hosch250

LOL, looking through the old chat and found this:

Apr 10 '17 at 22:33, by Hosch250

Now that the northerner is gone, I can tell a bunch of fish stories about how we deal with snow.

I read my family the messages immediately after that, and my brother said they didn't realize I had those trolling skills.

this

02:00

@Comintern That actually makes sense

Comintern

Speaking of which, after this morning I'm about out of places to pile it.

Hosch250

@this Thanks. This got me going on some gems of old transcripts.

this

wait, just to make sure is IInspector UI-bound?

@Hosch250 you're welcome!

Still wondering what fish has to do with snow, though.

Hosch250

@Comintern I can tell you that.

Comintern

@this No. It's only getting used by the UI currently.

Hosch250

02:03

@this merriam-webster.com/dictionary/fish%20story

this

I know - I was being funnily literal.

Hosch250

So, if I have your attention, @Comintern, here's the reason for that.

See, we used to have IInspectionResult expose the quick fixes.

Comintern

I'm sure there was.

this

@Comintern then it shouldn't be too bad, I think. Until Hosch came along. :D

Hosch250

But the inspection new'ed up the result, right? So that meant the inspection had to new the quick fixes up.

Comintern

02:04

@this There are some instances of fish falling from the sky in large numbers, usually related to huge waterspout events.

Hosch250

So, the inspections knew about all the quick fixes as well as the inspection.

So, I inverted the relationship and make a quick fix provider (IIRC, I don't remember all the details).

It was getting really clumsy in both the inspections and the tests and stuff, because the quick fix is not a responsibility of an inspection.

The inspection inspects the code, and only inspects it. The quick fix only fixes it.

Comintern

OK, that makes sense. Same deal as attaching the provider really.

Hosch250

And, this way, we can instantly translate the quick fix inspections in the UI.

Comintern

That's the part I'm fighting with now.

Hosch250

Before, the Fix dropdown text was inserted into the inspection result.

Now, it queries the provider for inspections given an inspection result.

The inspection result says which inspection type it is for, and the quick fixes declare whether they can fix that type.

Also, IIRC, the quick fixes can use DI now, or maybe not.

They weren't before.

And, before, we used to create a new instance of the quick fix for every result.

Comintern

02:10

OK, I'm starting to like the idea of an InspectionModel more and more, with the IInspector responsible for associating the QFs with the IRs.

Hosch250

Now, we have a singleton instance of the fix, IIRC (since it's a pure class--no side effects within itself), which saves a reasonable amount of memory too.

this

@Comintern me too.

but I'm not sure if the name is the best

because Model implies it's just data

Comintern

@Hosch250 That's a good point - the QFs should be lazy.

Hosch250

@Comintern Since I just popped in, could you explain that again?

Or point me to the original conversation?

this

@Hosch250 that was what we were talking about just right before you popped in.

Comintern

02:11

OK, so currently all of the inspection results pass through the IInspector - it's responsible for performing all of them.

this

11 mins ago, by Comintern

That might be the least painful solution though - have the IInspector perform the compositing.

Hosch250

Correct. The inspections run and create the inspection results themselves.

Some are declaration results, some QMN results, and some parse tree node results.

Comintern

Instead of having the VM provide the QFs, the IInspector could composit them.

Hosch250

So, the inspection creates the inspection result, the inspector inserts a reference to the eligible quick fixes into them?

(Sorry if I'm misunderstanding.)

Comintern

No, the IInspector would provide a new class that encapsulates both the result and the QF provider.

Hosch250

02:14

Oh.

Comintern

IItsBrokeAndHowToFixIt

Hosch250

And we still have a singleton provider.

Comintern

Right.

Hosch250

And singleton fixes in the provider.

Sounds good.

Comintern

And it's still lazy.

Hosch250

02:15

Yep.

Comintern

And it can also take some of the specific behaviors back from the UI. I.e., performing the fix.

Hosch250

Correct.

Comintern

OK. Doin' that then.

Hosch250

Sounds good.

Comintern

Even if it breaks all 5000 tests, it's going to be easier and faster than fighting with the stupid WPF context menus...

Hosch250

02:19

LOL.

Most of those tests, you'll be able to use regex search/replace to fix a lot of it, probably.

Or, perhaps write a console app to update it (yes, I've done that before).

Comintern

I've done that before too. Not on RD.

Hosch250

@Comintern I'm pretty sure it does.

I've gotten NRE's before by binding the selected item before the collection source.

Comintern

I've suspected that. I mean it needs to pick the binding order somehow, right.

Hosch250

That one took me a day to figure out.

Until I tried creating an MVCE, and accidentally did it the other way.

That worked, then I checked it line by line and found the difference, swapped it, and it failed.

Dishes time. TTYL.

Duga

03:57

[rubberduck-vba/Rubberduck] bclothier synchronized pull request #4784: Misc bug fixes

this

@Duga the pool for betting on when anyone notices that ducky knows a thing or two about PPT's weird-ass VBA behavior is now open. My bet: Never.

2

Duga

04:17

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

[rubberduck-vba/Rubberduck] retailcoder merged pull request #4784: Misc bug fixes from bclothier/Rubberduck/MiscBugFixes into rubberduck-vba/Rubberduck/next

[rubberduck-vba/Rubberduck] retailcoder closed issue #4708: "Open Designer" does not work for document modules

[rubberduck-vba/Rubberduck] retailcoder closed issue #4625: Return value not used false positive given object variable

[rubberduck-vba/Rubberduck] retailcoder did something with some project card

[rubberduck-vba/Rubberduck] retailcoder is bored so why not move a project card

[rubberduck-vba/Rubberduck] retailcoder closed issue #4728: Code Inspection: Assignment not used when assigning nothing

[rubberduck-vba/Rubberduck] retailcoder closed issue #4454: Getting a parse error with no result in search results window

[rubberduck-vba/Rubberduck] retailcoder is playing around with a project card

[rubberduck-vba/Rubberduck] retailcoder closed issue #4758: MIDL artifacts shouldn't be included

[rubberduck-vba/Rubberduck] retailcoder is checking what fun stuff @Duga can say about project cards

[rubberduck-vba/Rubberduck] retailcoder closed issue #2799: Word executes AutoOpen/AutoClose and not Auto_Open/Auto_Close

[rubberduck-vba/Rubberduck] retailcoder closed issue #4779: IVbeNativeApi is bound incorrectly.

[rubberduck-vba/Rubberduck] retailcoder project card. Enough said.

Duga

04:35

[rubberduck-vba/Rubberduck] bclothier created comment on issue #4043: IRefactoringDialogs should not use Windows Forms API

> This already has been addressed in PR #4072

[rubberduck-vba/Rubberduck] bclothier closed issue #4043: IRefactoringDialogs should not use Windows Forms API

[rubberduck-vba/Rubberduck] bclothier project card. Enough said.

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

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

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

Son Goku ssj4

08:51

Hi folks. :)
Noticed you updated Test Explorer. Added time and it's not rolling up automatically anymore. GOOD JOB :-) Just wanted to say that. And I see new icons and functionality? there too :-p
Really nice changes.

Peter M Taylor

09:14

@this umm, what's the general criterion to make enterprise worthy? this is news to me.

this

10:55

@PeterMTaylor That was mainly sarcasm. In majority of "enterprise" systems, they have an overinflated and excessively deep object hierarchy, including stuff like IFactoriesFactory or IProviderProvider or something like that, which makes it very hard to use because you end up having to bring in 10 of its cousins just to execute an operation.

Duga

11:53

[rubberduck-vba/Rubberduck] ScottDennison forked us into ScottDennison/Rubberduck

2

Alexis Duque

12:15

Hi guys

I have an error in this line: github.com/rubberduck-vba/Rubberduck/blob/next/Rubberduck.Main/…

this

What does the error say

M.Doerner

@Comintern Why should the IInspectorbe responsible for combining inspection results and quickfixes or even run them? That is a different responsibility then getting all inspection results, which is its current responsibility.

Alexis Duque

'UnitTestConfigProvider' is an ambiguous reference between 'Rubberduck.Settings.UnitTestConfigProvider' and 'Rubberduck.UnitTesting.Settings.UnitTestConfigProvider'

this

hmmm.

M.Doerner

What I could see is to have an IInspectionModelProvider with an IInspector and IQuickfixProvider injected.

this

12:18

it looks like your merge took too much changes

github.com/rubberduck-vba/Rubberduck/blob/next/Rubberduck.Main/…

I bet you have a line using Rubberduck.Settings.UnitTestConfigProvider;

If so, I would delete that.

You should only have that line: github.com/rubberduck-vba/Rubberduck/blob/next/Rubberduck.Main/…

@M.Doerner As mentioned before, I'm a bit ambivalent about calling it "InspectionModel" primarily because model implies it's just data. The InspectionResult is already a model, and we are now tying it to a set of quickfixes, so it's no longer just data but rather an object with methods to execute.

That said, I admit I don't have any better suggestions for the name. I thought about the InspectionOperation but that doesn't read easily.

M.Doerner

But does it really have to have methods?

this

AIUI, Comintern wants to decouple the quickfixes from the UI

M.Doerner

You can still lazy load in a property.

I do not really see where they are currently really coupled to the UI.

Btw, the IQuickFixProvider should really be two interfaces.

It also contains an IQuickFixRunner.

Alexis Duque

@this I do not have that line that you tell me

using Rubberduck.Settings.UnitTestConfigProvider;

this

hmmm.

if you search in your Visual Studio solution for UnitTestConfigProvider, do you get more than one matches?

in the next it exists only in Rubberduck.UnitTesting/Settings/UnitTestConfigProvider.cs and no other places.

@M.Doerner That would meant the provider would end up keeping only one method (HasQuickFixes); with rest going to the runner, right?

I guess and the QuickFixes, too.

Alexis Duque

12:35

Rubberduck-next\Rubberduck.Main\Root\RubberduckIoCInstaller.cs(248)

Rubberduck.Core\Settings\UnitTestConfigProvider.cs(5)
Rubberduck.Core\Settings\UnitTestConfigProvider.cs(10)
Rubberduck.UnitTesting\Settings\UnitTestConfigProvider.cs(5)
Rubberduck.UnitTesting\Settings\UnitTestConfigProvider.cs(10)

this

I think the problem still is that the various Fix*** methods takes a IEnumerable<IInspectionResult> but Comintern wants (if I understood him correctly) something more akin to (IEnumerable<IQuickFix>, IEnumerable<IInspectionResult>) so that he can easily bind the list of available QFs for the UI.

M.Doerner

Yes, HasQuickFixes and QuickFixes, which is actually exactly what I would expect on a provider.

this

@AlexisDuque that's the problem - it shouldn't exist in the Rubberduck.Core

not here: github.com/rubberduck-vba/Rubberduck/tree/next/Rubberduck.Core/…

so I would delete the file Rubberduck.Core\Settings\UnitTestConfigProvider.cs

Alexis Duque

@this Problem solved

M.Doerner

I think the problem here is that the setting file has been moved since the last release, so git has reintroduced it when merging master into next.

this

12:45

^

M.Doerner

I hope that there are no other complications.

Probably, creating a new branch off next and cherry picking the translations into it might have been easier.

However, I have to admit that I do not know how to properly cherry pick with the CLI.

this

We'll find out once we see the PR

I think at this point, getting the PR up will help us all so we can see how to proceed from there

M.Doerner

@this I rather think the structure would be Dictionary<IInspectionResult, IQuickFix>.

this

there are more than one possible QF for a IR; you'd need at least Dictionary<IInspectionResult, IEnumerable<IQuickFix>> to avoid duplicating the IInspectionResult for each available QF?

M.Doerner

Another option would have been the do a soft reset, to commit into a new branch branched off master and then to rebase that branch onto next.

this

12:51

it also feels quite inefficient, since the numbers won't change based on the results but rather than types and types are basically static at runtime.

M.Doerner

@this that is what I meant but did not type.

this

I can see a IDictionary<Type, IEnumerable<IQuickFix>> where Type : IInspectionResult. (not correct code but that's the idea)

M.Doerner

I get the impression that that would be painful to bind.

The context menu of a result always binds to the list of its available quickfixes.

And the toolbar quickfix dropdown binds to those of the selected result.

When I talked about InspectionResultModel, I meant a struct/class with the properties InspectionResult and QuickFixes and nothing else.

Alexis Duque

[image](https://user-images.githubusercontent.com/46509440/53170932-5732b280-35af-11e9-9400-f58e5db8643e.png)

Should I erase these empty keys?

M.Doerner

That would only exist for the benefit of the UI and could be created when loading new inspection results into the backing collection, using the help of the IQuickFixProvider.

I think these keys have a different name now.

Do you see similar keys further above with a missing translation?

Alexis Duque

13:11

Yes, I already translated them.

FreeMan

great. start the morning with a reboot 'cause Excel is acting weird...

Comintern

@AlexisDuque Those keys were renamed - i.e. TestExplorer_RunMenuAllTests

Mathieu Guindon

> Rubberduck.Setup.2.4.0.4488.exe (4.03 MiB) - downloaded 840 times. Last updated on 2019-01-28

Alexis Duque

Then I can erase the empty keys.

Comintern

Are the renamed keys showing?

Alexis Duque

13:19

Yes

Duga

[rubberduck-vba/Rubberduck] bclothier added label difficulty-01-duckling to issue #4814: Code Metrics faceplanting

Comintern

OK, then yes - the empty ones will be duplicates.

Duga

[rubberduck-vba/Rubberduck] bclothier opened issue #4814: Code Metrics faceplanting

> **Rubberduck version information**
Version 2.4.0.4532
OS: Microsoft Windows NT 10.0.17763.0, x64
Host Product: Microsoft Office x64
Host Version: 16.0.11231.20174
Host Executable: MSACCESS.EXE

**Description**
After a lengthy session, I managed to arrive to a state where code metrics would not run correctly, throwing an exception. Because the code metrics is run as part of parsing, the exception runs up all the parsing stack, resulting in a parser state of `Unexpected error`

**To

[rubberduck-vba/Rubberduck] bclothier added label feature-code-metrics to issue #4814: Code Metrics faceplanting

[rubberduck-vba/Rubberduck] bclothier added label up-for-grabs to issue #4814: Code Metrics faceplanting

[rubberduck-vba/Rubberduck] bclothier added label bug to issue #4814: Code Metrics faceplanting

FreeMan

fortunately, with the SSD, it reboots pretty quick.

Duga

[rubberduck-vba/Rubberduck] retailcoder created comment on issue #4814: Code Metrics faceplanting

> Sounds like `CodeMetricsAnalyst` could try/catch its work and wrap any exception inside some `CodeMetricsException` (with the original exception as the `InnerException`), and then we could catch that in `OnStateChanged` (?), log it, and not break the entire parser state.

Wouldn't fix it, but would help segregating state-change handlers' exceptions from the rest of the parse run.

[rubberduck-vba/Rubberduck] MDoerner created comment on issue #4814: Code Metrics faceplanting

> It might be a good idea to wrap the execution of the event handlers in RubberduckParserState.OnStateChanged into a try catch block and then log and swallow all exceptions there, with the exception of OperationCanceledException.

Comintern

13:28

@M.Doerner The root of the issue that I'm running into is access to the QF provider. There's currently nothing that links an inspection result to its quick-fixes, so it makes it really difficult to work with if the UI is solely responsible for coordinating that.

M.Doerner

@Duga That will only be a safety measure of course.

@Comintern But why not derive the connection while updating the model with the new inspection results?

Comintern

Mainly because that still ties it to a specific VM.

M.Doerner

Sorry, I do not really follow.

Comintern

The example I mentioned last night was implementing r# style UI in the code panes.

M.Doerner

The generic way to combine them is to use the IQuickFixProvider, which is independent of the UI.

Comintern

13:34

Right, and that part is fine. It's more a question of what is using the IQuickFixProvider to link them.

Mathieu Guindon

Some CodePaneInspectionResultsViewModel?

this

IIWillDiscoverWhatQuickFixesYouCanHaveForThisInspectionResult

M.Doerner

How do we currently update the inspection results?

Comintern

Sure, except whatever it is will be duplicating the work of linking result to fixes.

Mathieu Guindon

IQuickFixProviderProvider is out, right?

Comintern

13:36

@M.Doerner They're coming from the IInspector - that's why I proposed doing the aggregation there instead.

M.Doerner

But that has no business with quickfixes.

this

@MathieuGuindon but the enterprise moniker!

Comintern

@MathieuGuindon No, I think not - Hosch brought up a good point that we don't need to materialize all the quickfixes.

@M.Doerner Why does the UI have any more business with it?

Something has to do that.

this

IThingamigjig, then.

M.Doerner

Really, what we are talking about is calling IQuickFixProvider.QuickFixes(result).

Comintern

13:40

I guess the root of my design question is, "why would we ever need an inspection result without knowing how to fix it"?

Mathieu Guindon

good question... we don't.

Comintern

@M.Doerner Yes - more who is responsible for making that call.

M.Doerner

But we intentionally decided to decouple them like last year in order to update all inspections whenever we introduce a somewhat brought quickfix.

Comintern

That's why I was thinking along the lines of a functional wrapper that combines the result and the fix provider to coordinate it on a per result basis.

M.Doerner

On that point we are completely aligned.

I just think that the creation of the wrapper is not a responsibility of the IInspector but whatever command is updating the model of the inspection results view.

Comintern

13:45

The reason I was leaning toward the inspector to do that is that it makes it reusable if we ever implement different or multiple UIs for applying fixes. I.e. a code pane margin.

Duga

[rubberduck-vba/Rubberduck] ScottDennison created pull request #4815: Reduce target language dependence of grammar to merge ScottDennison/Rubberduck/next into rubberduck-vba/Rubberduck/next

> Background There are currently two ANTLR v4 grammars for parsing VB6/VBA code: - The one available from antlr/grammars-v4 (duplicated at uwol/proleap-vb6-parser). - The one in this repository. The grammar in this repository is able to parse VB6/VBA code that the other grammars are unable to parse. However, the...

current grammar includes several semantic predicates that are specific to C# (which isunderstandable given this is a C# project). It may be very useful for this grammar to be used in other non-C# projects, without having to adapt the C# semantic prediates into the desired language each time the grammar is pulled from this repository. Change The semantic predicates have been changed to remove the dependence on C# and make it almost language independent...

(in still depends on the target language supporting the operators '&&', '||' and '!', which unfortunately excludes python). It instead uses methods that are defined in a base class, with the base class being in the approriate language. The C# implementation of VBABaseLexer and VBABaseParse) have been written and included. (A java version has also been written, but is not included in this pull request)

Comintern

O HAI

M.Doerner

Personally, I would just use some InspectionResultsModelProvider that takes the IInspector and IQuickfixProvider as a dependency.

Comintern

That would work just as well.

> which unfortunately excludes python

Somebody has been watching the star race. I'd swing the other way on "unfortunately".

this

Does the other repo even use ANTLR?

Comintern

13:56

I don't think so? There's at least one that uses an old version of the RD one IIR.

this

quick search suggest no. I assume it's just reading the binary directly

M.Doerner

@Duga That change looks nice.

this

@Comintern lolwut?

Comintern

The macros could be named better. I still don't know what an LA is.

this

while I kind of agree, it comes from the ANTLR

M.Doerner

13:57

True

this

it's also documented there, too

M.Doerner

I guess that comes from the Antlr documentation.

this

so if we changed it to L<whatever>T<whatever>, it would not be as obvious to someone else familiar with ANTLR

M.Doerner

Too slow

this

antlr.org/api/Java/org/antlr/v4/runtime/IntStream.html#LA(int)

M.Doerner

13:59

Well, we could call it CharacterAtOffset.

Or was that another one?

this

yes, we could but the trouble is that it now adds another layer of obfuscation above the ANTLR's API

#ChooseYourPoison

Duga

[rubberduck-vba/Rubberduck] comintern created comment on pull request #4815: Reduce target language dependence of grammar

> Nice! Obviously haven't had a chance to go over this yet, but at a glance it looks incredibly convenient if for no other reason than using non-C# tools to debug the grammar. It should be noted that the grammar does include some Rubberduck specifics, particularly the annotations that may or may not be relevant to external uses.

M.Doerner

The name in the Antlr api is really only useful for people into language processing, I guess.

Duga

[rubberduck-vba/Rubberduck] bclothier created comment on pull request #4815: Reduce target language dependence of grammar

> I might suggest that the java implementation be included. Though it won't be used in the build process, but as @comintern notes, it makes it much convenient for using java-based tools to analyze/debug the grammar so we'll still need it if only for our development purposes.

Duga

14:13

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

this

14:25

I think I will need to start wearing a rubberband. Each time I suffix a class Manager, pull it.

3

FreeMan

@Duga did you notice that he only forked the project ~2 hours prior to this PR?

this

@FreeMan Yeah, it came up in chat then.

Comintern

@this You could just write an IRubberbandManager to do that for you.

FreeMan

LOL

this

@Comintern ~.~

Hosch250

14:38

@this Snap!

FreeMan

oh snap!!

Duga

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

> @comintern
I would hope it would be easy enough for people that are not interested in the rubberduck specific annotations, by just treating `annotationList` the same way as `comment` - This is what I plan to do in the java project that will use the grammar, though I haven't investigated further yet.

@bclothier
I can commit the two java files if that is desired. However, given there is no java code (at least that I could see), where should they be placed, and under which package?

FreeMan

duck check: WHERE column = 'Some String' is going to execute faster than WHERE column like 'Some Str%', right? for most flavors of SQL, Access excluded because, well, Access...

this

Yes, for all databases, including Access.

That should be a seek, rather than a range scan.

(assuming index exists, of course)

FreeMan

Assumed as such, but you know what happens when you ass-u-me

this

14:41

I've seen people do silly things like column LIKE 'Some String'

the optimizer should transform that into column = 'Some String' but that may not always happen, esp if you write obtuse enough SQL.

FreeMan

I've done that on occasion where 'Some String' is passed into a SP and some times I need to have the LIKE condition, sometimes I know the complete string...

which probably does not get optimized away..

this

likely doesn't. Definitely not if you are not using OPTION RECOMPILE

FreeMan

@FreeMan oSnap if you're an accountant writing in VBA...

Duga

[rubberduck-vba/Rubberduck] bclothier created comment on pull request #4815: Reduce target language dependence of grammar

> I probably would create a new folder -> Rubberduck.Parsing/Development/Java

[rubberduck-vba/Rubberduck] comintern created comment on pull request #4815: Reduce target language dependence of grammar

> > @comintern
> I would hope it would be easy enough for people that are not interested in the rubberduck specific annotations, by just treating `annotationList` the same way as `comment` - This is what I plan to do in the java project that will use the grammar, though I haven't investigated further yet.

@ScottDennison Absolutely - that was more a heads up than anything else. ;-)

Comintern

Self ping.

FreeMan

15:18

BWAHAHAAHAHA...

this

DSDOS?

FreeMan

I remember DRDOS, but not DSDOS

this

Distributed Self Denial of Service

FreeMan

of course! ???

KySoto

15:45

would a SE question asking what the VB7 version of a set of API calls be on topic?

Mathieu Guindon

watch for duplicates, but in itself I don't see why it wouldn't be

this

wouldn't that be SO, rather than SE?

KySoto

probably

this

(not sure which SE it is. SE is several sites)

KySoto

im a little bit derped out due to nerve pinch pain

this

15:47

ah, sorry.

pretty sure you do want SO

KySoto

no problem

id rather not make stupid mistakes

yeah, i know the proper API calls for about half of the ones this form is doing

but the other half...

Transcript for