ppescher / resizablelib

A set of MFC classes to easily make resizable windows
Other
55 stars 27 forks source link

Small changes #6

Closed irwir closed 6 years ago

irwir commented 6 years ago

Prefer default initialization. Remove unused and predefined preprocessor macros. Add /Zc:throwingNew option.

irwir commented 6 years ago

Project file changed as a side effect of general clean-up. This is not mandatory for the other changes in this pull request, but still an improvement.

Github's online diff tool is awful, it shows the whole file was messed; Visual Studio's internal compare is better. For each build configuration only one line changed and one line added.

I assume some macro definitions remained from the days before Visual C++ 6.0. Now, there is no need to define WIN32, _WINDOWS and _DEBUG. The last one is predefined, the first two are never used anywhere. Also there are predefined alternatives such as _WIN32.

The option /Zc:throwingNew forces C++ standard behaviour for new operator, which is throw instead of returning a null pointer.

ppescher commented 6 years ago

Yes the project files have been converted from VC++6.0 first time on Visual Studio 2008. So many things have changed since then that maybe it's worth it to re-create all the projects with recent IDE versions, so I get default build options that are more up to date. I may also drop non-unicode builds, since I don't (and I couldn't anyway) support Win9x anymore.

irwir commented 6 years ago

The latest VS still does non-Unicode builds. Perhaps the library could keep non-Unicode versions while no additional efforts or code changes were required,

ppescher commented 6 years ago

I guess project defaults now have Unicode enabled, so having Debug|Release build configurations that are MBCS, and custom configurations for Unicode, might be anachronistic or just misleading. Maybe it should be the other way round, that's why I thought I should re-create the projects using the latest IDE, so I get default behavior.

irwir commented 6 years ago

Therea are ANSI builds also. My idea is to keep things compatible in any possible way while it goes for free.

ppescher commented 6 years ago

Sure, I don't want to break compatibility. Just "refreshing" the project files so they are more suitable for todays IDE's default options.

ppescher commented 6 years ago

By the way, how do you specify ANSI builds with MFC support?

ppescher commented 6 years ago

I guess I had messed up build options and mistakenly substitute MBCS for ANSI builds when I first converted the project files. Now I have a somewhat "bad" release 1.5 that I feel I should either fix or retire.

irwir commented 6 years ago

It seems you already figured out that ANSI character set is the default. It is either Not set or an empty line in Properties dialog. MBCS is obsolete now, and Unicode should be prefered instead. Possibly you could create version 1.5.1 with the latest additions and projects being fixed.

ppescher commented 6 years ago

I had a look at default project options and finally decided to merge without /Zc:throwingNew and the other project file changes. It seems that project wizard still adds those preprocessor macros.

irwir commented 6 years ago

Thanks for merging.

It could be an apprentice project wizard. :) The wizard makes a template for further changes, not a finished product.. There are many more urgent issues in VS than taking care of the wizard, which was not badly broken yet. No big deal if it adds some useless, and maybe undocumented, macros.

The compiler option in question was recommended in VC++ blog: https://blogs.msdn.microsoft.com/vcblog/2015/08/06/new-in-vs-2015-zcthrowingnew/ Earlier VC++ versions warn about unknown option, but nothing more.

For a more lightweight project you could delete the lines <CharacterSet>Not set</CharacterSet> It still would be an ANSI build.

This was not for reopening this pull request, but explanation of the reasons behind the changes.

ppescher commented 6 years ago

Thanks irwir for the explanation and the reference.

I feel like I have many things to catch up with about Windows desktop programming. Now most of my work is with embedded systems (microcontrollers) and my Windows programming is a bit rusty.

I had a look at that MSDN post and I must say I agree with many of the comments: that option should have been documented (1.5 years later nothing changed) and now added to the new IDE (VS2017). It should have also become the default for new projects, but since it is still undocumented of course that could not happen. Go figure why...

I left out those changes to project files because I want to fix the last release, and introduce only strictly required changes. I anticipate to refresh all the projects in the future, so I'll make those changes afterwards.

I would like to provide builds for ANSI (probably as a sort of "legacy" or "compatibility" option only, maybe a different project?) and Unicode, as well as x86/x64. I would gladly hear any suggestions about the proper way to do that. I don't really like the idea of ending up with "Debug Static Unicode x64" and so on... do you know of a better way?

irwir commented 6 years ago

To make sure it was not missed, Github sent 4 copies of the latest message to me.

"Debug Static Unicode x64"

Platform is taken care automatically, no need for x64; though you did not add that part already.

It is possible to leave only generic configurations and let users to adjust as required (change platform and character set); but I am not sure that would be a better option. Unfortunately, no great way to name configurations exists; the names are either long and descriptive, or short and unclear.

Use of macros such as $(Platform)\$(Configuration) might be preferred to explicit directory names such as .\Release - that is what VS does in new projects. Also you might change resources' culture from Italian 0x410 to neutral.

ppescher commented 6 years ago

Yes, that is also planned (but I think it will come very slowly now that I ended my holidays). Thank you!