VBA Rubberducking - 2017-12-11

« first day (1284 days earlier) ← previous day next day → last day (1896 days later) »

12:00 AM

RELOAD!

[rubberduck-vba/Rubberduck] 17 commits. 1 opened issue. 4621 additions. 9314 deletions.

[Zomis/AdventOfCode2017] 2 commits. 69 additions. 6 deletions.

14 hours later…

2:12 PM

@this Yeah, but I couldn't get past the last roadblock.

@Mat'sMug I threw my work away. It's not that hard.

@NelsonVides Nah, we'll just use this chat.

I'll take another look sometime this week/next week (I have next week off).

Just keep me posted, and I'll let you know if I've worked through the problem already.

@Hosch250 Hey there!

so let me go through my changelog here:

FYI, I'm at work now, so my responses may be sporadic.

taking VBAParser and VBALexer, I generated Lexer, Parsers, Visitors, and Listeners, all using the last version of antlr from the website. Only had to add the namespace `Rubberduck.Parsing.Grammar` to the Parsers.
Then, on the project, I got rid of all the ANTLR references and installed using NuGet the Antlr.Runtime.Standard, 4.7.1.1, on all the necessary subprojects of RD.
Then API breaking stuff: things that were typed as `IReadOnlyList<>` are now plain [] arrays, which I had to cast to the mentioned `IReadOnlyList` to make the code work. Other methods have been fully capitalised (`context…

now my chat is not inlining the code, what have I missed... (?)

don't worry, whenever you can, I'm also at work :P

so, with all of that, everything compiles flawlessly. And 600 tests are failed and another 900 ignored :(

2:27 PM

Ummm, wow.

OK, which files did you change?

Because you can't edit the generated files at all.

@NelsonVides did you actually change all the derived listeners and visitors? Those only need to derive from the VBAParserListenerBase and VBAParserVistiorBase, both which are generated. The derived aren't generated, however.

Also, you said you used the last version from the website.... but is it also the 7.1.1?

Isn't it 4.7.2, or something like that?

Yeah and I think the generated files has to be at the same version of the runtime

Also, with the tip about _input, I might be able to get past my last roadblock now.

let me push everything to my github so you can have a look, one sec

now: github.com/NelsonVides/Rubberduck/tree/antlr47

2:39 PM

There's also the preprocessor and expression-evaluation grammars

github.com/NelsonVides/Rubberduck/blob/antlr47/…

Yeah, that's just not going to work.

We actually need to tweak our grammars to work with the newer version.

yeah, I didn't change the .g4 file, I just updated the generated one with the change I mentioned

You just can't do that.

That's where I was being blocked.

I generated the new .cs files from the original .g4, using the last download from here: antlr.org/download.html

#1, I didn't know what to replace _input with.

2:42 PM

we can't redo these changes every time there needs to be a grammar tweak, it's not sustainable.

yeah, the most feasible change would be to tweak the grammar itself

#2, Their runtime uses .SLL, while their generator creates .Sll (the biggest problem).

I don't know if that's in the grammar too, or if it's the generator.

I was just testing if it would work to change it manually for now

OK.

which reminds me, anybody knows how to make it add the namespace automatically? the generated file didn't have any namespace attached and then the rest of the build was not finding the parser class

2:45 PM

Hmm, I'd have thought that if _input was now invalid, the files should have never been generated at all?

I thought that that one (github.com/NelsonVides/Rubberduck/blob/antlr47/…) is just a harcoded string that the builder throws on the new .cs file (?)

@this It just sticks it in as a raw string :P

groans

I hate that kind of thing

if I give it bad input, don't make more nonsense with it, mmmkay?

It's not a compiler, it's just a parser.

You gave it valid input, so it generated its output, even though it was invalid to the compiler.

hmph. too much work

So am I to understand that @NelsonVides did change the generated files' _input to TokenStream to get it compile?

2:51 PM

yes, I manually changed the generated files

Ok, I follow now. The trouble however is if that if you have 600 failing, then the changes weren't 100% identical

just added the namespace, _input into TokenStream, and capitalised the .Sll into .SLL. Those changes were manual into the generated files.

don't knwo what got weird on the API, I'm having trouble to find changelogs and detailed documentations of antlr

Thanks for the tips. I'll see if I can take them and run with it :)

the namespace and capitalization should have been trivial changes

if the _input -> TokenStream was the only major change, then it's likely the culprit, especialy if they aren't equivalent (?)

It was my main suspicion, but I could perfectly be wrong somewhere, as I said, I had trouble finding a proper documentation

3:00 PM

assuming it's the parser class....

antlr.org/api/Java/org/antlr/v4/runtime/Parser.html#Z:Z_input

Suggests that InputStream should be used, maybe TokenStream is too generic or something. IDK.

2 hours later…

4:54 PM

@NelsonVides Which command line options did you use when generating the .cs files?

5:32 PM

@M.Doerner if I recall correctly: -DLanguage=CSharp -visitor -listener -VBAParser.cs

6:05 PM

That looks about right. I just hope the options are not case sensitive; the documentation uses the option Dlanguage.

I may have written Dlanguage this morning, I just told you by memory a while ago

if it were case-sensitive, I think it'd have either erred or generated nonsense that wouldn't have been compiled by the project.

6:41 PM

@this just got off the phone with Cheryl Ng of the Microsoft MVP award program

and?

@Mat'sMug Curious ducklings want to know.

Nomination is being put forward next month, if not this Thursday.

She laughed when I said I cringed at some of the VBA code being put online by some Excel MVPs

3

It's nearly official. The Mug is going to be taken from his duckling realm and introduced to the big mallards.

Very good. My plan for world domination is coming to fruition.... Soon! Soon! cackles

@Mat'sMug just to keep things in perspective - not all MVPs are MVP because of their coding prowess. They are MVP because they've done great things with Excel (maybe formulas? maybe reporting? maybe Power*** something?). There's lot of diversity in the bunch.

7:01 PM

Yeah. She kept saying VBA is part of Excel and I kept biting my tongue every time.

Actually I did say to me VBA was a completely separate thing,...but 90% of RD users are using it in Excel anyway - as I am. So Excel category made the most sense.

yeah. I am inclined to agree. They did used to have a category.... Office Systems? Office Development?

but that was since re-categorized few years ago

as such there is no direct category for VBA development.

She seemed pretty impressed with the SO activity and reach

Anyway, it's looking good

keep it up! I'm sure you'll get it. I think you're worth it.

Thanks!

7:26 PM

Did you think it was a prank call at first?

lol nope, I called in :)

It's time I be recognized for my contributions!

I really can't envision that coming from Mug.

@IvenBach trust me, he didn't.

8:16 PM

Now that I know unit tests should be fast seeing 23 seconds for a single unit tests lets me know I didn't set it up correct the first time. #GottaStartSomewhere

that's at least a thousand times too slow :)

LOL.

Unit testing your hangman?

Nope. Refreshing my mind regarding changes I made to a file. I'm still waiting on an update to be handed to me to integrate divergent copies...

I do need to try and make progress on hangman though.

OK. No hurry.

Need to figure out how to abstract out access to a worksheet and so many other things before it though.

I put the pressure on myself. I want to rewrite this workbook so it's coded properly.

8:27 PM

I feel you.

I'm rewriting a widget to make it more accessible and to try to make it cleaner.

I don't think it's going to be any cleaner, though.

If anything, it's significantly messier.

Before, we were using Knockout JS to handle sending data live between the UI and the JS.

It was really handy, but some people used it way, way too much.

Unfortunately, KO doesn't support certain accessibility software that we are targeting, so now we can't use it.

So...

We are using jQuery, and it's quite a mess to work with highly dynamic UIs.

Before, we could just do data-bind="if: condition" on an element, and it would automagically show/hide.

@Hosch250 when you say they used KO too much, you mean they are basically making a database out of the JS front-end?

Or data-bind="text: variableName", and it would automagically display the text in that variable.

@this Well, sort of :P

They were building huge VM's that contained way, way more than they needed.

Like, there were variables that could literally have been inlined into the HTML with the ASP.NET MVC view model.

But JS is like the new sliced bread!

But they serialized that view model into the JS, stuck it into an attribute on the node, then read it out of that attribute into the JS.

.....

8:32 PM

Then they would build this huge VM in the JS and use that to control the view.

I know.

I wish I didn't ask now.

I mean, we could do <div>@Model.Variable</div>

And they went through all that.

maybe it was make-work?

KO is beautiful for what it does. It doesn't want to be abused, though.

Unfortunately, we can't use it at all now.

I mean, most fields work, but what doesn't doesn't.

KO authors goofed up if they didn't make it accessible

8:33 PM

It is in a way.

oh ok thought you couldn't because of accessibility

It's really weird.

We are using this voice software to populate the fields.

It does fire events that KO should be listening for (e.g. keyup).

Except KO wasn't listening or updating.

I could see which events it was firing, and KO said it listens for those events...

Most of the bindings were working, but one wasn't.

but it just doesn't.

Yeah, and I think it has something to do with the events not coming from the browser.

Because the structure of the events was different.

I'm guessing it was essentially try/catching and recovering, or something.

8:49 PM

Well done from me @Mat'sMug and let’s see where the ride takes you. Just enjoy playing your harmonica while watching the view.

=)

@this I just had a very short look at what you merged into the extract method branch yesterday. Now, I am wondering why you introduced code module extensions pulling the module text from the module directly.

You should be able to get the same from a rewriter.

9:06 PM

@M.Doerner would rewriter have seen the compilation directives and the dead codes?

Yes, it's `GetText method does not care for channels.

i thought it would be working off only the live code

That is how we are able to rewrite without destroying the directives.

Hhmm. Good to know. I can update the extension to take a rewriter instead and read from the text

AT this point, I only care that there's any directives within selection, which would be a dangerous thing

Btw, the rewriter does not change the stream it is rewriting.

9:08 PM

because without having parsed the directives, it'd be hard to validate if selection handles the directives correctly, so extracting a method with directives could be broken

Ok, how does it work exactly? I gathered it isn't applied until you call Rewrite() method

every call to a rewriter method creates a "program instruction"

the complete "program" isn't allowed to change a given token more than once

the "program" executes when you call Rewrite()

gotcha - if I do pass 2 instructions that hits the same token - the last instruction wins?

IIRC an exception is thrown

k

so your logic is responsible for ensuring only the changes that need to be made, are made.

i.e. if you're adding a Dim foo As Bar instruction, you can't go in and make it Static foo As Bar afterwards

so you make it Static foo As Bar right away instead

9:12 PM

of course not.

Actually, the underlying antlr rewriter never touches the token stream. It just has a GetText method. Rewrite is something we added, I think.

hmm that's possible

yeah

just to clarify, though I'm not rewriting anything to the module... yet.

Rewrite() gets the rewriter text and rewrites the module

in that context, I'm just building a preview code

9:13 PM

so all you'll need is the rewriter's text

and then you can cancel and discard the changes, or okay and Rewrite()

hmm that made me realize, though - I'd have to call Rewrite() multiple times

which implies that I should hold onto the original code

Btw, the antlr rewriter supports multiple 'rewriting channels'.

track the "instructions" to be then executed by the rewriter.

I believe that can be done

We would just have to expose that.

9:14 PM

might be the ModuleRewriter isn't exposing enough for that though

yeah it's not

and I wasn't sure if we should be

I'm trying to avoid making too much changes to the underlying classes

But, @M.Doerner what exactly does multiple rewirting channels buy me?

I'm just previewing a piece of code that may never get created.

@this eh, many, many things will need to be revisited when we're able to parse a single method

that's just one of many :)

They are basically separate named rewriting programs.

Ok fine, but I really only need one?

let me rephrase

once I've built preview code version 1

Admittedly, you can do the same simply by using different rewriters.

9:17 PM

yeah,

that's what I was getting at

hmm. if I did use channels, tho, that means I can revert to a previous version....

not sure that's useful to the user, however.

it's just a sandbox at this point.

@this don't you want the preview to keep up with the user's selections and entries in the dialog?

Yes it will

right now it's being managed in the get property

(though it might need to be refactored at some point. Just not sure how yet)

agreed - makes quite a lot of work for a simple getter... but meh

#StepOneGetItWorking

this.

it's the dim statements that's giving me lot of fun. I already re-build the missing Dim statements when they are not in the selection but when they are, well things get wonky.

only if the identifier is used underneath the selection

this is rather easy to find out, just iterate the declaration's IdentifierReferences collection and if there's any with a Selection that's below your working selection, then that variable needs to be a byref input, or it's a potential output

9:29 PM

I'm already doing that

and as I said, it works well when the selection doesn't contain the Dim statement

Regarding the Dim statements, do we really want extract method to present the option to put elevate them to module level? To me, that smells like another refactorings job.

@M.Doerner I wouldn't let it do that, no

because I can grab the Declaration from the IdentifierReference

That was the example in the screenshot from yesterday.

it's when the Dim statement themselves are inside the selection, do I have a problem with the current version.

9:30 PM

the one where the original procedure moved down.

@Mat'sMug right now it's very liberal. It lets you choose from 6 possible choices. Remind that it's also dumb.

Dumb in the sense that it won't try to analyze whether it should be a parameter, a local variable or whatever.

2 of the 6 choices are Public Module Field, and Private Module Field

hmm

IIRC it was because I figured that the user knew the best -- maybe they intend to extract this method to make it reusable for other stuff in the module? Maybe one of the variable is stateful.

IDK, if reparsing after the refactoring is going to yield "variable foo can be in a smaller scope" inspection results, users will be like "yeah, thanks, why didn't you put it in there in the first place then"

the egg's on the user cos it's not default

right now, the default is always ByRef Parameter for variables

9:40 PM

fair enough

and eventually should be refined to be either Local Variable or ByRef Parameter

Hm, I just tought of somehing we probably should never exract from, a Static procedure. I also have my doubts about how to handle Static variables.

^ good point

I don't think it ever will select ~~Static Variable,~~ Private Module Field or Public Moudle Field as a default.

that's why I had to handle the Static Variable

Public Module Field probably shouldn't be in that dropdown

9:41 PM

heh. I should revise. There, fixed it

I don't like it myself either.

But what if users go but I wanna public fields!

do you wanna to tell them suck it up chumps and close issues 200x times over?

really though, the rules for what to do with what variable are very much static (as in static code analysis, not Static) - it's just a matter of encoding the mental process of what you're looking at when you decide what to do with this or that variable

Do we have a move to module scope refactoring?

not sure. I think we have a promote variable somewhere

Introduce Field is what you're looking, I believe

17

Q: You are Promoted, You are Promoted, We All are Promoted!

My latest refactoring for Rubberduck is called Introduce Field - it promotes a local variable to a private field. The three overridden Refactor() methods are the members of IRefactoring, and are used to start the refactoring sequence. The other methods are the worker methods. Overall, I am pre...

c# meta-programming rubberduck

yup

9:43 PM

@this Beat me to it.

OK, so the user can promote to a field and then extract.

yeah, that's an excellent point.

that means I can chuck out the 2 choices, narrow only to 4 choies

which would be Local Variable, Static Variable, ByRef Parameter and ByVal Parameter

Hmm. What if you made that 2,2 choices?

and avoid the pain point withe Public Module Field which I never wanted but felt obligated because of Private Module Field

Like, they choose Variable, then they choose Local/Static.

Or they choose Parameter, then ByRef/ByVal.

9:46 PM

that is possible but that means I have to change the dropdown

and that's more clicking on the users

Yeah, just a thought.

right way, it's only 4 choices, one dropdown

but here's the thing: Static Variable can break the code by changing the semantics, Local Variable can duplicate declarations, ByVal Parameter can break the code by failing to return a value to the caller

LOL, now you're seeing my pain. :)

that reminds me why I had the module fiels in there

if you're not going to infer what's what, then the only viable solution is to make everything a ByRef parameter then

9:47 PM

because if one wants to avoid breaking Static Variable, it would be safe to make it a Private Module Field

but you can already know that

ok, but that still leaves me with the selection containing Dim/Static

(right?)

Yes, I can, and I can program for that

it would be a matter of writing a validator

but... #Step1GetItWorking

@Mat'sMug Or have it be byref by default.

9:49 PM

@Hosch250 yeah but if other choices break the code and we know it, then it's not exactly great is it

Which is why I always thought this the hardest refactoring.

@this whatever you end up with, it's probably going to look better than this:

        public ExtractMethodPresenter(IActiveCodePaneEditor editor, IExtractMethodDialog view, Declaration member, QualifiedSelection selection, Declarations declarations)
        {
            _editor = editor;
            _view = view;
            _member = member;
            _selection = selection;

            _selectedCode = _editor.GetLines(selection.Selection);

            var inScopeDeclarations = declarations.Items.Where(item => item.ParentScope == member.Scope).ToList();

            var inSelection = inScopeDeclarations.SelectMany(item => item.References)

@Hosch250 it is

my gosh... memories...

10

Q: Extract Method: A refactored refactoring

This post is somewhat of a follow-up to Rubberduck's "Extract Method" refactoring implementation. A tremendous amount of things have changed since version 1.2, when this feature was first introduced - the more important thing being the advent of the Declarations API, which implies that the featu...

TBH, I don't think this refactoring was even possible until relatively recently.

And it is still only barely possible :P

Yeah. the important thing I have to do is to correctly handle the Dim/Static within the selections as well those not within the selection

so that I know that the new method won't have missing declaration for its variable or redundant declaration that collide with a parameter

@Mat'sMug IDK if this is any better but that's where most of work is done....

         public string PreviewCode
         {
             get
             {
                 _fieldsToExtract = new List<string>();
                 _parametersToExtract = new List<string>();
                 _variablesToExtract = new List<string>();

                 foreach (var parameter in Parameters)
                 {
                     switch (parameter.ParameterType)
                     {
                         case ExtractMethodParameterType.PublicModuleField:
                         case ExtractMethodParameterType.PrivateModuleField:

this section is still much a WIP, however. Who knows what will be still there and what will get chipped out.

holy getter, batman!

;-)

9:58 PM

indeed.

I do want to refactor that badly but I keep putting off because I'm still working out the logic.

as you see, it kinda of handles the Dim statements that aren't in the selection.

@Hosch250 I think I'm going to need to limit Hangman to a console app. I can't figure out how to do it in WPF and I'd rather get the logic down than be stuck. The UI part is what I don't believe I'm able to correctly do/understand yet.

make a Simon Says instead :)

github.com/retailcoder/SimonSays

38

Q: Simon Says: "Make me a pretty game"

In Memoriam Ralph H. Baer, co-inventor of the original "Simon" game, died Saturday December 6th 2014, at 92. With his passing, this friendly little challenge inadvertently became a memorial to the father of video game consoles. Rest in peace Mr. Baer, you have changed our lives forever. ...

c# wpf community-challenge async-await simon-says

I never got around to finish that game lol

it's 95% there, collecting dust on my GitHub

10:19 PM

reminds me of stack stv

aand I'm off again :)

10:37 PM

found my thing about _input and TokenStream!
_input is private: https://github.com/antlr/antlr4/blob/bdc05c87be2ad981744223df0fd745e8345baba9/runtime/CSharp/runtime/CSharp/Antlr4.Runtime/Parser.cs#L109
and TokenStream is its property getter/setter: https://github.com/antlr/antlr4/blob/bdc05c87be2ad981744223df0fd745e8345baba9/runtime/CSharp/runtime/CSharp/Antlr4.Runtime/Parser.cs#L647

11:11 PM

@NelsonVides I imagine back in 4.3 the property didn't exist. But now it does, it should work. But if that were all it was, then you shouldn't have 600 tests failing, I think.

the failing tests would be the parser + resolver tests. most tests would be inconclusive, failing a successful parse.

TTQW

I'm checking out every single character change I made, I'll tell you more details later

for now another change: github.com/antlr/antlr4/blame/… they added a parameter here, so all classes that override SyntaxError have to be changed as well

now time to sleep, tomorrow I'll check out more on this :)

11:30 PM

How do you set up VS to have RD as a repo?

I set up a new computer at home and will need to do that so I use it to work on RD.

May have figured it out.

Nope. XAML isn't recognizing elements. Hrm.

« first day (1284 days earlier) ← previous day next day → last day (1896 days later) »

Transcript for

Dec '1711

VBA Rubberducking

This chat has moved to Discord: discord.gg/MYX9RECenJ

antlr com-interop parsing rubberduck vb6 vba whiskey-tango-foxtrot

join this room
about this room

00:00

06:00

12:00

18:00

all times are UTC

site design / logo © 2024 Stack Exchange Inc; legal

mobile