* Can you please replace:
for char in WINDOWS_ILLEGAL_CHARS_MAP: self.testfile = self.testfile + char
with:
self.testfile = ''.join(WINDOWS_ILLEGAL_CHARS_MAP)
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.
* why are you doing this in abspath? path = path.replace('/', '\\') Seems unnecessary and may hide bugs in our code regarding multiplatform path management.
* Can you please replace: ILLEGAL_ CHARS_MAP:
self. testfile = self.testfile + char WINDOWS_ ILLEGAL_ CHARS_MAP)
for char in WINDOWS_
with:
self.testfile = ''.join(
* 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: replace( illegal_ char,
WINDOWS_ ILLEGAL_ CHARS_MAP[ illegal_ char])
partial = partial.
with:
for illegal_char, replacement in WINDOWS_ ILLEGAL_ CHARS_MAP. iteritems( ): replace( illegal_ char, replacement)
partial = partial.
* 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.
* why are you doing this in abspath? path = path.replace('/', '\\') Seems unnecessary and may hide bugs in our code regarding multiplatform path management.