[MERGE] add win32utils.get_local_appdata_location()
Mark Hammond
mhammond at skippinet.com.au
Wed Aug 27 04:08:49 BST 2008
Thanks for the review. A couple of followups:
> I also see you doing "os.environ[FOO].decode(user_encoding())" however,
> I thought you made the case that ENV vars are in MBCS encoding, not the
> current codepage. Certainly I would expect one or the other to be
> correct.
This is an interesting question: I made the code match the docs even though
I knew this issue was outstanding. On the other hand, if I change the docs
etc for the existing win32utils functions to say they use 'mbcs' encoding,
then the patch turns into a much wider issue.
How should I handle this in the patch? Maybe just add a qualification to
the comments that get_user_encoding() may not be correct after all? Use
'mbcs' in the tests even though that would not match the docs?
> I'm also a bit concerned, as I've seen my share of
> "C:\Progra~1\User~1\"
> values coming back out of things like environ, and that wouldn't match
> the return value of a win32 api call.
Yeah. What do you suggest? Add win32utils.get_short_path_name() and use
that to compare? OTOH, it could be argued that as this is "only" in the
test suite, we could deal with such issues as they arise so long as we are
confident the tests are checking the correct intent, that they work for most
of us, and that failures will be easily identified as being that specific
problem.
>
> +def _get_sh_special_folder_path(csidl):
> + """Call SHGetSpecialFolderPathW if available, or return None.
> +
> + Result is always unicode (or None).
> + """
> + if has_ctypes:
> + try:
> + SHGetSpecialFolderPath = \
> + ctypes.windll.shell32.SHGetSpecialFolderPathW
> + except AttributeError:
> + pass
>
> ^- I don't know if this function was even present on Win98, but I know
> Alexander has a lot of code to allow either the "A" form or the "W"
> form
> based on the settings. You may want to use a similar pattern here.
That might be worthwhile, but I don't think I changed those semantics in my
patch - I just added fall back to pywin32. I'll double check that I didn't
change that though (and note that MS and Python itself stopped supporting
win9x some time ago)
> As the tests look pretty much identical (aside from overriding
> has_ctypes), it seems like inheriting the other test, and providing a
> new setUp would be better than duplicating the tests verbatim. I
> believe
> the "preferred" method is to use load_all_tests() and provide an
> adapter. Though I still find that harder to do than just doing "class
> TestWin32Util(TestCtypes): "
Right - I actually tried to make the adaptors work, but ended up giving up,
especially as I only saw the potential for 2. A sub-class sounds like a
great idea.
Thanks,
Mark
More information about the bazaar
mailing list