TIL, the Common VBA registry key is for common settings, but the SDK allows a VBE developer to specify a host-specific registry key. That is, a host could have its own unique VBE settings, or just use the Common ones.
> There are a number of issues with Delaration.cs. This issue serves as a placeholder to discuss and revise this core class and its associated classes and enums.
Declarations are central to many features of RD, namely the resolver and inspections, so changes to these files will entail many changes in other parts of RD. Let's think very carefully about how we revise these critical files.
### Accessibility.cs The current enum has 6 members, despite VBA only having 3 levels of accessibility
> I agree except for two points: 1. I do not think we need a property to state whether that global has been used to declare the accessibility. That is only interesting for an inspection, which can simply look at the `context` or be implemented as parse tree inspection. 2. I think we should have an `IsStatic` property instead of `DeclarationIsExplicitlyStatic`.
> I agree except for two points: 1. I do not think we need a property to state whether that global has been used to declare the accessibility. That is only interesting for an inspection, which can simply look at the `context` or be implemented as parse tree inspection. 2. I think we should have an `IsStatic` property instead of `DeclarationIsExplicitlyStatic`. Whether it is explicitly static can be determined from the `context`.
In addition to the changes mentioned already, I think we shou
> Not an issue, but a request: I can click on one proc at a time and see its LOC and complexity. Is there a report for the whole project so I can rank procs for attention? 2.2.0.3398-pre
@CallumDA enums are just bunched-up constants, they're made for this. They're especially useful when you need to pass one of a limited number of meaningful numerical values to a method as a parameter: it's easier for the calling code to know what values are ok to use. MsgBox is a good example with its vbOkCancel and vbExclamation arguments =)
> For example, " parameter is passed byval but assigned a value", in the Location column I just see projectname.modulename so I get several of the same warning in the same location. It might be more explicit to show the procedure name as well.
> I can expand or collapse the general type eg Code Quality Issues. Would it be possible to add a subgroup button so that I can temporarily put away eg "parameter ... is implicitly passed by reference", work on the other ones, and then come back to the ones I grouped?
@M.Doerner they do (required for navigation)... but the 'location' column has always been more or less useless. It's using the QMN (module) rather than QMN (member), ..member was tried at one point but felt wrong half the time, e.g. VBProject.VBProject.VBProject at project level, VBProject.ModuleName.ModuleName at module level,...we need some layer of abstraction between the QMN and what's getting displayed I think.
Regarding the declaration type hierarchy update and the type library integration, I am planning to implement both if nobody wants to do it before I have time for it. However, I will be busy until October and not able to actually do it until then.
If you posted your actual real code on CR, you'd get a much more helpful & meaningful answer.
From a design standpoint, what you've got is a "coordinator" whose role is to invoke other procedures in a specific order, which isn't bad in itself.
From a pragmatic point of view, what you've got is ...
@MathieuGuindon Thanks. Guess there's some refactoring incoming. :)
I've been doing exactly that (stringing together Sub calls) and was thinking that it didn't feel exactly right. However I was not so sure how (and why exactly) to transform the subs into functions. Guess I'll give it a go now.
> I fixed one warning. Then another, and on reading I see the first has reverted to the pre-fix state. Example: Manually edit "space" to "spacetoken". Then click Fix for the "first" hint. *even if you save after the edit* But if after the edit, I click Refresh, the edit sticks. So ... after each Fix, shouldn't you automatically Refresh?
` Option Explicit
'test for Fix regression Sub testFix() Dim first As Integer Dim space As String '3 issues found. 'Suggestion: Project name i
@Inarion IMPOV, if there is no state to maintain, a function/sub is a good enough. But if you have to have a state (for example, a open file, a database table, whatever), encapsulating that state into its own class means that it's no longer part of global state and thus you know that the state itself is "scoped" which means you can manage it better than if you allowed it to pollute the global state.
@this There is a state to maintain, yes. The last thing I did was a version checking, backup generating update routine for the DB backend. So there's a couple of things to check and some values to store until it finished.
And it's almost layed out like a class. It just felt a little to big/multi-purpose to put in into a single class. And I've not yet figured out what parts would warrant being put into a class of their own. :)
> If RD is modifying the code, it should re-parse automatically. If you are modifying the code and then tell RD to make another change, then you have RD working off a stale parse tree. There's really nothing we can do about this, it's always been like this.
> That said, at this point we should be able to have a quickfix or refactoring first verify whether the module has been modified since the parse tree was built, and abort (?) or re-parse (?) before proceeding.
@this Do you, by any chance, know of some kind of library or anything similar for Access that deals with collapsible form elements? At some point in the past I was needing something like that and as I couldn't find anything useful on the net, I was implementing my own collapsibles. And now I wonder if it would be worth it to post it to CR. (As I was more than happy to find @MathieuGuindon 's list and dict classes there, among others.)
@this IMO if the module was changed, then we should change that module's parser state to Pending - doing that would change parser state to Pending, which would cause all state-dependent commands to be disabled. Inspection Results toolwindow needs to disable its fixes (and navigation?) when parser state isn't Ready
the way I've implemented it, you define any control as bounding box that will hold all controls to be collapsed and initialize the class with bounding box and an actuator control (to expand/collapse). Then you'll need to add one line to the actuator's on click event and it's all working.
IIRC, someone posted a trick using the page break as a way to divide the section, and by toggling the visible property (note that it won't show on the property sheet but it does have that property), you can achieve the same effect.
Mind, I never had a need to use it though. I maybe only used it like, once some years ago for an Access report.
I see. Could have a similar effect. And if so, that would likely have been much easier than what I did. (At least for a start. Then I expanded the class to support horizontal and vertical collapsing.)
So I guess I'll check whether I would get into trouble for posting it and if not, try to find some time to polish it up. (I'm also curious what possible improvements and issues you guys would find.)
@MathieuGuindon No, I'm thinking that while quickfix is running, there shouldn't be any parses running. Putting it in a busy state would ensure that no parse is running while quickfix is being executed because it'll be mutating the parse tree.
@MathieuGuindon Yeah I don't think it might come from user clicking, but who knows. A funny thread switch here and there, and bam! Or an errant reparse request from some piece of code that had no idea what state we are in.
TBH, the busy PR made me realize how little I know about concurrency and threading and how hard it is to architect one.
actually, that would be a nice stress-test for the cancellation mechanism. triggering a parse while a quickfix is running would likely skip that [not-yet-modified] module, and then the quickfix runs and triggers a reparse, which cancels the currently-running parse
When I run my code, I get excel not responding error at this sub as I step through the code, more specifically the write portion of this code. Can anyone help optimize this? I think because the loop is writing so many records cell by cell (50k) records its causing my excel sheet to not respond.
...
Two comments regarding the quickfixes and reparsing: first, it can do harm if you swap out the rewriter under the nose of the quickfix, in particular if you do it exactly at the moment when it gets accessed. Second, some quickfixes use refactorings that trigger a reparse themselves.
I wonder if we should make it so that taking a rewriter will require entering the busy state. That way, a lock is held as long as a rewriter is alive.
That hides the implementation details from the quickfix & anything else that needs a rewriter while guaranteeing that rewriter won't end up working off stale parse trees. Right?
I guess that implies only one rewriter can be provided at any given time. I don't know if that is a valid assumption.
@M.Doerner Why not just make the rewriter itself IDisposable?
@Vogel612 My PRs are not in a state to be merged. I do hope to get to Max's latest recommendations tonight and just perhaps I'll do it right for a change, so it may be merged then. :) The refactoring PR will be a while because of lot of tests I've broke. :\
@Vogel612 True. For Mat's PR, there's the feedback from Max to Do The Right Thing™. I'd rather have that but that's just me. For Mansellan's PR, I haven't reviewed. I'm fine w/ merging yours, however.
@this The rewriter has to be set while parsing because you need the token source. We do not create a new one each time we hand it out. So, the suspension should be active a much shorter time than the lifetime of the rewriter.
@M.Doerner Just to check - could we hide the suspension implementation by having a method like void Rewrite(TokenSource source, Action RewriteAction), so that suspension is taken as soon as we provide a source and all rewrites will be done in that delegate? Would that be short enough?
@Hosch250 Yeah I figured it was that way around. The number of "Senior Dev \ Team Lead \ Architects" I've worked with over the years... Maybe 10% of them weren't utterly useless.
> When AC is enabled, the yellow texttip that normally shows as soon as you start the argument list for a method no longer appear. To get it, one must type space to get it back only to lose it again when another AC is triggered (e.g. for quoting a string).
As an example:
``` DoIt| ```
typing `(` brings us:
``` Dolt(|) ```
without the intellisense. Typing a space, we get it:
``` DoIt( |) ```
But as soon as we type `"`, we get it AC'd but lost the intellisense again:
> No, Ctrl+Space would bring up the list of available member, which isn't the same as the yellow text tip that comes up. However, Ctrl+i would be indeed appropriate. For now, it's more important to retain the vanilla behavior, so we should do that at least OOB.
@Hosch250 Having it written in this way is acceptable because everything is private? While the example that @this had bar.DoThis ... is not because the members are already public and passing them to a void returning method achieves nothing additional beside the fact that it's in a method?
@Hosch250 I'm debating that myself. I've not been looking because of the autonomy I have for learning everything I've been able to learn from it. If I'm still limbo-status-no-work again I'll fill it with RD and C#.
Because there is not necessarily a one-to-one correspondence between keys pressed and character messages generated, the information in the high-order word of the lParam parameter is generally not useful to applications. The information in the high-order word applies only to the most recent WM_KEYDOWN message that precedes the posting of the WM_CHAR message.
If you posted your actual real code on CR, you'd get a much more helpful & meaningful answer.
From a design standpoint, what you've got is a "coordinator" whose role is to invoke other procedures in a specific order, which isn't bad in itself.
From a pragmatic point of view, what you've got is ...
VBA Rubberducking
VBA Rubberducking