Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: That shell-style contradicts with what fast-import.c says, though. It claims to grok \octal and described as C-style. As Peff mentionned, my last version is better, although still a bit incomplete. My new version documents things that _must_ be escaped, but not what _can_. Yeah, I agree. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
Junio C Hamano gits...@pobox.com writes: That shell-style contradicts with what fast-import.c says, though. It claims to grok \octal and described as C-style. As Peff mentionned, my last version is better, although still a bit incomplete. My new version documents things that _must_ be escaped, but not what _can_. I'm reluctant to try being exhaustive in the .txt documentation if there is an exhaustive comment in the code. One option would be to actually turn the comment into an official specification by moving it to the .txt file, but that goes far beyond the scope of my patch. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
The documentation mentionned only newlines and double quotes as characters needing escaping, but the backslash also needs it. Also, the documentation was not clearly saying that double quotes around the file name were required (double quotes in the examples could be interpreted as part of the sentence, not part of the actual string). Signed-off-by: Matthieu Moy matthieu@imag.fr --- Documentation/git-fast-import.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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`. The value of `path` must be in canonical form. That is it must not: -- 1.8.0.319.g8abfee4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
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/ 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
Jeff King p...@peff.net writes: So technically, your modification to the beginning of the sentence is not correct. I'd say the resulting sentence is somehow incorrect, but not more than the previous one (both say if ... without really telling what the condition was). If an `LF`, backslash or double quote must be encoded ^ missing comma as list delimiter Google tells me that my version was UK-correct but not US-correct. As french, I have no opinion on the subject, I take yours ;-). How about this: A path can use the C-style string quoting (this is accepted in all cases and mandatory if the filename starts with double quote or contains `LF`). In C-style quoting, `LF`, backslash, and double quote characters must be escaped by preceding them with a backslash. Also, the complete name should be surrounded with double quotes (e.g. `path/with\n, \\ and \ in it`). This should be technically correct, and this is accepted in all cases should encourrage people to use it. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
On Thu, Nov 29, 2012 at 07:47:32PM +0100, Matthieu Moy wrote: Jeff King p...@peff.net writes: So technically, your modification to the beginning of the sentence is not correct. I'd say the resulting sentence is somehow incorrect, but not more than the previous one (both say if ... without really telling what the condition was). That's fair. There is a lot left unsaid in the original. :) If an `LF`, backslash or double quote must be encoded ^ missing comma as list delimiter Google tells me that my version was UK-correct but not US-correct. As french, I have no opinion on the subject, I take yours ;-). Thanks, I had a vague recollection that it might be regional. I probably should have looked it up myself. How about this: A path can use the C-style string quoting (this is accepted in all cases and mandatory if the filename starts with double quote or contains `LF`). In C-style quoting, `LF`, backslash, and double quote characters must be escaped by preceding them with a backslash. Also, the complete name should be surrounded with double quotes (e.g. `path/with\n, \\ and \ in it`). This should be technically correct, and this is accepted in all cases should encourrage people to use it. I think that is much better, but it reads a little more easily to me if we rearrange the second sentence. To complete my English bikeshedding, here is how I would have written the whole paragraph: A path can use C-style string quoting; this is accepted in all cases and mandatory if the filename starts with double quote or contains `LF`. In C-style quoting, the complete name should be surrounded with double quotes, and any `LF`, backslash, or double quote characters must be escaped by preceding them with a backslash (e.g., `path/with\n, \\ and \ in it`). Feel free to incorporate or ignore any of my tweaks from that version. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
Jeff King p...@peff.net writes: 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/ 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`. That shell-style contradicts with what fast-import.c says, though. It claims to grok \octal and described as C-style. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git-fast-import.txt: improve documentation for quoted paths
On Thu, Nov 29, 2012 at 11:15:56AM -0800, Junio C Hamano wrote: -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`. That shell-style contradicts with what fast-import.c says, though. It claims to grok \octal and described as C-style. Yeah, I think it was just laziness by the original author to use shell-style to mean quotes and backslash escaping without thinking too hard about which escape sequences are available. Saying C-style is more accurate (and Matthieu's more recent update does that). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
