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

Reply via email to