Re: [PATCH v3 6/8] fast-import: document C-style escapes for paths

From: Junio C Hamano <gitster@pobox.com>
To: Thalia Archibald <thalia@archibald.dev>
Cc: git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>,
	 Chris Torek <chris.torek@gmail.com>,
	 Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH v3 6/8] fast-import: document C-style escapes for paths
Date: Wed, 10 Apr 2024 11:28:00 -0700	[thread overview]
Message-ID: <xmqqjzl5t6mn.fsf@gitster.g> (raw)
In-Reply-To: <1b07ddffe000ed2ab34bd41f4f0558ae8b2dd663.1712741871.git.thalia@archibald.dev> (Thalia Archibald's message of "Wed, 10 Apr 2024 09:56:03 +0000")

Thalia Archibald <thalia@archibald.dev> writes:

> +an unquoted string. In C-style quoting, the complete filename is
> +surrounded with double quote (`"`) and certain characters must be
> +escaped by preceding them with a backslash: `LF` is written as `\n`,
> +backslash as `\\`, and double quote as `\"`. Some characters may may

"may may"?

> +optionally be written with escape sequences: `\a` for bell, `\b` for
> +backspace, `\f` for form feed, `\n` for line feed, `\r` for carriage
> +return, `\t` for horizontal tab, and `\v` for vertical tab. Any byte can
> +be written with 3-digit octal codes (e.g., `\033`).

Separating the escaped characters into two classes (mandatory LF and
BackSlash, and others) is probably a good idea to clarify the
description.  Nicely done.

>  A `<path>` must use UNIX-style directory separators (forward slash `/`)
>  and must be in canonical form. That is it must not:
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index 13f98e6688..5cde8f8d01 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -3189,8 +3189,9 @@ test_path_eol_success () {
>  	'
>  }
>  
> -test_path_eol_success 'quoted spaces'   '" hello world.c "' ' hello world.c '
> -test_path_eol_success 'unquoted spaces' ' hello world.c '   ' hello world.c '
> +test_path_eol_success 'quoted spaces'   '" hello world.c "'  ' hello world.c '
> +test_path_eol_success 'unquoted spaces' ' hello world.c '    ' hello world.c '

It is annoying to see these changes (and the same for the next
hunk).  Would it make a lot of damage to existing lines in this file
if we just say "do not align with extra spaces in between strings"?
If so, that is a good reason to keep doing things this way, but if I
recall correctly, these test_path_eol/space_success are what this
series added to the file, so if we stop such alignment from the get-go,
it may be alright.

> +test_path_eol_success 'octal escapes'   '"\150\151\056\143"' 'hi.c'
>  
>  #
>  # Valid paths before a space: filecopy (source) and filerename (source).
> @@ -3256,8 +3257,9 @@ test_path_space_success () {
>  	'
>  }
>  
> -test_path_space_success 'quoted spaces'      '" hello world.c "' ' hello world.c '
> -test_path_space_success 'no unquoted spaces' 'hello_world.c'     'hello_world.c'
> +test_path_space_success 'quoted spaces'      '" hello world.c "'  ' hello world.c '
> +test_path_space_success 'no unquoted spaces' 'hello_world.c'      'hello_world.c'
> +test_path_space_success 'octal escapes'      '"\150\151\056\143"' 'hi.c'
>  
>  #
>  # Test a single commit change with an invalid path. Run it with all occurrences