sancarn / stdVBA

VBA Standard Library - A Collection of libraries to form a common standard layer for modern VBA applications.
MIT License
288 stars 60 forks source link

add support for MAC to stdSettings.cls -pb #69

Closed lopperman closed 1 year ago

lopperman commented 1 year ago

Add support for mac (look for '#If Mac' to see specific changes), primarily

sancarn commented 1 year ago

Hey! PR looks good/setup correctly.

I'd like to remove the need for dependencies. Could we generally use a collection instead of a dictionary? I think this would be more ideal than including Tim H's dictionary class.

lopperman commented 1 year ago

Yes, I could switch to a Collection -- just want to confirm you know the dependency is Mac only, right? (Most Mac users will either have that Dictionary class, or be happy to get it. Unless I hear otherwise, I'll make the changes to remove the Dict dependency

sancarn commented 1 year ago

just want to confirm you know the dependency is Mac only, right?

Yes understand that, still think we should remove dependencies where possible, and in this case it is possible. Not sure what the performance detriments are of using collections, but I know Tim's class uses collections anyway so from that angle there'd be no performance loss. Also, it'll probably end up simpler and not require conditional compilation, which is a big benefit :)

lopperman commented 1 year ago

I committed the changes to remove the Dictionary Dependency. It looks like that just gets added to my first pull request. Is there anything else I need to do?

lopperman commented 1 year ago

@sancarn I didn't convert the 'PC' code to use the Collection, I'd feel more comfortable if you made those changes. At the least, still need to use precompiler directives for the GetIdentity and ignore worksheet codename for Mac (it still stops the code even with 'On Error Resume Next'). Other than that all the code could be cross platform (probably want to change the name of 'MACKeyExists' function to 'KeyExists' or something)

sancarn commented 1 year ago

I didn't convert the 'PC' code to use the Collection, I'd feel more comfortable if you made those changes.

Sure that's fine by me 👍 Let me know when you've finished and I'll merge 😄

lopperman commented 1 year ago

Should be good to go

sancarn commented 1 year ago

Congrats on your first PR 😄