Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths

From: Jeff King <peff@peff.net>
Date: Nov 29, 2012, 18:11 UTC
Message-ID: <20121129181141.GA17309@sigill.intra.peff.net>
In-Reply-To: <1354208455-21228-1-git-send-email-Matthieu.Moy@imag.fr>

On Thu, Nov 29, 2012 at 06:00:54PM +0100, Matthieu Moy wrote:

> The documentation mentionned only newlines and double quotes as

s/nn/n/

Show 13 quoted lines

> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
> index 6603a7a..35b909c 100644
> --- a/Documentation/git-fast-import.txt
> +++ b/Documentation/git-fast-import.txt
> @@ -558,8 +558,9 @@ A `<path>` string must use UNIX-style directory separators (forward
>  slash `/`), may contain any byte other than `LF`, and must not
>  start with double quote (`"`).
>  
> -If an `LF` or double quote must be encoded into `<path>` shell-style
> -quoting should be used, e.g. `"path/with\n and \" in it"`.
> +If an `LF`, backslash or double quote must be encoded into `<path>`
> +shell-style quoting should be used, and the complete name should be
> +surrounded with double quotes e.g. `"path/with\n, \\ and \" in it"`.

I think the point of the original is that you do not _need_ to quote unless you have those two characters. IOW, you can do:

  M 100644 :1 file \with \backslashes

and it will stay in the "literal to the end of line" code path because the path does not begin with a double-quote. It is only when you trigger the "shell-style quoting" code path is in effect that you must then follow the rules of that quoting (which includes escaping backslashes). So technically, your modification to the beginning of the sentence is not correct.

That being said, I think what you have written is more helpful to an end user. There is no harm in quoting when we do not have to, as fast-import implementations must know how to unquote anyway (and we over-quote in fast-export in this case). And while the example above does work (and was always designed to), it is sort of an unintuitive area that I would not be surprised to see other fast-import implementations get wrong. As a writer of a stream, it probably pays to be defensive and err on the side of quoting more.

As for the text itself, a few minor punctuation suggestions:

> If an `LF`, backslash or double quote must be encoded

                       ^
                       missing comma as list delimiter

> into `<path>` shell-style quoting should be used, and the complete

               ^
               missing comma in if/then clause

This one was in the original as well, but it makes it harder to read and is worth fixing.

> surrounded with double quotes e.g. `"path/with\n, \\ and \" in it"`.

Should the parenthetical be in parentheses (or a separate sentence)?

-Peff