« first day (1007 days earlier)      last day (2173 days later) » 

04:06
Since you're only ever returning the first match, this might be more efficient:
            // statistically we'll be on an IdentifierReference more often than on a Declaration:
            var selected = selection.Value.Selection;
            var qualified = selection.Value.QualifiedName;
            var match = _referencesBySelection
                .Where(kvp => kvp.Key.QualifiedName.Equals(qualified)
                              && kvp.Key.Selection.ContainsFirstCharacter(selected))
                .Select(kvp => kvp.Value)
                .OrderByDescending(declaration => declaration.DeclarationType)
That prevents it from ever materializing the entire enumerable.
...although the sort would do that I guess. It saves the 2 potentially large memory allocations though.
problem is that if there's a With block then you can have multiple IdentifierReference instances with the same QualifiedSelection
the ctor blows up, that's what's failing 10 tests
9 now
        // statistically we'll be on an IdentifierReference more often than on a Declaration:
        var matches = _referencesBySelection
            .Where(kvp => kvp.Key.QualifiedName.Equals(selection.Value.QualifiedName)
                && kvp.Key.Selection.ContainsFirstCharacter(selection.Value.Selection))
            .SelectMany(kvp => kvp.Value)
            .OrderByDescending(reference => reference.Declaration.DeclarationType)
            .Select(reference => reference.Declaration)
            .Distinct();
Hmmm..
How is _referencesBySelection declared?
    private readonly IDictionary<QualifiedSelection, IEnumerable<IdentifierReference>> _referencesBySelection;
        _declarationsBySelection = declarations.Where(declaration => !declaration.IsBuiltIn)
            .ToDictionary(declaration => declaration.QualifiedSelection, declaration => declaration);
        _referencesBySelection = declarations
            .SelectMany(declaration => declaration.References)
            .GroupBy(reference => new QualifiedSelection(reference.QualifiedModuleName, reference.Selection))
            .ToDictionary(group => group.Key, group => group.AsEnumerable());
wtf
now it's _declarationsBySelection that's blowing up
oh
dafuq
.GroupBy(reference => new QualifiedSelection(reference.QualifiedModuleName, reference.Context.Selection))?
No, same problem.
TextBox1 and Form1 both have the same Selection.Home and QMN
04:17
Doh.
They're 0,0
Controls live on UserForms, not CodeModules.
yeah
there's a resolver test for ResolvesUsageInCodeBehind testing TextBox1.Height = 20
asserts TextBox1 resolves to the control declaration
that's the only failing test ATM
We could cheat and use the hashcode of the Control for the selection start.
Would need to use it for both start and end I guess.
or make declarationsBySelection work like the references
    private readonly IDictionary<QualifiedSelection, IEnumerable<Declaration>> _declarationsBySelection;
    private readonly IDictionary<QualifiedSelection, IEnumerable<IdentifierReference>> _referencesBySelection;
        _declarationsBySelection = declarations.Where(declaration => !declaration.IsBuiltIn)
            .GroupBy(declaration => declaration.QualifiedSelection)
            .ToDictionary(group => group.Key, group => group.AsEnumerable());
        _referencesBySelection = declarations
            .SelectMany(declaration => declaration.References)
            .GroupBy(reference => new QualifiedSelection(reference.QualifiedModuleName, reference.Selection))
            .ToDictionary(group => group.Key, group => group.AsEnumerable());
04:21
That would work.
BTW, swap the 2 conditions in QualifiedSelection.Equals.
It should short circuit on the less expensive test:
    return other.Selection.Equals(_selection) &&
           other.QualifiedName.Equals(_qualifiedName);
ha, good one
There's also an invalid cast lurking in there:
if (obj == null) { return false; }

var other = (QualifiedSelection)obj;
I suck at structs
it should implement IEquatable<QualifiedSelection>
Apparently I do too.
Why does this...
    var other = obj as QualifiedSelection;
    if (other == null) { return false; }
a struct can't be null
04:26
flag the condition as always true.
because a struct will never be null ;-)
Ah....
HTH do you do that test then?
1 min ago, by Mat's Mug
it should implement IEquatable<QualifiedSelection>
if (obj == null || (obj is QualifiedSelection) == false) { return false; }
if (!(obj is QualifiedSelection)) { return false; }
    public override bool Equals(object obj)
    {
        if (obj is QualifiedSelection)
        {
            return Equals((QualifiedSelection) obj);
        }
        return false;
    }
    public bool Equals(QualifiedSelection other)
    {
        return other.Selection.Equals(_selection) && other.QualifiedName.Equals(_qualifiedName);
    }
04:30
Much better. Pair programming FTW.
2
@the code is a mess but the nested loops are conditional so it is possible that the whole code might execute in reasonable time. That said, there is of course almost certainly a less convoluted way to do whatever OP is trying to do (though just what that is isn't very clear). — John Coleman 54 mins ago
I saw that. The worst case is 78,815,638,671,875 iterations.
that's brutal
Whew!
    public Declaration FindSelectedDeclaration(ICodePane activeCodePane, bool procedureLevelOnly = false)
    {
        return DeclarationFinder.FindSelectedDeclaration(activeCodePane);
    }
#DatRefactor
04:34
@JohnColeman - I stand corrected. It's only the worst case that runs 78,815,638,671,875 iterations. — Comintern 11 secs ago
Had to add the smiley, that came across as a little snide.
a wee bit
I hate feeling like this.
I want to start on the tests so much, but I have to go to bed soon, and I have school to do, and ...
On the other hand, if anyone likes checkers, please do beta-test my game.
@Comintern it's freakin' instant
3
04:41
Nice!
I mean you click somewhere and there's absolutely no lag whatsoever
How close are we to a super-stable 2.0?
Evidently the OP considered that a wasn't a meaningful name, and went with b, c, d, e, f, g, h, i and j instead? — ThunderFrame 11 secs ago
it's still not giving me the procedure scope when I'm in one but not on the proc name
@Hosch250 this much
We've been like that much since before my internship well, since after my internship.
04:43
seeing all the work @Comintern put into it, I wouldn't say that
OK, my mom getting ready for bed; that means I have 30+ minutes before I can.
opens VS
4
LOL, but we didn't know how far we were.
HE'S BACK!
Woot!
I don't know just what all @Comintern has been doing, but I did do my best.
04:44
I'm opening it in VS 2017. I hope it doesn't mess with the .sln or .csproj's--they changed the .sln format.
With a healthy dose of misguidance from me on some of it.
@Duga this branch is behind by ... screw it
@Mat'sMug Yup, except I did that about a month ago.
Roslyn moves so fast, you have to ditch it after about a week.
I mean, if you have tried to make changes without pulling every day.
@Mat'sMug - A unit test for #2451 might be in order, even if it's failing/ignored.
04:48
Went OK.
My internet is kind of slow since we moved the modem, though.
Wifi?
So, my IDE and browser keep hanging when downloading things.
Yeah.
We keep it in a different room now.
Used to be just behind the wall.
Do you have an external antenna jack?
No idea.
On my computer? I don't think so.
First thing I did when I moved into this house was wired the whole place for ethernet.
04:50
I'm updating Moq.
If the router has external antennae, you can want to set one perpendicular to the LOS to your PC.
@ThunderFrame That I don't know how to do and haven't done. How do you attach to Excel after it's already running?
I had company and went swimming. Just getting back.
gah, the Selection of a procedure declaration covers only its identifier, not the whole context
If it doesn't have external antennae, you can put a parabolic piece of aluminium foil behind it. ;-)
so the orderbydescending could be skipped I think\
04:53
@Comintern Umm, I wonder if putting my tin-foil hat would work.
or...
@IvenBach Debug.. Attach to Process. Choose Excel, then press Attach.
That's huge from a performance standpoint - the ordering materializes the enumerable.
C# 7:
var vbe = builder.BuildFromSingleStandardModule(inputCode, **out var component**, selection);
@Comintern OOOOH! Seems like it's going to let me go past it now. Going to step through.
04:55
Sweet, isn't it?
@IvenBach Do you have multiple versions of Office installed?
@IvenBach Let me know if the menus\commandbar work with that code. If they do, I'll PR the change and get the issue closed.
Actually, somebody else should PR it. My build is screwed half way to Sunday ATM.
@IvenBach is going to PR it
he knows how to PR from his fork to the main repo ;-)
@Mat'sMug Do you have any idea what in the world this does?
var mockHost = new Mock<IHostApplication>();
mockHost.SetupAllProperties();
And why we have that in a zillion tests, with no other references to mockHost?
there's a number of tests that do that for no reason
I remove them when I see them
04:59
So I found out.
I'm actively removing them now.
Should we check for other indexers that might be throwing in the wrappers?
definitely
That was just weird.
An exception of type 'System.ArgumentException' occurred in UIAutomationProvider.dll and wasn't handled before a managed/native boundary

Additional information: hwnd must not be IntPtr.Zero (0) or null.
05:03
@Mat'sMug Now you're just making fun of me for stuff I don't know how to do intentionally.
@IvenBach Did you get a call stack?
UIAutomationProvider.dll!System.Windows.Automation.Provider.AutomationInteropProvider.HostProviderFromHandle(System.IntPtr hwnd) Unknown
is the top
on start or on shutdown?
Copy the whole thing and paste it here.
05:05
> UIAutomationProvider.dll!System.Windows.Automation.Provider.AutomationInteropProvider.HostProviderFromHandle(System.IntPtr hwnd) Unknown
PresentationCore.dll!MS.Internal.Automation.ElementProxy.HostRawElementProvider.get() Unknown
[Native to Managed Transition]
[Managed to Native Transition]
UIAutomationProvider.dll!System.Windows.Automation.Provider.AutomationInteropProvider.RaiseAutomationPropertyChangedEvent(System.Windows.Automation.Provider.IRawElementProviderSimple element, System.Windows.Automation.AutomationPropertyChangedEventArgs e) Unknown
wow that's deep
This error occured when I was trying to go through the RD menu. I think it's my own supidity, as is usually the case, of trying to run unit tests without any unit testing modules.
WTH? That's something native.
@Comintern did you check other functions/statements for the missing whitespace? between COMMA and expression in (COMMA expression whiteSpace?)? chat.stackexchange.com/transcript/14929?m=35829134#35829134
Looks like a WPF error.
05:08
@ThunderFrame Not yet, that grammar is a bit messed up - I'm still trying to get it "fixed".
@Comintern not sure how to test that
ASSIGN should be EQ. No clue how it's working.
@Mat'sMug If you have identical identifiers with different scope (i.e. Module Foo and variable Foo) it should pick the one with the smallest scope.
so that we have something that fails if DeclarationType constants are ever reordered?
not a bad idea
@ThunderFrame That is much easier than I thought it would be :+1:
05:13
gosh I'm an idiot
@Mat'sMug Don't steal my catch phrase from me!
That's with next. I didn't test with the changes.
We're all idiots. You wouldn't believe some of the bone-headed code I've written.
2
@Comintern You can't write good code without first earning the bone-head badge.... If you get bone-head gold, you should probably consider running for President another profession.
05:18
At least my bone-head hasn't fossilized yet.
Why do I get The break will not currently be hit. No symbols have been loaded for this document. on my break points if I attach via Debug>Attach instead of <Green Triangle> Continue to load Excel via VS?
You don't want a calcium carbonate head?
RD only loads when the VBE starts
No symbols just means that you broke in imported code.
If you attach the debugger, it will load any symbols that it finds.
breakpoints aren't going to hit until RD is running, i.e. when you bring up the VBE
@Comintern it won't load anything with just EXCEL.EXE starting
05:21
But RD is already running.
oh
I appears I have missed that part
Unless you have the symbol server set up. ;-)
I get symbols loaded for some of the native libraries - not Excel though, MS doesn't publish those.
        [TestMethod]
        public void AmbiguousNameSelectsSmallestScopedDeclaration()
        {
            var code = @"
Option Explicit

Public Sub foo()
    Dim foo As Long
    foo = 42
    Debug.Print foo
End Sub
";
            var vbe = new MockVbeBuilder()
                .ProjectBuilder("foo", ProjectProtection.Unprotected)
                .AddComponent("foo", ComponentType.StandardModule, code, new Selection(6, 19, 6, 19))
                .MockVbeBuilder()
                .Build();

            var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));
Pure evil. I love it.
hmm, it's failing
05:25
Yeah, no worries - it's always been broken.
[Hosch250/Rubberduck] Ping: Approachable is better than simple.
#2330 is similar, just in the resolver.
expected variable, resolved to procedure
@Mat'sMug I'm trying to push my first commit, and I get this:
The following errors were reported during push:
	refs/heads/next, protected branch hook declined
Ever seen anything like that?
means you tried to push to rubberduck-vba/Rubberduck/next
I think
05:29
Oh crap.
RRRRRRRR
Now I need to re-clone and do all my work. Oh wait, what's the command for pushing to a specific branch?
did you not clone your fork?
I thought I did.
Wait, can I just change the origin to mine?
Can't you merge local hook to local next, then push next?
05:31
@Comintern From the bit I've tested seems your code update fixes the issue that I was having.
@IvenBach Not my code, @Mat'sMug's code.
Just put "Closes #2779" at the end of your commit message. ;-)
[Hosch250/Rubberduck] Hosch250 pushed commit f42f866a to cleanUpTests: Remove dead code
05:33
I just deleted the "origin" remote and re-added it.
That would do it.
git'r done.
okay, this one fails, and that's a problem:
        [TestMethod]
        public void AmbiguousNameSelectsSmallestScopedDeclaration()
        {
            var code = @"
Option Explicit

Public Sub foo()
    Dim foo As Long
    foo = 42
    Debug.Print foo
End Sub
";
            var vbe = new MockVbeBuilder()
                .ProjectBuilder("TestProject", ProjectProtection.Unprotected)
                .AddComponent("TestModule", ComponentType.StandardModule, code, new Selection(5, 5, 5, 5))
                .MockVbeBuilder()
                .Build();
trying to see if it's the resolver or the finder
> O_O

[#2597](https://github.com/rubberduck-vba/Rubberduck/issues/2597)
^^
oh crap
off-by-one
huh
nope, it just fails
IVBComponent component;
ICodeModule module;
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out component, out module, selection);
That clean enough for you, @Mat'sMug?
What about:
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out var component, out var module, selection);
(C# 7)
05:45
out emits the declaration, basically
Overloads that make the out parameters optional might be a good idea.
Yeah.
Or, I could make it a tuple (C 7 style?).
except it's not C# 7
heck it's not even C# 6 yet
Unfortunately.
Tuples aren't as readable.
05:46
but yeah I said I'd green-light C# 6 after the .12 release, and we're there
I'd completely support C# 6. I want me some foo?.bar;
I consider C# 6 started.
I just want that out var stuff, and the enhancements to is.
@Comintern I have two failing tests, both ignored...
So, do you think that is good enough for now, or do you envision something even cleaner?
05:50
Works for me. I'll take performance with identical incorrectness.
What works?
The cleaner test method, or something else?
Failing tests that we knew would fail.
oh, and I don't know what took so long to add this ctor:
    public Selection(int line, int column) : this(line, column, line, column) { }
I'd like feedback before I change some 1600 tests...
Before:
var builder = new MockVbeBuilder();
IVBComponent component;
var vbe = builder.BuildFromSingleStandardModule(inputCode, out component, selection);
var module = component.CodeModule;
05:52
@Mat'sMug Those are probably all over the place. I should tee up and golf some of the RD code...
After:
IVBComponent component;
ICodeModule module;
var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out component, out module, selection);
I still want C# 7 there
I still say overload.
05:53
Overload what?
if you don't need the component or the module, then make an overload of BuildFromSingleStandardModule that doesn't have the out params
There are plenty of tests where I don't care about the component or the module.
Oh.
Sure.
Give me an example.
oh crap, I thought I had pulled the remote changes
O(well)
Oh, there are some pretty funny puns in the ancient texts I've been reading.
Like, some culture said "we'll make the totem pole out of chestnut (li) because the people should fear (li)."
05:57
@Mat'sMug looks like the code below fixed my issue
` private void DeleteExistingCommandBar(string name)
{
try
{
var existing = Target.Cast<Microsoft.Office.Core.CommandBar>().FirstOrDefault(bar => bar.Name == name);
if (existing != null)
{
existing.Delete();
//Marshal.FinalReleaseComObject(existing);
}
}
catch
{
// specified commandbar didn't exist
}
}`
that's awesome
that issue highlighted a serious problem with the COM wrapper indexers
indexers. the enumerators work.
^^
There's only one other one I could find, but it calls the _ComObject directly.
public ICommandBar this[object index]
{
    get { return new CommandBar(IsWrappingNullReference ? null : Target[index]); }
}
Actually, that wraps the one that we bypass.
662 references.
No way that's right.
@Hosch250 MultilineParameter_ReturnsResult
...and several hundred others.
@Mat'sMug You've already incorporated it into a PR, right?
06:05
[retailcoder/Rubberduck] retailcoder pushed commit 676a62c4 to rd-next: refactored FindSelectedDeclaration
[retailcoder/Rubberduck] retailcoder pushed commit 3676cbdd to rd-next: fixes #2805
[Hosch250/Rubberduck] Hosch250 pushed commit da1e23e7 to cleanUpTests: Add methods
OK, @Mat'sMug and @Comintern, please review that ^
I want to get that right before I start making 101 changes.
Although, from the sounds of it, in C# 8, we'll be able to just use wildcards for the out params.
[retailcoder/Rubberduck] retailcoder pushed commit 60838a0a to rd-next: more merge artifacts fixes
In a new Workbook: Rename Sheet1 to Buzz (and codename to Buzz), rename Sheet2 as Fizz but don't rename the codename. Add a worksheet-level name called Buzz to sheet Buzz, and also to sheet Fizz. Then drop this in a standard module:
Option Explicit

Enum Fizz
  Buzz = 4
End Enum

Sub test()
  Fizz Buzz
End Sub

Public Sub Fizz(Optional Fizz As Fizz = Buzz)
  Dim Buzz As Fizz
  Debug.Print Fizz, Buzz, [Fizz], [Buzz], [Buzz!Buzz], [Fizz!Buzz], VBAProject.Buzz.Fizz
End Sub
It was planned for C# 7, but they decided to cut it.
@Hosch250 out parameters that aren't at the end of a signature... feels weird IMO
maybe it's just me though
06:07
and in sheet/component Buzz, add:
Option Explicit

Public Property Get Fizz()
  Fizz = 6
End Property
I'd agree.
Except what about those other two parameters?
@Mat'sMug That's funny - I usually put them first.
We only use them in a few tests. More overloads?
OVERLOAD ALL THE THINGS!!
06:08
Or, really, the typical way is to provide one short overload, and one long one.
So, really, I could do one without the module or the component, and one with both.
And one that requires everything.
just create that you see as a real need as you refactor the tests, no?
Sure, but you approve what I have so far?
@IvenBach I only had the CommandBars fix
@Hosch250 so far so good
I'd expect more red than green in further commits though
06:14
> This PR closes #2805 and severely refactors the FindSelectedDeclaration method, formerly implemented in the RPS (now in the DeclarationFinder, but still exposed on RPS). Also adds two new failing/ignored tests.
And I thought I was evil...
Wait a sec.
The module just returns component.Module.
What about just inlining that?
absolutely
06:14
^
But you still prefer the static method?
All things equal, I'd think a static would be safer.
whatever rocks your boat
[rubberduck-vba/Rubberduck] build for commit 60838a0a on unknown branch: AppVeyor build succeeded
You are going to be in the boat a lot longer than I, probably.
06:17
Is it just me or is that screener a parser issue?
We shouldn't be evaluating anything inside a foreign-identifier.
@Comintern better: the Fizz procedure call resolves to the enum
FML
CURSE YOU VBA!!!
11
on that note, I'm going to bed
@Mat'sMug Thanks for finding a resolution to the issue I was having.
06:21
@Comintern's solution would have worked just as well :)
thanks for reporting that issue in the first place - it's hard to fix a bug you're not having
@Comintern Actually, I think VBA treats the [Fizz] as a scoped identifier if a local scope exists, and otherwise treats it as a foreign identifier.
@IvenBach I just merged my PR, you can bring in these changes by making a new PR to your fork
[rubberduck-vba/Rubberduck] build for commit 7e7e87bc on next: AppVeyor build succeeded
@Mat'sMug Bear with me as I try to get it right the first time.
06:25
Oh man... I think I just got it to work, the first time!
@Comintern that is correct. Module1.Fizz is the scope, :Buzz is the identifier, (variable is its declaration type, and :Fizz) is that variable's type, which is declared As Fizz, the enum type
gosh what a mindwreck
@IvenBach awesome!
VBE thinks [Fizz] is the function name instead of the parameter name
VBA knows it is the parameter name
RD doesn't
Doesn't [Fizz] refer to the evaluate member of the implicit scope?
I.e. VBA.Global?
I think we need better language specs
06:28
From the COM spec, [Fizz] would resolve to VBA.Global.Evaluate("Fizz") where Evaluate is the evaluation member.
but does it evaluate in the execution context of the Fizz procedure?
I think VBA needs to have a spec.
@Comintern I think VBE treats the brackets as an escaped local name first, and then an external identifier?
@ThunderFrame looks like it
the good news is that that's what RD does anyway
> In some VBA contexts, a <FOREIGN-NAME> whose name value is identical to a <reserved-type-
identifier> can be used equivalently to that <reserved-type-identifier>. The identifier whose name
value is "object" is not a <reserved-identifier> but is generally used as if it was a <reserved-type-
identifier>.
That's clear as mud.
06:30
I need to think more EVIL - need a range called Fizz and a range named Buzz, and an intersection operator in en external name of `[Fizz Buzz]...
"the identifier whose name value is "object" - IOW the Object keyword
[Hosch250/Rubberduck] Hosch250 pushed commit a781419c to cleanUpTests: Clean up Remove Parameters tests; temporarily comment out broken tests
I'm going now.
so Object isn't a reserved identifier but generally used as if it were a reserved type idetnifier, whoop-dee-woot
> Excel is trying to recover your information
I need to sleep
</mug>
@Mat'sMug @Comintern, and anyone else: Please look through this and see if you want anything else while I'm at it:
06:32
tomorrow :)
I'll work on the commented out ones as I have time.
See you.
WTH does VBA let you name a class module Object?
with or without the question mark ;-)
AFAICT it's impossible to new it up.
Predeclare?
06:35
? is a type hint for WTF.
With Object?
Class Object:
Implements Missing
Implements IsNull
Predeclared fails too.
Dim bar As New Object '<-- Expected identifier.
  Dim o As VBAProject.Object
  Set o = New VBAProject.Object
^Works
looks in Locals Window: o is of Type Object/Object
Spoke too soon. Predeclared works too:
'Class Object
Public Sub Report()
    Debug.Print "Ola!"
End Sub

'Module
Public Sub Foo()
    Object.Report
End Sub
gah - TTQW
06:42
Too bad IDispatch is restricted:
'Class Object
Implements IDispatch

Public Sub Report()
    Debug.Print "Ola!"
End Sub
OK, TTGTB.
IUnknown?
Restricted.
Public Sub IUnknown_Release()
    'Nah, don't think so.
End Sub
Given an empty IFoo class module:
Implements IFoo

Public Sub IFoo_Release()   '<-- compile error "Function or interface marked as restricted."
    'Nah, don't think so.
End Sub
...which raises the question - is it safe to wrap a COM object with a Release method?
 
3 hours later…
10:11
Hi guys,
I have sync my fork to 2.0.13 that Matt recently updated and how you both paired to resolve the selected object
I found that I could not login Github at the moment as my phone isn't receiving an SMS from 2-factor verification so I will post this picture here.
let me know if you want any further testing I am happy to re-sync and re-run on my PC when I get home.

« first day (1007 days earlier)      last day (2173 days later) »