{"thread":{"id":"14776","subject":"[PATCH] git-name-rev: allow --name-only in combination with --stdin","startedAt":"2008-07-31T13:20:34Z","lastAt":"2008-08-03T20:44:10Z","messageCount":8,"participants":["Pieter de Bie","Junio C Hamano","Johannes Schindelin","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"85780","messageId":"1217510434-94979-1-git-send-email-pdebie@ai.rug.nl","threadId":"14776","inReplyTo":null,"subject":"[PATCH] git-name-rev: allow --name-only in combination with --stdin","fromName":"Pieter de Bie","fromEmail":"pdebie@ai.rug.nl","sentAt":"2008-07-31T13:20:34Z","receivedAt":"2008-07-31T13:20:34Z","isPatch":true,"sender":{"key":"pdebie@ai.rug.nl","avatar":null},"body":"\nSigned-off-by: Pieter de Bie <pdebie@ai.rug.nl>\n---\n\n\tOr was there a specific reason not to allow this?\n\n Documentation/git-name-rev.txt |    3 +--\n builtin-name-rev.c             |   10 ++++++++--\n 2 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt\nindex 6e77ab1..c8a72dd 100644\n--- a/Documentation/git-name-rev.txt\n+++ b/Documentation/git-name-rev.txt\n@@ -38,8 +38,7 @@ OPTIONS\n \tInstead of printing both the SHA-1 and the name, print only\n \tthe name.  If given with --tags the usual tag prefix of\n \t\"tags/\" is also omitted from the name, matching the output\n-\tof 'git-describe' more closely.  This option\n-\tcannot be combined with --stdin.\n+\tof 'git-describe' more closely.\n \n --no-undefined::\n \tDie with error code != 0 when a reference is undefined,\ndiff --git a/builtin-name-rev.c b/builtin-name-rev.c\nindex 85612c4..0536af4 100644\n--- a/builtin-name-rev.c\n+++ b/builtin-name-rev.c\n@@ -266,8 +266,14 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tif (!name)\n \t\t\t\t\t\tcontinue;\n \n-\t\t\t\t\tfwrite(p_start, p - p_start + 1, 1, stdout);\n-\t\t\t\t\tprintf(\" (%s)\", name);\n+\t\t\t\t\tif (data.name_only) {\n+\t\t\t\t\t\tfwrite(p_start, p - p_start + 1 - 40, 1, stdout);\n+\t\t\t\t\t\tprintf(name);\n+\t\t\t\t\t}\n+\t\t\t\t\telse {\n+\t\t\t\t\t\tfwrite(p_start, p - p_start + 1, 1, stdout);\n+\t\t\t\t\t\tprintf(\" (%s)\", name);\n+\t\t\t\t\t}\n \t\t\t\t\tp_start = p + 1;\n \t\t\t\t}\n \t\t\t}\n-- \n1.6.0.rc1.163.gc85c5.dirty\n"},{"id":"85868","messageId":"7vtze5td00.fsf@gitster.siamese.dyndns.org","threadId":"14776","inReplyTo":"1217510434-94979-1-git-send-email-pdebie@ai.rug.nl","subject":"Re: [PATCH] git-name-rev: allow --name-only in combination with --stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-01T07:23:27Z","receivedAt":"2008-08-01T07:23:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pieter de Bie <pdebie@ai.rug.nl> writes:\n\n> Signed-off-by: Pieter de Bie <pdebie@ai.rug.nl>\n> ---\n>\n> \tOr was there a specific reason not to allow this?\n\nI'll let Dscho answer that one.\n\n> diff --git a/builtin-name-rev.c b/builtin-name-rev.c\n> index 85612c4..0536af4 100644\n> --- a/builtin-name-rev.c\n> +++ b/builtin-name-rev.c\n> @@ -266,8 +266,14 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n>  \t\t\t\t\tif (!name)\n>  \t\t\t\t\t\tcontinue;\n>  \n> -\t\t\t\t\tfwrite(p_start, p - p_start + 1, 1, stdout);\n> -\t\t\t\t\tprintf(\" (%s)\", name);\n> +\t\t\t\t\tif (data.name_only) {\n> +\t\t\t\t\t\tfwrite(p_start, p - p_start + 1 - 40, 1, stdout);\n> +\t\t\t\t\t\tprintf(name);\n> +\t\t\t\t\t}\n> +\t\t\t\t\telse {\n> +\t\t\t\t\t\tfwrite(p_start, p - p_start + 1, 1, stdout);\n> +\t\t\t\t\t\tprintf(\" (%s)\", name);\n> +\t\t\t\t\t}\n>  \t\t\t\t\tp_start = p + 1;\n>  \t\t\t\t}\n>  \t\t\t}\n\nIs it just me to find that this part is getting indented too deeply to be\nreadable?\n"},{"id":"85887","messageId":"alpine.DEB.1.00.0808011256330.9611@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"14776","inReplyTo":"7vtze5td00.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-name-rev: allow --name-only in combination with --stdin","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-08-01T10:57:47Z","receivedAt":"2008-08-01T10:57:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 1 Aug 2008, Junio C Hamano wrote:\n\n> Pieter de Bie <pdebie@ai.rug.nl> writes:\n> \n> > Signed-off-by: Pieter de Bie <pdebie@ai.rug.nl>\n> > ---\n> >\n> > \tOr was there a specific reason not to allow this?\n> \n> I'll let Dscho answer that one.\n\n... who let's Shawn answer that one.\n\n> > diff --git a/builtin-name-rev.c b/builtin-name-rev.c\n> > index 85612c4..0536af4 100644\n> > --- a/builtin-name-rev.c\n> > +++ b/builtin-name-rev.c\n> > @@ -266,8 +266,14 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n> >  \t\t\t\t\tif (!name)\n> >  \t\t\t\t\t\tcontinue;\n> >  \n> > -\t\t\t\t\tfwrite(p_start, p - p_start + 1, 1, stdout);\n> > -\t\t\t\t\tprintf(\" (%s)\", name);\n> > +\t\t\t\t\tif (data.name_only) {\n> > +\t\t\t\t\t\tfwrite(p_start, p - p_start + 1 - 40, 1, stdout);\n> > +\t\t\t\t\t\tprintf(name);\n> > +\t\t\t\t\t}\n> > +\t\t\t\t\telse {\n> > +\t\t\t\t\t\tfwrite(p_start, p - p_start + 1, 1, stdout);\n> > +\t\t\t\t\t\tprintf(\" (%s)\", name);\n> > +\t\t\t\t\t}\n> >  \t\t\t\t\tp_start = p + 1;\n> >  \t\t\t\t}\n> >  \t\t\t}\n> \n> Is it just me to find that this part is getting indented too deeply to be\n> readable?\n\nNo.\n\nCiao,\nDscho\n"},{"id":"85890","messageId":"1217589372-4151-1-git-send-email-pdebie@ai.rug.nl","threadId":"14776","inReplyTo":"alpine.DEB.1.00.0808011256330.9611@pacific.mpi-cbg.de.mpi-cbg.de","subject":"[PATCH] builtin-name-rev: refactor stdin handling to its own function","fromName":"Pieter de Bie","fromEmail":"pdebie@ai.rug.nl","sentAt":"2008-08-01T11:16:12Z","receivedAt":"2008-08-01T11:16:12Z","isPatch":true,"sender":{"key":"pdebie@ai.rug.nl","avatar":null},"body":"\nSigned-off-by: Pieter de Bie <pdebie@ai.rug.nl>\n---\n\n    On 1 aug 2008, at 09:23, Junio C Hamano wrote:\n    >Is it just me to find that this part is getting indented too deeply to be\n    >readable?\n\n    How about something like this then?\n\n builtin-name-rev.c |   93 ++++++++++++++++++++++++++++------------------------\n 1 files changed, 50 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin-name-rev.c b/builtin-name-rev.c\nindex 0536af4..057172d 100644\n--- a/builtin-name-rev.c\n+++ b/builtin-name-rev.c\n@@ -176,6 +176,52 @@ static char const * const name_rev_usage[] = {\n \tNULL\n };\n \n+static void handle_stdin_line(char *p_start, int name_only)\n+{\n+\tint forty = 0;\n+\tchar *p;\n+\tfor (p = p_start; *p; p++) {\n+#define ishex(x) (isdigit((x)) || ((x) >= 'a' && (x) <= 'f'))\n+\t\tif (!ishex(*p))\n+\t\t\tforty = 0;\n+\t\telse if (++forty == 40 && !ishex(*(p+1))) {\n+\t\t\tunsigned char sha1[40];\n+\t\t\tconst char *name = NULL;\n+\t\t\tchar c = *(p+1);\n+\n+\t\t\tforty = 0;\n+\n+\t\t\t*(p+1) = 0;\n+\t\t\tif (!get_sha1(p - 39, sha1)) {\n+\t\t\t\tstruct object *o =\n+\t\t\t\t\tlookup_object(sha1);\n+\t\t\t\tif (o)\n+\t\t\t\t\tname = get_rev_name(o);\n+\t\t\t}\n+\t\t\t*(p+1) = c;\n+\n+\t\t\tif (!name)\n+\t\t\t\tcontinue;\n+\n+\t\t\tif (name_only) {\n+\t\t\t\tfwrite(p_start, p - p_start + 1 - 40,\n+\t\t\t\t\t1, stdout);sssss\n+\t\t\t\tprintf(name);\n+\t\t\t}\n+\t\t\telse {\n+\t\t\t\tfwrite(p_start, p - p_start + 1, 1, stdout);\n+\t\t\t\tprintf(\" (%s)\", name);\n+\t\t\t}\n+\t\t\tp_start = p + 1;\n+\t\t}\n+\t}\n+\n+\t/* flush */\n+\tif (p != p_start)\n+\t\tfwrite(p_start, p - p_start, 1, stdout);\n+\n+}\n+\n int cmd_name_rev(int argc, const char **argv, const char *prefix)\n {\n \tstruct object_array revs = { 0, 0, NULL };\n@@ -234,53 +280,14 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \n \tif (transform_stdin) {\n \t\tchar buffer[2048];\n-\t\tchar *p, *p_start;\n+\t\tchar *p_start;\n \n \t\twhile (!feof(stdin)) {\n-\t\t\tint forty = 0;\n-\t\t\tp = fgets(buffer, sizeof(buffer), stdin);\n-\t\t\tif (!p)\n+\t\t\tp_start = fgets(buffer, sizeof(buffer), stdin);\n+\t\t\tif (!p_start)\n \t\t\t\tbreak;\n \n-\t\t\tfor (p_start = p; *p; p++) {\n-#define ishex(x) (isdigit((x)) || ((x) >= 'a' && (x) <= 'f'))\n-\t\t\t\tif (!ishex(*p))\n-\t\t\t\t\tforty = 0;\n-\t\t\t\telse if (++forty == 40 &&\n-\t\t\t\t\t\t!ishex(*(p+1))) {\n-\t\t\t\t\tunsigned char sha1[40];\n-\t\t\t\t\tconst char *name = NULL;\n-\t\t\t\t\tchar c = *(p+1);\n-\n-\t\t\t\t\tforty = 0;\n-\n-\t\t\t\t\t*(p+1) = 0;\n-\t\t\t\t\tif (!get_sha1(p - 39, sha1)) {\n-\t\t\t\t\t\tstruct object *o =\n-\t\t\t\t\t\t\tlookup_object(sha1);\n-\t\t\t\t\t\tif (o)\n-\t\t\t\t\t\t\tname = get_rev_name(o);\n-\t\t\t\t\t}\n-\t\t\t\t\t*(p+1) = c;\n-\n-\t\t\t\t\tif (!name)\n-\t\t\t\t\t\tcontinue;\n-\n-\t\t\t\t\tif (data.name_only) {\n-\t\t\t\t\t\tfwrite(p_start, p - p_start + 1 - 40, 1, stdout);\n-\t\t\t\t\t\tprintf(name);\n-\t\t\t\t\t}\n-\t\t\t\t\telse {\n-\t\t\t\t\t\tfwrite(p_start, p - p_start + 1, 1, stdout);\n-\t\t\t\t\t\tprintf(\" (%s)\", name);\n-\t\t\t\t\t}\n-\t\t\t\t\tp_start = p + 1;\n-\t\t\t\t}\n-\t\t\t}\n-\n-\t\t\t/* flush */\n-\t\t\tif (p_start != p)\n-\t\t\t\tfwrite(p_start, p - p_start, 1, stdout);\n+\t\t\thandle_stdin_line(p_start, data.name_only);\n \t\t}\n \t} else if (all) {\n \t\tint i, max;\n-- \n1.6.0.rc1.214.g5f0bd\n"},{"id":"85910","messageId":"7vtze4sgea.fsf@gitster.siamese.dyndns.org","threadId":"14776","inReplyTo":"1217589372-4151-1-git-send-email-pdebie@ai.rug.nl","subject":"Re: [PATCH] builtin-name-rev: refactor stdin handling to its own function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-01T19:07:41Z","receivedAt":"2008-08-01T19:07:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pieter de Bie <pdebie@ai.rug.nl> writes:\n\n> Signed-off-by: Pieter de Bie <pdebie@ai.rug.nl>\n> ---\n>\n>     On 1 aug 2008, at 09:23, Junio C Hamano wrote:\n>     >Is it just me to find that this part is getting indented too deeply to be\n>     >readable?\n>\n>     How about something like this then?\n\nMuch nicer, except that this refactoring should come first and then a new\nfeature.  Dropping those extra five 's' so that it would compile would be\na nice bonus as well ;-)\n\n> ...\n> +\n> +\t\t\tif (name_only) {\n> +\t\t\t\tfwrite(p_start, p - p_start + 1 - 40,\n> +\t\t\t\t\t1, stdout);sssss\n> +\t\t\t\tprintf(name);\n> +\t\t\t}\n"},{"id":"85996","messageId":"7vfxpnmgkc.fsf@gitster.siamese.dyndns.org","threadId":"14776","inReplyTo":"1217510434-94979-1-git-send-email-pdebie@ai.rug.nl","subject":"Re: [PATCH] git-name-rev: allow --name-only in combination with --stdin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-02T18:12:51Z","receivedAt":"2008-08-02T18:12:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pieter de Bie <pdebie@ai.rug.nl> writes:\n\n> \tOr was there a specific reason not to allow this?\n\nThe --name-only option to the command was added in 1.5.3 cycle, but the\noriginal focus was only to support describe.  I do not see any fundamental\nreason to disallow it --- we could even say this is a bug.\n\nI've applied the \"split single line processing to a separate function\" and\nthen this patch to 'maint' as a bugfix.\n"},{"id":"86072","messageId":"4895B641.1050500@lsrfire.ath.cx","threadId":"14776","inReplyTo":"7vfxpnmgkc.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git-name-rev: don't use printf without format","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2008-08-03T13:44:33Z","receivedAt":"2008-08-03T13:44:33Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"printf() without an explicit format string is not a good coding practise,\nunless the printed string is guaranteed to not contain percent signs.  While\nfixing this, we might as well combine the calls to fwrite() and printf().\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n builtin-name-rev.c |   12 +++++-------\n 1 files changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-name-rev.c b/builtin-name-rev.c\nindex 7055ac3..08c8aab 100644\n--- a/builtin-name-rev.c\n+++ b/builtin-name-rev.c\n@@ -189,6 +189,7 @@ static void name_rev_line(char *p, struct name_ref_data *data)\n \t\t\tunsigned char sha1[40];\n \t\t\tconst char *name = NULL;\n \t\t\tchar c = *(p+1);\n+\t\t\tint p_len = p - p_start + 1;\n \n \t\t\tforty = 0;\n \n@@ -204,13 +205,10 @@ static void name_rev_line(char *p, struct name_ref_data *data)\n \t\t\tif (!name)\n \t\t\t\tcontinue;\n \n-\t\t\tif (data->name_only) {\n-\t\t\t\tfwrite(p_start, p - p_start + 1 - 40, 1, stdout);\n-\t\t\t\tprintf(name);\n-\t\t\t} else {\n-\t\t\t\tfwrite(p_start, p - p_start + 1, 1, stdout);\n-\t\t\t\tprintf(\" (%s)\", name);\n-\t\t\t}\n+\t\t\tif (data->name_only)\n+\t\t\t\tprintf(\"%.*s%s\", p_len - 40, p_start, name);\n+\t\t\telse\n+\t\t\t\tprintf(\"%.*s (%s)\", p_len, p_start, name);\n \t\t\tp_start = p + 1;\n \t\t}\n \t}\n"},{"id":"86100","messageId":"7vvdyhu8v9.fsf@gitster.siamese.dyndns.org","threadId":"14776","inReplyTo":"4895B641.1050500@lsrfire.ath.cx","subject":"Re: [PATCH] git-name-rev: don't use printf without format","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-03T20:44:10Z","receivedAt":"2008-08-03T20:44:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n\n> printf() without an explicit format string is not a good coding practise,\n> unless the printed string is guaranteed to not contain percent signs.  While\n> fixing this, we might as well combine the calls to fwrite() and printf().\n\nGood catch; I should have caught it when I applied the \"split overlong\nfunction\" patch, but I apparently was blind.\n\nThanks.\n"}]}