[PATCH] Re: hgignore and extensions

[Chad Netzer]

On Mon, 2005-07-11 at 18:40 -0700, K Thananchayan wrote:

> When I submitted the original (buggy) patch, I
> believed that
> users who work mainly under Windows platform likly to 
> find the use of `\` for  file separator natural and
> therefore mercurial should use \ in the external
> interfaces. However, it is preferable to  use '/' for
> file separator repository is platform agnostic.

Let me just make sure we are on the same page.  Is it the case that
internally Mercurial uses and manipulates Posix paths (ie. using '/' as
a separator)?  Is it also the case that it uses, internally, relative
paths (ie. so drivenames like "C:", "D:", etc. are not an issue)?

If this is the case, one needs to be very careful using os.path, because
it will tend to put things into platform specific form, rather than
canonical Posix form.

> In case of .hgignore, I do not think that ability to
> use '\' as path separator under Windows is important,
> as users need to tinker with .hgignore infrequently.
> So, if there is consensus, we could document that
> mercurial supports only '/' as file separator in
> .hgignore and omit conversion in .hgignore patterns.

Right, a firm decision should be made as to how .hgignore, and other
platform specific path issues are to be handled.  As Posix paths, or
platform paths.

> > I may otherwise be missing something, but I know
> > that having a
> > pconvert() function and a patconvert() function,
> > just to convert
> > backslashes to slashes, is madness.
> 
> ????

Currently, there is a pconvert() function in util.py that is:

    def pconvert(path):
        return path.replace("\\", "/")

Your patch added yet another function patconvert():

+    def patconvert(pathpat):
+        return pathpat.replace("\\\\", "/")
+


So then we would have two functions which do practically the same thing,
and looking only at util.py I would have no clue as to when to use one
and not the other, and why I'd need to use them at all.

If the intent is to have a function that converts platform paths into
Posix paths, there should be one function to do it, which handles all
the cases and is well named.  At the very least, in the short-term could
you simply fix pconvert to convert both "\\\\" and "\\" into "/"
strings, rather than adding a separate function?

Here's my crude attempt:

def pconvert(path):
    path = path.replace(r"\\", "/")
    path = path.replace(r"\", "/")
    return path

Note that this may not properly handle a number of valid path cases.
Perhaps a better attempt is this:

import os.path  # Assumes this imports ntpath

def pconvert(path):
    path = os.path.normpath(path)
    path = path.replace(r"\", "/")
    return path

Although I think both of these functions may fail miserably with
absolute rather than relative paths, but I'm no expert.

Anyway, something to think about.

Chad