RELOAD!

[Hosch250/Rubberduck] 31 commits. 14598 additions. 12913 deletions.

[rubberduck-vba/Rubberduck] 42 commits. 2 closed issues. 23724 additions. 19119 deletions.

@ThunderFrame Please try not to post animated GIF's when the chat isn't moving.

Some of us (might) have that brain issue where we can go into seizures from this kind of stuff.

I've (thankfully!) never had a seizure, but animations tend to give me headaches.

@PeterMTaylor That's an interesting idea. I think I'd pass for most companies in a C# repo.

LOL.

BTW, what is that thing called?

photosensitivity

Oh, epilepsy.

watches YouTube, masters After Effects

goes back to YouTube to work out transparency

Oh I get it, you are making a new swirl or hourglass counter for the next release when waiting for inspections finish. Is that to annoy the user the beep out of them @ThunderFrame

Actually, my tone might have been rough there. It's a cool crafty idea if implemented or an Easter egg moment only?!?!?!?!?

@PeterMTaylor ^ less annoying?

@Mat'sMug Quick question:

Should I inject the inspector into the parse coordinator and run it from there?

Or should I pass the cancellation token for each parse along with the event?

I'm leaning toward the second, because what happens if we want to cancel something else later?

What if we add CI and automatically run unit tests after a parse?

Do we need to inject that into the parse coordinator then too?

At what point does it stop being the parse coordinator and start being App.cs.2?

Well the inspections need the cancel token from the parser

the idea is that we stop relying on state change to trigger the inspections

remember inspections are moving towards becoming parse tree annotations: by then they'll essentially be part of the parsing process, a bit like Roslyn's diagnostics

True.

Well, Roslyn's run as soon as any changes are parsed into their AST, AFAIK, only on the selection that changed.

It isn't like they are part of the compile process at all--they just get called.

I think they listen for node creation, or something like that.

We can't do that on a lot of ours because we check the references; Roslyn doesn't give us the references for performance reasons.

Hmm, what if, instead of having an "Inspector", each inspection registered itself?

Not sure how that would work for the IParseTreeInspection's, but the others should work well enough.

Just listen for the appropriate state changed--some run on ResolvedDeclarations (although this might slow the Code Explorer/Unit Test Explorer down some).

The rest run on Ready.

And the IParseTreeInspection's can probably run during the initial parse when a node is created.

@ThunderFrame hmm,...yes an improvement in of the duck spinning however in terms of the colour, far, far, far distracting as it's the only obvious colour that grabs the attention of the eyes most of the time on a white screen vs black screen.

In comparing the Win 10 or google spin waits they used less distracting colours. So to offer suggestions, I would think remove the yellow colour keep the outline only and the spin, another is the find another colour but I would think it defeats the name rubberduck purpose and pun intended.

So we may need to experiment and collect feedback from users as to keep it or optionally toggle this feature off in the settings panel.

I very much prefer injecting the Inspector or something new implementing an IInspectionRunner interface. (The IInspection interface actually returns a task containing the inspection results, which would not be too useful in the ParseCoordinator.)

First, triggering the inspections from there makes it much more obvious which longrunning things get triggered in a parsing run.

Second, I do not think that the concellation token of a parse should really be a concern of the stat(change). It is a thing rather internal to the parsing run.

Third, it brings the concurrently running processes more together, meking it easier to avoid concurrency bugs.

This point is also a point why I do not like triggering things on state chenge events.

It makes it completely intransparent what is running concurrently.

Moreover, I do not think that we should trigger anything not UI related on any state change but Ready. That does just add overhead from context switches because nearly all the time the parsing run already schedules work to all CPUs.

I disagree on most of these points.

First, we shouldn't care what is a long running task--by 3.0, the inspections are going to rerun every time a key is pressed.

We should only rerun the relevant ones.

Second, the cancellation token is not a concern of just the parsing run, it is a concern of everything that uses the data from that parsing run.

Third, maybe, but we don't have a concurrency bug here because it is more of a fire-and-forget thing.

We don't need to know whether it is running or not because we don't care about whether the inspections are running in the parser.

Finally, we will keep the code explorer and unit test explorer updating on ResolvedDeclarations to keep them more responsive; that was already thoroughly discussed in 2.0.

Mat was considering going back to the clumsy regex-style unit testing and getting declarations right from the VBE API until I moved those before the references resolver pass.

@PeterMTaylor Well, at work, on IE11 at 100% zoom, the GIF looks like cr@p :-(

damn you internet, pick WEBP and/or APNG and make it work already.

When you look at Roslyn's analyzers/fixes, they pass the cancellation to each analyzer/fix in the "SemanticModel" they pass around.

That SemanticModel is kind of like RubberduckParserState. It has all the information about the parse context, and is updated with each new parse (although, I think they create a new one for each parse instead of managing state--not sure).