[MERGE] add win32utils.get_local_appdata_location()

[Aaron Bentley]

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

Mark Hammond wrote:
>> As far as this patch goes, the tests fail on Linux but it looks ok
>> otherwise.
> 
> Oops - fixed.
> 
> Thanks,
> 
> Mark

bb:comment

There are a couple of things that *look* like they ought to be cleaned
up, but I'm interested in your feedback.

> === modified file 'bzrlib/tests/__init__.py'
> --- bzrlib/tests/__init__.py	2008-09-04 20:32:04 +0000
> +++ bzrlib/tests/__init__.py	2008-09-19 00:50:24 +0000
> @@ -1209,7 +1209,8 @@
>          new_env = {
>              'BZR_HOME': None, # Don't inherit BZR_HOME to all the tests.
>              'HOME': os.getcwd(),
> -            'APPDATA': None,  # bzr now use Win32 API and don't rely on APPDATA
> +            # bzr now uses the Win32 API and doesn't rely on APPDATA, but the
> +            # tests do check our impls match APPDATA

^^^ This comment looks pretty mysterious.  Do we still need an APPDATA:
None?  If not, do we need the comment at all?

> +    def test_appdata_matches_environment(self):
> +        # Typically the APPDATA environment variable will match
> +        # get_appdata_location
> +        # XXX - See bug 262874, which asserts the correct encoding is 'mbcs',
> +        encoding = osutils.get_user_encoding()
> +        env_val = os.environ.get("APPDATA", None)
> +        if not env_val:
> +            raise TestSkipped("No APPDATA environment variable exists")
> +        self.assertPathsEqual(win32utils.get_appdata_location(),
> +                              env_val.decode(encoding))

^^^ This seems like a missing feature, not a generic TestSkipped.


> +    def test_local_appdata_not_using_environment(self):
> +        # Test that we aren't falling back to the environment
> +        first = win32utils.get_local_appdata_location()
> +        self._captureVar("LOCALAPPDATA", None)
> +        self.assertPathsEqual(first, win32utils.get_local_appdata_location())
> +
> +    def test_local_appdata_matches_environment(self):
> +        # LOCALAPPDATA typically only exists on Vista, so we only attempt to
> +        # compare when it exists.
> +        lad = win32utils.get_local_appdata_location()
> +        env = os.environ.get("LOCALAPPDATA")
> +        if env:
> +            # XXX - See bug 262874, which asserts the correct encoding is 'mbcs'
> +            encoding = osutils.get_user_encoding()
> +            self.assertPathsEqual(lad, env.decode(encoding))

^^^ This shouldn't pass if there's no LOCALAPPDATA-- it's a missing
feature on pre-Vista Windows.

> === modified file 'bzrlib/win32utils.py'
> --- bzrlib/win32utils.py	2008-08-19 19:44:10 +0000
> +++ bzrlib/win32utils.py	2008-09-04 11:39:30 +0000
> @@ -75,6 +75,10 @@
>  except ImportError:
>      has_win32api = False
>  
> +# pulling in win32com.shell is a bit of overhead, and normally we don't need
> +# it as ctypes is preferred and common.  lazy_imports and "optional"
> +# modules don't work well, so we do our own lazy thing...
> +has_win32com_shell = None # Set to True or False once we know for sure...
>  
>  # Special Win32 API constants
>  # Handles of std streams
> @@ -84,6 +88,7 @@
>  
>  # CSIDL constants (from MSDN 2003)
>  CSIDL_APPDATA = 0x001A      # Application Data folder
> +CSIDL_LOCAL_APPDATA = 0x001c# <user name>\Local Settings\Applicaiton Data (non roaming)

^^^^ "Application" is misspelled.


> +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
> +        else:
> +            buf = ctypes.create_unicode_buffer(MAX_PATH)
> +            if SHGetSpecialFolderPath(None,buf,csidl,0):
> +                return buf.value
> +
> +    global has_win32com_shell
> +    if has_win32com_shell is None:
> +        try:
> +            from win32com.shell import shell
> +            has_win32com_shell = True
> +        except ImportError:
> +            has_win32com_shell = False

^^^ This seems like it should be split out into a function.


> +    if has_win32com_shell:
> +        # still need to bind the name locally, but this is fast.
> +        from win32com.shell import shell

^^^ if has_win32com_shell is True, won't shell already be imported?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFI1/Co0F+nu1YWqI0RAgMeAJ0U0fU7aeWmNMJL09gp0AzTn1IpZQCfRI80
hKcWWuDZWi7CJkDogFX44YM=
=d7ph
-----END PGP SIGNATURE-----