Re: [PATCH] git-p4: add "--path-encoding" option

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 31, 2015, 17:40 UTC
Message-ID: <xmqqpp23we4d.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1441035616-39128-2-git-send-email-larsxschneider@gmail.com>

larsxschneider@gmail.com writes:

Show 22 quoted lines

> From: Lars Schneider <larsxschneider@gmail.com>
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>  Documentation/git-p4.txt        |  4 ++++
>  git-p4.py                       |  6 ++++++
>  t/t9821-git-p4-path-encoding.sh | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 48 insertions(+)
>  create mode 100755 t/t9821-git-p4-path-encoding.sh
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index 82aa5d6..98b6c0f 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -252,6 +252,10 @@ Git repository:
>  	Use a client spec to find the list of interesting files in p4.
>  	See the "CLIENT SPEC" section below.
>  
> +----path-encoding <encoding>::
> +	The encoding to use when reading p4 client paths. With this option
> +	non ASCII paths are properly stored in Git. For example, the encoding 'cp1252' is often used on Windows systems.
> +

This line is overly long. Let AsciiDoc wrap it upon output and keep the source within a reasonable limit (see existing lines around the new text to see what is considered reasonable).

Do I see too many dashes before the option name, by the way, or is it my e-mail client tricking my eyes?

Show 20 quoted lines

> diff --git a/t/t9821-git-p4-path-encoding.sh b/t/t9821-git-p4-path-encoding.sh
> new file mode 100755
> index 0000000..f6bb79c
> --- /dev/null
> +++ b/t/t9821-git-p4-path-encoding.sh
> @@ -0,0 +1,38 @@
> +#!/bin/sh
> +
> +test_description='Clone repositories with non ASCII paths'
> +
> +. ./lib-git-p4.sh
> +
> +test_expect_success 'start p4d' '
> +	start_p4d
> +'
> +
> +test_expect_success 'Create a repo containing cp1251 encoded paths' '
> +	cd "$cli" &&
> +
> +	FILENAME="$(echo "a-�_o-�_u-�.txt" | iconv -f utf-8 -t cp1252)" &&

Hmm, we'd be better off not having a bare UTF-8 sequence in the source like this, especially when you already have the same thing backslash-escaped in the "expect" file below. Perhaps

	NAME="a-\303\244_o-\303\266_u-\303\274.txt" &&

	UTF8=$(printf "$NAME") &&
        CP1252=$(printf "$NAME" | iconv -t cp1252) &&
        echo "\"$UTF8\"" >expect &&

        >"$CP1252" &&
        p4 add "$CP1252" &&
        ...

or something along that line?

Show 24 quoted lines

> +	>"$FILENAME" &&
> +	p4 add "$FILENAME" &&
> +	p4 submit -d "test"
> +'
> +
> +test_expect_success 'Clone repo containing cp1251 encoded paths' '
> +	git p4 clone --destination="$git" --path-encoding=cp1252 //depot &&
> +	test_when_finished cleanup_git &&
> +	(
> +		cd "$git" &&
> +		git init . &&
> +		cat >expect <<-\EOF &&
> +		"a-\303\244_o-\303\266_u-\303\274.txt"
> +		EOF
> +		git ls-files >actual &&
> +		test_cmp expect actual
> +	)
> +'
> +
> +test_expect_success 'kill p4d' '
> +	kill_p4d
> +'
> +
> +test_done