-
Notifications
You must be signed in to change notification settings - Fork 296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds CorelDRAW #964
Adds CorelDRAW #964
Conversation
Please confirm Unit Test Explorer runs test methods and displays expected test outcomes, given:
If that works, then the qualified method calls are ok, so add another test module with the same test schemes (keep the default method names, so there will be one Thanks! |
I can get Rubberduck to load, and I can use nearly all of the functionality. For tests, I can add a Test Module and I can add Test Methods using the UI. The RunMacro signature in CorelDRAW is
So I can call it from VBA using
The interop signature is
|
|
||
public override void Run(QualifiedMemberName qualifiedMemberName) | ||
{ | ||
Application.GMSManager.RunMacro(qualifiedMemberName.QualifiedModuleName.ProjectName.ToString(), qualifiedMemberName.ToString(), null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the 3rd parameter is params
(ParamArray
in VBA), then it should be omitted; passing a null
there amounts to passing a Nothing
parameter value to a parameterless procedure - not going to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the ProjectName
is already a string, the ToString
call is redundant, and calling ToString
on the qualifiedMemberName would return the qualified member name, like Proj.TestModule1.TestMethod1
, which doesn't seem like what the API is expecting. Would this work?
Application.GMSManager.RunMacro(qualifiedMemberName.QualifiedModuleName.ProjectName, qualifiedMemberName.MemberName);
Despite the misleading parameter names, The method is expecting:
|
Ok. None of the two alternatives you've tried provide the expected As for the 3rd argument, it's a Try this:
Your two snippets pass Quoting your working VBA code:
The |
I'll give it a try tonight, but VS complains when passing 2 arguments for a method that doesn't have a 2-argument overload. That said, passing the correct project name and module.method name should at least concentrate the bug tracking on the 3rd argument. I'm thinking of tenporarily creating an auxiliary helper method
|
@ThunderFrame the signature is this, right?
If that's the case, the 3rd parameter isn't an array, but a |
Random thought, is the interop method expecting an instance of the |
@ckuhn203 Yes, I was just thinking that. It's very 2008ish, but I'll try it tonight. |
I just had a thought... I can add a private helper method that accepts a Will try tonight. |
Well, it seems that a COM signature of:
gets turned into an Interop signature of:
And you'd think that the
If I create my own private method in C#, then I can call it without any 3rd argument, as you'd expect the
So it seems that despite the I've come to the conclusion that there's either some bizarre COM factors at play, or, as seems more likely, CorelDRAW's GMSManager object (short for Global Macros Manager) is conflicting with Rubberduck trying to call a Macro that GMSManager manages. That is, I don't think the RunMacro method was ever intended to be called by an automation client. |
@ThunderFrame have you tried passing in an empty object array?
|
yes, I tried that.... But I just found this, which had this gem:
So I adjusted my code to omit the RunMAcro method, and just tried to get a reference to the GMSManager, and just that is enough to crash CorelDRAW.
|
You've not done that. May be worth trying. If it works, try calling it in the classes ctor so it doesn't get called on every test. |
Well... if |
@retailcoder the whole blowing up thing seems to only apply to late binding. If it works, this ctor
Would just need to be changed to
|
Ah, yes, that would work. But passing anything into the params would still
make the call fail, because test methods are parameterless.
|
I think the InitializeVBA method is necessary when you're automating CorelDRAW as a new instance, but RubberDuck is operating on an existing instance, and an instance that must have VBE/VBA open before RubberDuck can be called. RubberDuck can insert a test module/method, for example. I will try the constructor nonetheless. The GMSManager loads a number of document-less projects by default, which I manually unload (because RubberDuck complains about the compiler directives), so there is a chance that the GMSManager object requires some of those projects to be present, and so maybe that's why my attempt to get a handle to GMSManager fails. And if the test methods require a parameter, then can't we just inform the user of that requirement? RubberDuck would still be able to call a method with a parameter. |
I think Application is never set, so for example, in the Run override method for CorelDRAW, even this line crashes CorelDRAW:
I think it is this line that is failing
Indeed, I can't get CorelDraw.Application using GetObject from Excel either:
Any ideas on how to get the Application? |
…iably for Excel and Word. Added a ctor override to HostApplication Base, that gets the host application via vbe.vbProject.vbComponent.Properties. That allows the vbe to reliably find the real host, if there is more than 1 copy of a host application open.
So.. unit testing works? Got a screenshot? :-) |
@@ -62,11 +62,11 @@ public static IHostApplication HostApplication(this VBE vbe) | |||
switch (reference.Name) | |||
{ | |||
case "Excel": | |||
return new ExcelApp(); | |||
return new ExcelApp(vbe); | |||
case "Access": | |||
return new AccessApp(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks like it's going to break the Access, PowerPoint, & Publisher hosts. Can we just call it quits on this? Would it be nice to support CorelDraw? Yeah, is it worth making changes that could break many more existing users? No.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't break anything. I've overloaded the HostApplication constructor, so ExcelApp() is valid, and so is ExcelApp(vbe). Unit testing still works under Excel and Word.
But, because #811 , ExcelApp() doesn't necessarily get you the correct instance of Excel, whereas, the overload Excel(app), does get you the right instance (and if it can't get the instance from vbe, then it gets an instance in the same way as ExcelApp().
All of the host applications could benefit from using the overloaded ctor, but I didn't want to change all of them at once, and Access, for example, won't always have a document-type VBComponent (Access forrm or report), so it doesn't get as much benefit as overloading Excel and Word ctors.
Yes, unit testing works. Here's a screenshot with 3 tests (1 test, and 2 expected error tests). One of the Expected Error tests intentionally fails (because the expected error doesn't occur. The Insert Test Module, insert Test Method and insert Test Method Expected Error menu items all work. Test Explorer finds all test methods. I can Run all the menu items for Running tests. |
@ThunderFrame can you confirm that PowerPoint isn't broken to calm @ckuhn203 down (:wink:)? If you have MS-Access to test as well, would be nice. Good job! |
Powerpoint 2013 tested. Unit Tests work |
@ckuhn203 why call quits? it works! |
I'll take the blame if anything breaks. |
@Hosch250 🙉 🙈 |
Ok. Ok. Just calling out an odd looking change. Good work @ThunderFrame! |
Based on a CreativeSuite X6 (Update 4.1) TLB/PIA.
I'm unsure about Rubberduck.VBEEditor/VBEHost/CorelDRAWApp.cs
I haven't yet run unit tests under CorelDRAW, but will do on next commit.