On Tue, 30 Jun 2020 15:43:29 +0200, Manuel Jacob wrote: > >>> - path = pycompat.fsencode(unicodepath) > >>> + return False > >>> + path = unicodepath.encode(fsencoding) > >> > >> I think pycompat.fsencode() is more correct since the path will be > >> later > >> tested by os.path.*() functions. On Python 2, I'm not sure. Maybe our > >> encoding.unitolocal() is okay. > > > > It was deliberate to use something that fails when it can’t be > > encoded. However, I thought about failing in case of a logic error, > > not about that the URL could contain a (UTF-8-encoded) code point that > > is not available in `fsencoding`. > > > > On Windows, Subversion will be able to handle all code points. We’re > > unnecessarily restricting ourselves by doing the rest of the function > > on bytes. Although not entirely Mercurial-idiomatic, we should maybe > > consider doing the rest of the function in Unicode. We can also bail > > out, but we have to make sure the warning mentions that it’s not the > > user’s fault. > > > > On Unix, Subversion will try later to convert the path to the locale > > encoding, which fails if the code point is not encodable. We should > > bail out in this case. > > Let me add that with "bail out", I included that a warning describing > the problem is shown. > > Since what I described is mostly about showing nicer messages, should > they be added in an amended patch or as follow-ups?
I'm not sure if I understood what you mean. I just pointed out that the fsencoding is the encoding in which Subversion will convert unicode to bytes, whereas fsencode() is the function for Python layer. And the path should be a bytes encoded in Python way. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel