Code review comment for lp:~mandel/ubuntuone-client/illegal_windows_chars

> * Can you please replace:
> for char in WINDOWS_ILLEGAL_CHARS_MAP:
> self.testfile = self.testfile + char
> with:
> self.testfile = ''.join(WINDOWS_ILLEGAL_CHARS_MAP)
>
> * typos: "illechal", "illegla"
>
> * can you please use os.path.splitdrive(path)
> (http://docs.python.org/library/os.path.html#os.path.splitdrive) instead of
> the custom logic in diff line 392 (and everywhere else where you split the
> volume letter)?
>
> * can you replace:
>
> for illegal_char in WINDOWS_ILLEGAL_CHARS_MAP:
> if illegal_char in path:
> return True
>
> with:
>
> any(c in WINDOWS_ILLEGAL_CHARS_MAP for c in path)
>
> * can you please replace:
>
> for illegal_char in WINDOWS_ILLEGAL_CHARS_MAP:
> partial = partial.replace(illegal_char,
> WINDOWS_ILLEGAL_CHARS_MAP[illegal_char])
>
> with:
>
> for illegal_char, replacement in WINDOWS_ILLEGAL_CHARS_MAP.iteritems():
> partial = partial.replace(illegal_char, replacement)
>
> * can you please add a logger.exception statement inside the except OSError,
> e: within set_no_rights? it will be useful to diagnose potential issues from
> log reports from our users. Same for remove_file, remove_dir, make_dir,
> open_file, rename, stat_path.
>

All the above are fixed.

> * why are you doing this in abspath? path = path.replace('/', '\\') Seems
> unnecessary and may hide bugs in our code regarding multiplatform path
> management.

I have left it because if not there is a vm test that fails... the test uses / for the paths, and while we return the correct path, the assertion fails because on windows we use \\. Since verterok is not here I have left it there with a #TODO asking if it is needed.