For #45220: Tweaks support for non-ascii document names. #50
Conversation
manneohrstrom
left a comment
manneohrstrom
left a comment
There was a problem hiding this comment.
i think we need to change it so that the return data is consistent, but no need for another CR pass.
| # | ||
| # Note: This does not appear to be an issue on OS X. | ||
| if path and sys.platform.startswith("win") and not os.path.exists(path): | ||
| path = path.decode("utf-8") |
There was a problem hiding this comment.
We should add a :returns: to the docstring. But if i am reading this correctly, looks like the returns would look like:
returns a utf-8 encoded path to the document, UNLESS you are on
windows and the path does not exist or contains non-ascii characters,
in which case a unicode string is returned.
That doesn't seem like good encapsulation or consistency and seems like it can lead to subtle bugs.
As far as i understand, i think there are two ways to go, both approaches returning a consistent string from _document_path():
-
If there are several more complex path operations inside the photoshop hook logic, all requiring unicode paths on windows to function, we return all paths from
_document_path()as unicode. Ideally for all OSes, but possibly for windows only if that makes sense. -
if path operations are atomic and simple (like for example in the loader), we convert to unicode right before the operation (
os.path.exists(path.decode("utf-8"))
I am happy either way as long as we keep the return values consistent and we explain the logic in some comments.
There was a problem hiding this comment.
Makes sense. Thanks!
This attempts to resolve issues with publishing documents with paths that include non-ascii characters. The situation is different on Windows compared to OS X, so there is a small amount of OS-specific logic going on in one publish plugin, and in the adobe bridge.
In general, we return utf-8 strings, but there is now one situation on Windows where we're forced to decode to unicode before returning the document path.