[MERGE] add win32utils.get_local_appdata_location()

John Arbash Meinel john at arbash-meinel.com
Thu Aug 28 21:09:57 BST 2008


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Mark Hammond wrote:
> 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.

Well, I think we already have mixed usage, as you've used 'mbcs' elsewhere. I
would be fine with making a note/bug of it, and just coming back and unifying
it everywhere.

I suppose the other problem is that on all other platforms we would assume
that ENVIRON is bzrlib.user_encoding. And I'm not 100% sure how to get around
that.

> 
> 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?

A comment is okay. We should at least add a bug report.

> 
>> 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.

I guess we can start with a comment in the code that "TODO: these *might* be
short forms, handle that case".

> 
>> +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)

I realize that, but Alexander still is using it, and as he still contributes
to bzr (if only to qbzr) I'd like to continue supporting a platform he
actively uses.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFItwYVJdeBCYSNAAMRApWRAKCVGExs4n1COsB6j+QM/c1QqBBiEwCeL+6D
akcUkpKWOHPVZkqQz7MEUgY=
=Uib8
-----END PGP SIGNATURE-----



More information about the bazaar mailing list