On Sat, Mar 24, 2012 at 09:53:26PM +0100, Ivan Todoroski wrote:
> From c4bb55f9f27569faa368d823ca6fe4b236e37cd6 Mon Sep 17 00:00:00 2001
> From: Ivan Todoroski <grnch@xxxxxxx>
> Date: Sat, 24 Mar 2012 15:13:05 +0100
> Subject: [PATCH/RFC 1/2] fetch-pack: new option to read refs from stdin
You can drop these lines; they are redundant with the actual email
headers.
> ---
> Documentation/git-fetch-pack.txt | 9 ++++++++
> builtin/fetch-pack.c | 44 ++++++++++++++++++++++++++++++++++++--
> fetch-pack.h | 3 ++-
> 3 files changed, 53 insertions(+), 3 deletions(-)
Give more of a commit message. Why is this option useful (I know, of
course, from our previous discussion. But keep in mind the audience of
developers reading "git log" a year from now).
> +--stdin::
> + Take the list of refs from stdin, one per line. If there
> + are refs specified on the command line in addition to this
> + option, then the refs from stdin are processed after those
> + on the command line.
> + If '--stateless-rpc' is specified together with this option
> + then the list of refs must be in packet format (pkt-line)
> + with a flush packet terminating the list.
Nice. Thanks for taking the time not just to solve your problem, but to
give sane semantics in the non-stateless-rpc case.
I think there is a minor formatting bug in the above. Asciidoc will make
your two paragraphs into a single one, won't it? I think you need to do
the (horribly ugly):
--stdin::
First paragraph.
+
Second paragraph.
to appease asciidoc.
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -23,7 +23,7 @@ static struct fetch_pack_args args = {
> };
>
> static const char fetch_pack_usage[] =
> -"git fetch-pack [--all] [--quiet|-q] [--keep|-k] [--thin] [--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] [--no-progress] [-v] [<host>:]<directory> [<refs>...]";
> +"git fetch-pack [--all] [--stdin] [--quiet|-q] [--keep|-k] [--thin] [--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] [--no-progress] [-v] [<host>:]<directory> [<refs>...]";
Not a problem you introduced, but maybe it is time for us to cut down
this gigantic line (it's 180 characters even before your patch!). Even
breaking it across lines at 80 columns would help.
> @@ -972,6 +976,42 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
> if (!dest)
> usage(fetch_pack_usage);
>
> + if (args.refs_from_stdin) {
> + char ref[1000];
Ick. Is there any reason not to use a strbuf here? 1000 is probably
plenty, but we are generally moving towards removing such limits where
possible.
You'd also get to use strbuf_getline and strbuf_trim in the
newline-delimited case.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html