Skip to content
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

Improve windows path aspect implementation #40

Merged
merged 1 commit into from
May 19, 2015
Merged

Improve windows path aspect implementation #40

merged 1 commit into from
May 19, 2015

Conversation

BurningEnlightenment
Copy link
Contributor

Hello everyone,
The path returned by app_data_path() should point to %USERPROFILE%\AppData\Local, since user specific application data should not be roamed across multiple machines.
Furthermore I simplified the implementation quite a bit, which should make it somewhat faster and easier to maintain. For the latter reason I removed the legacy WinXP code and in addition to that it isn't quite a best-practice to target platforms which aren't supported anymore.

Add KnownFolderPath() which wraps SHGetKnownFolderPath(). Refactor
home_path(), app_data_path(), config_path() to use the new wrapper
and remove the unnecessary fallback code.

- Reimplement temp_path() using a single array.
- limit path_from_me() memory allocation to 32,5KiB
  (the module path size cannnot exceed 32KiB)
- Remove Windows XP legacy code
@coveralls
Copy link

Coverage Status

Coverage increased (+0.44%) to 37.86% when pulling 14cdc44 on BurningEnlightenment:master into b0a419c on retf:master.

@retf
Copy link
Owner

retf commented May 19, 2015

Hi, thanks for your contribution!

retf added a commit that referenced this pull request May 19, 2015
Improve windows path aspect implementation
@retf retf merged commit ee42b51 into retf:master May 19, 2015
@cnd
Copy link
Contributor

cnd commented May 19, 2015

@BurningEnlightenment @retf may you help me with this change

I'm getting following errors now:

e\hdr\application\include\boost/application/detail/windows/path_impl.hpp(90): error C2039: 'SHGetKnownFolderPath' : is not a member of '`global namespace''
e\hdr\application\include\boost/application/detail/windows/path_impl.hpp(90): error C2065: 'KF_FLAG_CREATE' : undeclared identifier
e\hdr\application\include\boost/application/detail/windows/path_impl.hpp(90): error C3861: 'SHGetKnownFolderPath': identifier not found

win 8.1 / msvc 12

@retf
Copy link
Owner

retf commented May 19, 2015

This is strange, all these symbols are declared in "Shlobj.h" that is included in the path "path_impl.hpp"! @BurningEnlightenment do you can help here! tks!

@BurningEnlightenment
Copy link
Contributor Author

as I stated I dropped winXP support, so you need to define your NTDDI_VERSION as NTDDI_VISTA or higher.
I usually use this declaration at the beginning of my prefix header file:

#if /* some magic determing the operating system */
#   ifndef _WIN32_WINNT
#       include <winsdkver.h>
#       define _WIN32_WINNT _WIN32_WINNT_WIN7
#       include <SDKDDKVer.h>
#   endif
#endif

please note that you should not define NTDDI_VERSION directly. Furthermore you should ensure that this windows sdk version selection ist done before the inclusion of any windows header file.

@cnd
Copy link
Contributor

cnd commented May 19, 2015

@BurningEnlightenment I'm using win 8.1 so detector works wrong somewhere... maybe it will be better to keep XP support for this case?

@BurningEnlightenment
Copy link
Contributor Author

Your problem is that you didn't define the version of the windows SDK you want to use and the default still seems to be winXP. What we can do is to introduce some preprocesor magic which ensures a minimum windows sdk version and produces a helpful error message.

@cnd
Copy link
Contributor

cnd commented May 19, 2015

@BurningEnlightenment that will force many people to include that prefix header to each codebase but maybe it's right.

@BurningEnlightenment
Copy link
Contributor Author

so I created WinSdkVerVista.h which does the version selection and should be included before every win sdk header at least once if you didn't roll your own win sdk version selection solution.
Furthermore I added an #error to path_impl.h which describes the thing and points to the file above. @retf is this sufficient? And if it is I encourage you to pull those additions ;)

@retf
Copy link
Owner

retf commented May 19, 2015

Hi, I dont know if many users of Applicatio uses XP, anyway, send a pull! I will add it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants