VBA Rubberducking

FreeMan

8:00 PM

Heck, that's a showstopper for Refactor|Rename!!

Mathieu Guindon

(as is AC being annoying)

FreeMan

lol

Comintern

@MathieuGuindon Yup. I think you're right. At this point I would also consider the subclassing attachment and event stack as a show stopper too.

this

there's the hotkey (not sure if that's a separate thing from subclassing attachment)

we also should have an unit test for that case to avoid that regression, too.

Comintern

@this IIR, the hotkeys use the subclass class.

this

8:03 PM

@Comintern not sure what you are referring to by "event stack"?

Comintern

@this We register for unmanaged events, then translate them into managed events. That's what I consider the "stack".

this

@Comintern NAIUI. github.com/rubberduck-vba/Rubberduck/blob/…

@Comintern gotcha

Comintern

Huh. I thought the RegisterHotKey was busted in Office.

this

This needs to go anyway. It's a bit too "global"

Wayne suggested using uh, what was it? SetKeyboardHook? Which Mat is already doing in the subclassing for AC (right?) so we just need to pimp it a bit more.

Comintern

Pretty sure I tried SetKeyboardHook at one point. Maybe that's the one I was thinking of. Something kept trashing the keybuffer.

Mathieu Guindon

8:10 PM

@MaciejLos sure. And then come back here with another question when their code breaks down because they're passing typed arrays around without wrapping them in a Variant. Typed arrays in VBA are an utter mess. Use Variant and save yourself the headaches. I'll wear this downvote as a badge of honor. — Mathieu Guindon 8 secs ago

this

@Comintern no, i didn't remember the name -- it's actually SetWindowsHookEx -- link: chat.stackexchange.com/transcript/14929?m=42127159#42127159

Comintern

chat.stackexchange.com/messages/30523467/history

this

hmm.

Comintern

Tried that. That may? have been the attempt where Office trashed the keystate? I think I tried 3 or 4 different API methods, but Office doesn't follow the subscriber rules in those message streams.

this

that sucks

do you remember if you tried the low level?

there's two ways to hook into keyboard, one being the low level.

Comintern

8:18 PM

answers.microsoft.com/en-us/office/forum/…

^^ Is that the other one?

this

yeah, not reassuring at all

Comintern

IKR?

I must have re-written that a dozen times.

this

guess the only way we'll ever improve the hotkey is to actually use avalonedit.

Comintern

That won't solve the issue.

The hotkeys need to be responsive in any number of different windows, and they aren't all editors or controlled by RD.

this

Oh, yeah, you're right.

Comintern

8:22 PM

That's part of the reason the event stack is there - it's a way to keep track of focus changes between managed and unmanaged windows.

2

A: C# Global keyboard hook in Office Addin 2013

The issue is not Powerpoint specific, it occurs with any Office product. I'm working on a Outlook addin and fancing this problem. It has been reported (without any answer unfortunely) to Microsoft: https://social.msdn.microsoft.com/Forums/office/en-US/93d08ccc-9e77-4f72-9c51-477468d89681/keyboa...

^^ Now that's new. Might warrant a closer look.

We also tried RawInput.

M.Doerner

Hm, I cannot build RD anymore.

This part of the error message is a bit irritating:

error CS0006: Metadata file '..\RubberduckCodeAnalysis\bin\Release\RubberduckCodeAnalysis.dll' could not be found

Why Release?

this

because it's used by the project itself

Mathieu Guindon

huh?

@this that's confusing, no?

this

8:37 PM

I had set it up to avoid it getting picked when debugging.

Mathieu Guindon

hmm

this

in the configuration manager, the RubberduckCodeAnalysis is set to build Release when the solution is Debug

Yes it is confusing and I don't like that but that was necessary to ensure that when the analyzer runs on the other projects, we are using release version

Duga

[rubberduck-vba/Rubberduck] daFreeMan created comment on issue #4202: 'Rename Parameter' QuickFix fixes too much

> Very interesting...

I've come across 2 `Consider renaming variable 'RS'` inspection results. Fixing the first one did _not_ fix the second one...

```VBA
Dim RS As ADODB.Recordset
Set RS = CurrentDb.OpenRecordset(SQLString)
Do While Not RS.EOF And Not RS.BOF
...
```
and
```VBA
Dim RS As ADODB.Recordset
Set RS = New ADODB.Recordset
Set RS = cmd.Execute
```

And now, after reverting the change (`Ctrl-Z`), I'm only getting 1 inspection result despite me being

this

The alternative would be to push it into its own solution but I did not like that since that makes it hard to ensure that all contributors and AppVeyor are getting updates on the RubberduckCodeAnalysis project.

FreeMan

TTFN - I might get started on updating my local RD repository tonight if my file server will quit causing me issues...

(RD is a lower priority than ensuring all the family photos are safe and sound!)

this

8:49 PM

@M.Doerner are you able to build the RubberduckCodeAnalysis project?

Comintern

@this Would it make it less confusing to copy the release binary to the debug folder as the final build action?

this

@Comintern that'd make the output a total lie?

Comintern

Then put it somewhere else? Do we need to be able to access the debug build?

this

normally, no.

Vogel612

honestly code analysis seems to be a bit... weird

this

8:52 PM

you only need the debug when you are actually adding some new analysis rule.

weird how, Vogel?

Vogel612

it's recorded in the csproj settings

but it's not visible from the Code Analysis in the properties page of a project

this

it should be.

Vogel612

also it doesn't generally run on every build, but when you run Analyze

Comintern

It's also tied to a specific build configuration, so it isn't like we're hiding anything.

this

there were 3 projects or so that didnt' have it included, which my last PR fixed.

you don't run Analyze

Actually I never tried that, not sure what it does

it will run either realtime or when you try to build a project

Vogel612

8:54 PM

runs the analysis ruleset specified in project properties

running that yields 448 warnings

this

all analysis in the analyzer will be an error

Vogel612

most of them seem rather... inconsequential, others seem stupid

this

448 warning are probably from the default ruleset

we only have like.... 4 or 5 rules.

to observe this yourself, go to any COM visible class, comment out the Guid attribute

and try to build. you'll get an error

Hosch250

@MathieuGuindon Big news.

M.Doerner

Hm, now it builds.

Phrancis

9:00 PM

@FreeMan Maybe I'll make it a point to come in here more often, I enjoyed it :)

Mathieu Guindon

@Hosch250 what's up?

M.Doerner

I switched the build option to Debug: build failed.

Then I switched it back to Release: build succeeds.

this

it should build even in Debug.

Hosch250

@{
    string Foo()
    {

    }
}

That's legal now in ASP.NET MVC.

M.Doerner

However, when it was on Release originally, it failed.

Hosch250

9:01 PM

Because it sees it as a local method, which is legal in C# 7.

M.Doerner

Oh, the analysis project itself always built.

The new ones referencing it did not.

Mathieu Guindon

@Hosch250 that's cool... I really need to do something about the website... that'll be handy.

Hosch250

You'll have to update it, and probably a couple NuGet packages, IIRC.

M.Doerner

Now, failing again.

Hosch250

I use it in my checkers website, but didn't realize it was a C# 7 feature until I just tried to use it at work.

this

9:03 PM

@M.Doerner for the new project, check the Analyzer

Hosch250

We are stuck with C# 5 on the ASP.NET MVC side because we've got old code using it that "works" and we won't update because it's almost obsolete.

this

do you see a yellow icon on the RubberduckCodeAnalysis?

(it's under hte references)

M.Doerner

This is stupid, the first build after a clean fails, the second works.

Vogel612

@M.Doerner try to override the build order

build the analysis projects first (for both Rd and the tests)

Duga

[rubberduck-vba/Rubberduck] DevulskyA opened issue #4203: Creating Wrong "End Sub"

> Public Sub ImportDoc()

End Sub '<--- ERROR Created by RUBBERDUCK

Dim Ret As Long

'FileName = "MODELO_DE_EDITAL.docm" ' Arquivo DOCM com o modelo de edital. Deve estar sempre em branco. Só contém código.

Dim FileName As Variant
FileName = "MODELO_DE_EDITAL.docm" ' Arquivo DOCM com o modelo de edital. Deve estar sempre em branco. Só contém código.
'PathSHAREPOINT = "\\MODELS\exemaple.docx" ' Local principal mas...
'PathF = "\\fsadm1\SAD\COMP\5 - SEEDI\MODELOS

M.Doerner

9:04 PM

Never had to do that before.

Duga

[rubberduck-vba/Rubberduck] Vogel612 edited issue #4203: Creating Wrong "End Sub"

this

damn, I now know what is going wrong

I completely forgot to make the projects dependent on the code analysis project

Duga

[rubberduck-vba/Rubberduck] Vogel612 added label duplicate to issue #4203: Creating Wrong "End Sub"

[rubberduck-vba/Rubberduck] Vogel612 added label feature-autocomplete to issue #4203: Creating Wrong "End Sub"

this

referencing them is not enough - it will try to build in parallel and fail

Duga

[rubberduck-vba/Rubberduck] Vogel612 added label bug to issue #4203: Creating Wrong "End Sub"

[rubberduck-vba/Rubberduck] DevulskyA edited issue #4203: Creating Wrong "End Sub"

M.Doerner

9:06 PM

Ok, how do I do that?

this

right-click solution

choose Project Dependnecies

for each project that's Rubberduck.***, check off the RubberduckCodeAnalysis`

Duga

[rubberduck-vba/Rubberduck] DevulskyA edited issue #4203: Creating Wrong "End Sub"

M.Doerner

Found it already in the Project menu.

Which ones do I need to tweak?

Comintern

@M.Doerner That's probably because of a reference to the release executable. Is the second run pulling the release .dll into the debug bin path?

Duga

[rubberduck-vba/Rubberduck] DevulskyA edited issue #4203: Creating Wrong "End Sub"

this

9:08 PM

IIRC, there were 3 projects--- Resources, Interaction, and Refactoring

@Comintern no that's not the problem

the problem is that it will try to build in parallel

Vogel612

you'll probably only need Resources and Interaction

this

which you can't since you need that DLL built before you try to build the other project.

Duga

[rubberduck-vba/Rubberduck] DevulskyA edited issue #4203: Creating Wrong "End Sub"

this

@Vogel612 in theory, all projects should be validated.

If they are not referenced, then we can't know that we violated a rule in that project.

Vogel612

correct, but since Refactoring depends on Resources we only need to make Resources depend on the analysis to correct the build order

this

9:11 PM

ah, I see what you mean. Sure, but I did it explicitly anyway. #BeltsNSuspenders

M.Doerner

What should the tests depend on? RubberduckTestsCodeAnalysis?

yes

no, doesn't apply

huh why?

it's its own thing.

all it does is test the RubberduckCodeAnalysis

Vogel612

9:12 PM

aaaah....

this

I did at one point try to stick it in RubberduckTests but it was not pretty....

M.Doerner

OK, should it depend on the usual analysis project then?

this

the tests? No.

only Rubberduck.*** need to

M.Doerner

OK

this

sorry for the inconvenience.

IvenBach

9:14 PM

@Vogel612 do you use git rebase much?

M.Doerner

Ok, it works now.

Vogel612

@IvenBach depends... not anymore

I used to

M.Doerner

Time to run all test and get a cup of tea in the meantime.

IvenBach

@Vogel612 rebase only for when you want to have things in a specific order?

Hosch250

I pretty much only rebase when I do a bunch of work in a different branch.

Then I do a soft reset, commit, and force-push to the remote to clean things up, then rebase on the master, so I just have to resolve conflicts once and the commit history is cleaner.

this

9:16 PM

@Hosch250 I would love to see the rule for chained access on com wrapper implemented. Still up to it?

Hosch250

Yeah, but I forgot.

Ping me again late Friday.

this

No worries. Deal.

Vogel612

@IvenBach and when I want to squash commits

and I used to be a big fan of the feature-bubble, because that made it significantly easier to read the console output for log --graph

IvenBach

I've had too many 'I forgot' commits. Squashing in one I need to get better at.

M.Doerner

@ThunderFrame Of all your hiddeous line continued examples in the issue, the current parser only fails at the underscore at the start of comment and at the start of the line ones.

My next PR will fix those as well.

Duga

9:25 PM

[rubberduck-vba/Rubberduck] Vogel612 removed label duplicate from issue #4203: Creating Wrong "End Sub"

IvenBach

@MathieuGuindon For our BDFL ^

Vogel612

@IvenBach where's that from?

IvenBach

learngitbranching.js.org

The 2nd remote tab at the top.

Vogel612

that is pretty cool

it doesn't support reflog, though :/

IvenBach

I need kindergarten colors to help me understand sometimes. This is doing the job pretty well.

ThunderFrame

9:44 PM

@M.Doerner yes I know, the other examples were designed to ensure your changes don't break them in the process.

M.Doerner

Which issue was it again to which you added the parser label a few days ago, the one with the line continuation in front of a line number?

Comintern

@M.Doerner Was it #4196?

M.Doerner

No, that is the underscore one.

I mean the one where there was a line number that did not have to be at the start of the line because there was a line continuation on the previous one.

Comintern

I think that had an indenter tag on it.

Vogel612

#3210

Comintern

9:51 PM

Like a label and a line number?

M.Doerner

Thanks

I will try to fix that from the parser side.

this

@M.Doerner will your PR include the change to the solution file regarding project dependencies?

otherwise I will make a PR to ensure other contributors don't run into same problem you had earlier.

M.Doerner

10:08 PM

Yes, it will.

BTW, why do we have tests that combine like 12 tests in one?

this

name of test?

M.Doerner

I just disected it: AssignedByValParameter_PassByReferenceQuickFixWorks.

this

might been a old test written long before data driven test was a thing

Duga

[rubberduck-vba/Rubberduck] BZngr synchronized pull request #4127: Unreachable Case Inspection: follow-up

[rubberduck-vba/Rubberduck] [codecov[bot]](github.com/apps/codecov) edited comment on pull request #4127: Unreachable Case Inspection: follow-up

> # [Codecov](https://codecov.io/gh/rubberduck-vba/Rubberduck/pull/4127?src=pr&el=h1) Report
> Merging [#4127](https://codecov.io/gh/rubberduck-vba/Rubberduck/pull/4127?src=pr&el=desc) into [next](https://codecov.io/gh/rubberduck-vba/Rubberduck/commit/5d5a35562ebe653848201b5cd477b5eaf8c6068a?src=pr&el=desc) will **increase** coverage by `0.4%`.
> The diff coverage is `93.21%`.

```diff
@@ Coverage Diff @@
## next #4127 +/- ##
==========================

[rubberduck-vba/Rubberduck] BZngr commented on Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTre‌eExpressionEvaluator.cs of pull request #4127: Unreachable Case Inspection: follow-up

M.Doerner

It should just be seven separate tests.

No data driven tests necessary.

Duga

10:33 PM

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

M.Doerner

Hm, this is more complicated than I thought.

Duga

[rubberduck-vba/Rubberduck] [codecov[bot]](github.com/apps/codecov) edited comment on pull request #4127: Unreachable Case Inspection: follow-up

> # [Codecov](https://codecov.io/gh/rubberduck-vba/Rubberduck/pull/4127?src=pr&el=h1) Report
> Merging [#4127](https://codecov.io/gh/rubberduck-vba/Rubberduck/pull/4127?src=pr&el=desc) into [next](https://codecov.io/gh/rubberduck-vba/Rubberduck/commit/5d5a35562ebe653848201b5cd477b5eaf8c6068a?src=pr&el=desc) will **increase** coverage by `0.56%`.
> The diff coverage is `92.74%`.

```diff
@@ Coverage Diff @@
## next #4127 +/- ##
=======================

M.Doerner

The SLL parser does not like my solution.

Duga

[rubberduck-vba/Rubberduck] build for commit b39ad352 on unknown branch: 52.78% (target 0%)

Mathieu Guindon

@M.Doerner but it only dislikes comment-underscore, right? We can live with that I think

(sorry if that's not relevant, I need to catch up with the transcript)

Comintern

10:40 PM

Sanity check. Is a With New Foo context the only way in VBA to have a member access on New Foo?

Mathieu Guindon

That I know of, yeah

M.Doerner

@MathieuGuindon It is not about the underscore; I fixed that already.

It ia about the line numbers and line labels not on the start of line because thay are preceded by a line continuation.

Comintern

OK, that's what I thought.

Duga

[rubberduck-vba/Rubberduck] BZngr created comment on pull request #4127: Unreachable Case Inspection: follow-up

> The latest update/sync addresses the comments to date including some general improvements:
1. `Date` type is now supported (decorates the `DateValue` type from the `Preprocessor` to add `IComparable<T>` support for `ExpressionFilter `ops).
2. Improved the result `Type` determination of all operators and their operands. The Unary and Binary OperatorDeclaredType combinations from the VBA Language spec are encapsulated within a (new) `OperatorDeclaredTypeProvider` class.
3. Refactored/Renam

Comintern

@M.Doerner That's what breaks it in the indenter too.

M.Doerner

10:46 PM

Ok, fixed it from the parser side. PR incoming.

We are now a bit more permissive than the VBE, but I do not really care.

Hm, I should write one more test.

this

@M.Doerner as long it's not possible to write compilable but unparsable code, the infidelity is fine. ;)

M.Doerner

That one is failing, and I know why.

That approach will not work.

It would allow to put a line number on a statement after a statement separator followed by a line continuation.

Duga

[rubberduck-vba/Rubberduck] BZngr added label PR-Status: Review Requested to pull request #4127: Unreachable Case Inspection: follow-up

this

@M.Doerner wouldn't that be a compile error?

M.Doerner

Yes

I said that we are more permissive than the VBA in this case.

However, my problem is that the line continuation does not contain the trailing whitespace.

And if I add it to the rule, I break multiple line continuations in a row.

I think I will leave fixing it for the SLL parser for later.

mansellan

11:50 PM

Fun fact: RD still works fine in Office 2000 :-)

#BecauseWhyNot

Comintern

:-)

Phrancis

@mansellan I'm surprised Office 2000 still works fine all by itself lol

mansellan

@Phrancis well, for certain values of "fine" :-)

Comintern

WTH is wrong with my unit test. The code works in the "live" environment, but I can't get NUnit to pass the exact same code and setup.

Oh, so now it passes with no changes. WTH?

mansellan

Clippy!!! Back from the dead!

Comintern

11:54 PM

Clippy needs a new alpha mask.

mansellan

yah, I think I broke him

Comintern

I think if you search this chat, there's a discussion of an Agent for RD...

mansellan

lol!

Comintern

Ah, yes: chat.stackexchange.com/transcript/message/29106070#29106070

mansellan

on a more serious note, looks like an Office install (any Office install as proven here) gets VB6 working again. So I'm less worried about #4192 for now. Number of VB6 users without any Office install must be very low.

Duga

11:57 PM

[rubberduck-vba/Rubberduck] comintern assigned comintern to issue #3343: Member not found inspection doesn't fire for With Block variables

Comintern

Wasn't there some re-distributable component of VB6 that they stopped shipping with Windows Vista?

Like MS Common controls or something like that?

Transcript for