From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 14 Sep 2015 18:40:33 GMT
Subject: Re: [PATCH v7] git-p4: improve path encoding verbose output
Message-ID: <xmqqbnd4x2se.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1442250640-93838-2-git-send-email-larsxschneider@gmail.com>

larsxschneider@gmail.com writes:

> From: Lars Schneider <larsxschneider@gmail.com>
>
> If a path with non-ASCII characters is detected then print always the
> encoding and the encoded string in verbose mode.

Earlier if the user tells us that s/he knows what she is doing
by setting the configuration, we just followed the instruction
without complaining or notifying.  The differences in this version
are

 (1) if the path is in ASCII, the configuration is not even
     consulted, and we didn't do any path munging.

 (2) for a non-ASCII path, even if the user tells us that s/he knows
     what she is doing, we notify what we did under "--verbose"
     mode.

I think (1) is a definite improvement, but it is not immediately
obvious why (2) is an improvement.  It is clearly a good thing to
let the user know when we munged the path without being told, but
when the configuration is given, it can be argued both ways.  It may
be a good thing to reassure that the configuration is kicking in, or
it may be a needless noise to tell the user that we did what we were
told to do.

In any case, I suspectq that the call to decode-encode to munge
relPath is indented one level too deep in this patch.  You would
want to use the configured value if exists and utf8 if there is no
configuration, but in either case you would want to munge relPath
when it does not decode as ASCII, no?

> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>  git-p4.py | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index d45cf2b..da25d3f 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -2220,16 +2220,15 @@ class P4Sync(Command, P4UserMap):
>              text = regexp.sub(r'$\1$', text)
>              contents = [ text ]
>  
> -        if gitConfig("git-p4.pathEncoding"):
> -            relPath = relPath.decode(gitConfig("git-p4.pathEncoding")).encode('utf8', 'replace')
> -        elif self.verbose:
> -            try:
> -                relPath.decode('ascii')
> -            except:
> -                print (
> -                    "Path with Non-ASCII characters detected and no path encoding defined. "
> -                    "Please check the encoding: %s" % relPath
> -                )
> +        try:
> +            relPath.decode('ascii')
> +        except:
> +            encoding = 'utf8'
> +            if gitConfig('git-p4.pathEncoding'):
> +                encoding = gitConfig('git-p4.pathEncoding')
> +                relPath = relPath.decode(encoding).encode('utf8', 'replace')
> +            if self.verbose:
> +                print 'Path with non-ASCII characters detected. Used %s to encode: %s ' % (encoding, relPath)
>  
>          self.gitStream.write("M %s inline %s\n" % (git_mode, relPath))