{"thread":{"id":"26931","subject":"[PATCH 0/4] Miscellaneous Improvements","startedAt":"2011-02-11T17:48:17Z","lastAt":"2011-03-30T19:21:21Z","messageCount":8,"participants":["Michael Witten","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"164698","messageId":"d92be3a1-6f30-4b04-ac38-39058e5a6959-mfwitten@gmail.com","threadId":"26931","inReplyTo":"1fbceaa8-398c-44ec-8833-a03e4cca6805-mfwitten@gmail.com","subject":"[PATCH 4/4] Clean: Remove useless parameters from both get_commit_info() functions","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-02-11T17:48:17Z","receivedAt":"2011-02-11T17:48:17Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"Both `builtin/blame.c' and `reflog-walk.c' have static get_commit_info()\nfunctions:\n\n    $ echo; git grep get_commit_info 7811d9600f02e70c9f835719c71156c967a684f7 | cut -c42-\n\n    builtin/blame.c:static void get_commit_info(struct commit *commit,\n    builtin/blame.c:\tget_commit_info(suspect->commit, &ci, 1);\n    builtin/blame.c:\tget_commit_info(suspect->commit, &ci, 1);\n    builtin/blame.c:\t\t\tget_commit_info(suspect->commit, &ci, 1);\n    reflog-walk.c:static struct commit_info *get_commit_info(struct commit *commit,\n    reflog-walk.c:\t\tget_commit_info(commit, &info->reflogs, 0);\n\nEvery time one of the get_commit_info() functions is called, the\nlast parameter is invariably set to the same constant, rendering that\nparameter effectively useless.\n\nThis commit removes those last parameters and updates the function bodies\nand calls accordingly.\n\nSigned-off-by: Michael Witten <mfwitten@gmail.com>\n---\n builtin/blame.c |   14 ++++----------\n reflog-walk.c   |   18 ++++--------------\n 2 files changed, 8 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex f6b03f7..5b5fc6a 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1413,8 +1413,7 @@ static void get_ac_line(const char *inbuf, const char *what,\n }\n \n static void get_commit_info(struct commit *commit,\n-\t\t\t    struct commit_info *ret,\n-\t\t\t    int detailed)\n+\t\t\t    struct commit_info *ret)\n {\n \tint len;\n \tconst char *subject;\n@@ -1447,11 +1446,6 @@ static void get_commit_info(struct commit *commit,\n \t\t    sizeof(author_mail), author_mail,\n \t\t    &ret->author_time, &ret->author_tz);\n \n-\tif (!detailed) {\n-\t\tfree(reencoded);\n-\t\treturn;\n-\t}\n-\n \tret->committer = committer_name;\n \tret->committer_mail = committer_mail;\n \tget_ac_line(message, \"\\ncommitter \",\n@@ -1493,7 +1487,7 @@ static int emit_one_suspect_detail(struct origin *suspect)\n \t\treturn 0;\n \n \tsuspect->commit->object.flags |= METAINFO_SHOWN;\n-\tget_commit_info(suspect->commit, &ci, 1);\n+\tget_commit_info(suspect->commit, &ci);\n \tprintf(\"author %s\\n\", ci.author);\n \tprintf(\"author-mail %s\\n\", ci.author_mail);\n \tprintf(\"author-time %lu\\n\", ci.author_time);\n@@ -1664,7 +1658,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)\n \tchar hex[41];\n \tint show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);\n \n-\tget_commit_info(suspect->commit, &ci, 1);\n+\tget_commit_info(suspect->commit, &ci);\n \tstrcpy(hex, sha1_to_hex(suspect->commit->object.sha1));\n \n \tcp = nth_line(sb, ent->lno);\n@@ -1850,7 +1844,7 @@ static void find_alignment(struct scoreboard *sb, int *option)\n \t\t\tlongest_file = num;\n \t\tif (!(suspect->commit->object.flags & METAINFO_SHOWN)) {\n \t\t\tsuspect->commit->object.flags |= METAINFO_SHOWN;\n-\t\t\tget_commit_info(suspect->commit, &ci, 1);\n+\t\t\tget_commit_info(suspect->commit, &ci);\n \t\t\tif (*option & OUTPUT_SHOW_EMAIL)\n \t\t\t\tnum = utf8_strwidth(ci.author_mail);\n \t\t\telse\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex 5d81d39..3357331 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -87,22 +87,12 @@ struct commit_info_lifo {\n };\n \n static struct commit_info *get_commit_info(struct commit *commit,\n-\t\tstruct commit_info_lifo *lifo, int pop)\n+\t\tstruct commit_info_lifo *lifo)\n {\n \tint i;\n \tfor (i = 0; i < lifo->nr; i++)\n-\t\tif (lifo->items[i].commit == commit) {\n-\t\t\tstruct commit_info *result = &lifo->items[i];\n-\t\t\tif (pop) {\n-\t\t\t\tif (i + 1 < lifo->nr)\n-\t\t\t\t\tmemmove(lifo->items + i,\n-\t\t\t\t\t\tlifo->items + i + 1,\n-\t\t\t\t\t\t(lifo->nr - i) *\n-\t\t\t\t\t\tsizeof(struct commit_info));\n-\t\t\t\tlifo->nr--;\n-\t\t\t}\n-\t\t\treturn result;\n-\t\t}\n+\t\tif (lifo->items[i].commit == commit)\n+\t\t\treturn &lifo->items[i];\n \treturn NULL;\n }\n \n@@ -214,7 +204,7 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)\n {\n \tstruct commit_info *commit_info =\n-\t\tget_commit_info(commit, &info->reflogs, 0);\n+\t\tget_commit_info(commit, &info->reflogs);\n \tstruct commit_reflog *commit_reflog;\n \tstruct reflog_info *reflog;\n \n-- \n1.7.4.18.g68fe8\n"},{"id":"164696","messageId":"5bddd028-bf38-46b9-a189-bdb09038dfdd-mfwitten@gmail.com","threadId":"26931","inReplyTo":"1fbceaa8-398c-44ec-8833-a03e4cca6805-mfwitten@gmail.com","subject":"[PATCH 2/4] Clean: Remove superfluous strbuf 'docs'","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-02-15T23:12:04Z","receivedAt":"2011-02-15T23:12:04Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"Signed-off-by: Michael Witten <mfwitten@gmail.com>\n---\n strbuf.h |   37 +------------------------------------\n 1 files changed, 1 insertions(+), 36 deletions(-)\n\ndiff --git a/strbuf.h b/strbuf.h\nindex f722331..07060ce 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -1,42 +1,7 @@\n #ifndef STRBUF_H\n #define STRBUF_H\n \n-/*\n- * Strbuf's can be use in many ways: as a byte array, or to store arbitrary\n- * long, overflow safe strings.\n- *\n- * Strbufs has some invariants that are very important to keep in mind:\n- *\n- * 1. the ->buf member is always malloc-ed, hence strbuf's can be used to\n- *    build complex strings/buffers whose final size isn't easily known.\n- *\n- *    It is NOT legal to copy the ->buf pointer away.\n- *    `strbuf_detach' is the operation that detaches a buffer from its shell\n- *    while keeping the shell valid wrt its invariants.\n- *\n- * 2. the ->buf member is a byte array that has at least ->len + 1 bytes\n- *    allocated. The extra byte is used to store a '\\0', allowing the ->buf\n- *    member to be a valid C-string. Every strbuf function ensures this\n- *    invariant is preserved.\n- *\n- *    Note that it is OK to \"play\" with the buffer directly if you work it\n- *    that way:\n- *\n- *    strbuf_grow(sb, SOME_SIZE);\n- *       ... Here, the memory array starting at sb->buf, and of length\n- *       ... strbuf_avail(sb) is all yours, and you are sure that\n- *       ... strbuf_avail(sb) is at least SOME_SIZE.\n- *    strbuf_setlen(sb, sb->len + SOME_OTHER_SIZE);\n- *\n- *    Of course, SOME_OTHER_SIZE must be smaller or equal to strbuf_avail(sb).\n- *\n- *    Doing so is safe, though if it has to be done in many places, adding the\n- *    missing API to the strbuf module is the way to go.\n- *\n- *    XXX: do _not_ assume that the area that is yours is of size ->alloc - 1\n- *         even if it's true in the current implementation. Alloc is somehow a\n- *         \"private\" member that should not be messed with.\n- */\n+/* See Documentation/technical/api-strbuf.txt */\n \n #include <assert.h>\n \n-- \n1.7.4.18.g68fe8\n"},{"id":"164695","messageId":"ca8eabbf-ed1b-4b46-a7f7-4b068a2de5b7-mfwitten@gmail.com","threadId":"26931","inReplyTo":"1fbceaa8-398c-44ec-8833-a03e4cca6805-mfwitten@gmail.com","subject":"[PATCH 1/4] Typos: t/README","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-02-22T17:15:00Z","receivedAt":"2011-02-22T17:15:00Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"Signed-off-by: Michael Witten <mfwitten@gmail.com>\n---\n t/README |   11 +++++------\n 1 files changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex ccf6a53..39408b4 100644\n--- a/t/README\n+++ b/t/README\n@@ -201,7 +201,7 @@ we are testing.\n If you create files under t/ directory (i.e. here) that is not\n the top-level test script, never name the file to match the above\n pattern.  The Makefile here considers all such files as the\n-top-level test script and tries to run all of them.  A care is\n+top-level test script and tries to run all of them.  Care is\n especially needed if you are creating a common test library\n file, similar to test-lib.sh, because such a library file may\n not be suitable for standalone execution.\n@@ -248,7 +248,7 @@ This test harness library does the following things:\n    consistently when command line arguments --verbose (or -v),\n    --debug (or -d), and --immediate (or -i) is given.\n \n-Do's, don'ts & things to keep in mind\n+Dos, don'ts & things to keep in mind\n -------------------------------------\n \n Here are a few examples of things you probably should and shouldn't do\n@@ -285,9 +285,8 @@ Do:\n  - Check the test coverage for your tests. See the \"Test coverage\"\n    below.\n \n-   Don't blindly follow test coverage metrics, they're a good way to\n-   spot if you've missed something. If a new function you added\n-   doesn't have any coverage you're probably doing something wrong,\n+   Don't blindly follow test coverage metrics; if a new function you added\n+   doesn't have any coverage, then you're probably doing something wrong,\n    but having 100% coverage doesn't necessarily mean that you tested\n    everything.\n \n@@ -431,7 +430,7 @@ library for your script to use.\n  - test_tick\n \n    Make commit and tag names consistent by setting the author and\n-   committer times to defined stated.  Subsequent calls will\n+   committer times to defined state.  Subsequent calls will\n    advance the times by a fixed amount.\n \n  - test_commit <message> [<filename> [<contents>]]\n-- \n1.7.4.18.g68fe8\n"},{"id":"164697","messageId":"a59d19d0-f279-43fe-8ac6-06c4bd13c941-mfwitten@gmail.com","threadId":"26931","inReplyTo":"1fbceaa8-398c-44ec-8833-a03e4cca6805-mfwitten@gmail.com","subject":"[PATCH 3/4] Clean: Remove unnecessary `\\' (line continuation)","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-02T15:25:23Z","receivedAt":"2011-03-02T15:25:23Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"Signed-off-by: Michael Witten <mfwitten@gmail.com>\n---\n t/t8001-annotate.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t8001-annotate.sh b/t/t8001-annotate.sh\nindex 45cb60e..68ac828 100755\n--- a/t/t8001-annotate.sh\n+++ b/t/t8001-annotate.sh\n@@ -8,7 +8,7 @@ PROG='git annotate'\n \n test_expect_success \\\n     'Annotating an old revision works' \\\n-    '[ $(git annotate file master | awk \"{print \\$3}\" | grep -c \"^A$\") -eq 2 ] && \\\n+    '[ $(git annotate file master | awk \"{print \\$3}\" | grep -c \"^A$\") -eq 2 ] &&\n      [ $(git annotate file master | awk \"{print \\$3}\" | grep -c \"^B$\") -eq 2 ]'\n \n \n-- \n1.7.4.18.g68fe8\n"},{"id":"164694","messageId":"1fbceaa8-398c-44ec-8833-a03e4cca6805-mfwitten@gmail.com","threadId":"26931","inReplyTo":"d92be3a1-6f30-4b04-ac38-39058e5a6959-mfwitten@gmail.com","subject":"[PATCH 0/4] Miscellaneous Improvements","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-30T16:13:18Z","receivedAt":"2011-03-30T16:13:18Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"Michael Witten (4):\n  Typos: t/README\n  Clean: Remove superfluous strbuf 'docs'\n  Clean: Remove unnecessary `\\' (line continuation)\n  Clean: Remove useless parameters from both get_commit_info() functions\n\n builtin/blame.c     |   14 ++++----------\n reflog-walk.c       |   18 ++++--------------\n strbuf.h            |   37 +------------------------------------\n t/README            |   11 +++++------\n t/t8001-annotate.sh |    2 +-\n 5 files changed, 15 insertions(+), 67 deletions(-)\n\n-- \n1.7.4.18.g68fe8\n"},{"id":"164709","messageId":"7vd3l8o2p1.fsf@alter.siamese.dyndns.org","threadId":"26931","inReplyTo":"ca8eabbf-ed1b-4b46-a7f7-4b068a2de5b7-mfwitten@gmail.com","subject":"Re: [PATCH 1/4] Typos: t/README","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-30T18:58:34Z","receivedAt":"2011-03-30T18:58:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Witten <mfwitten@gmail.com> writes:\n\n> @@ -248,7 +248,7 @@ This test harness library does the following things:\n>     consistently when command line arguments --verbose (or -v),\n>     --debug (or -d), and --immediate (or -i) is given.\n>  \n> -Do's, don'ts & things to keep in mind\n> +Dos, don'ts & things to keep in mind\n>  -------------------------------------\n\nA quick googling seem to indicate that both forms are accepted and widely\nused (72k hits for \"Do's and Don'ts\" vs 45k hits for \"Dos and Don'ts\" in\nwww.google.com/books search), so I'd rather drop this hunk.\n\nBut all others are unambiguous improvements.  Thanks.\n"},{"id":"164710","messageId":"AANLkTi=S9LJ+Pwae_mtfNqjiPY_sfguTBrW-DPuSTWiW@mail.gmail.com","threadId":"26931","inReplyTo":"7vd3l8o2p1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] Typos: t/README","fromName":"Michael Witten","fromEmail":"mfwitten@gmail.com","sentAt":"2011-03-30T19:02:16Z","receivedAt":"2011-03-30T19:02:16Z","isPatch":true,"sender":{"key":"mfwitten@gmail.com","avatar":"https://avatars.githubusercontent.com/u/597101?v=4"},"body":"On Wed, Mar 30, 2011 at 13:58, Junio C Hamano <gitster@pobox.com> wrote:\n> A quick googling seem to indicate that both forms are accepted and widely\n> used (72k hits for \"Do's and Don'ts\" vs 45k hits for \"Dos and Don'ts\" in\n> www.google.com/books search), so I'd rather drop this hunk.\n\n*grrrrumble grumble grumble* :-)\n\nOK.\n"},{"id":"164712","messageId":"7v8vvwo1n2.fsf@alter.siamese.dyndns.org","threadId":"26931","inReplyTo":"a59d19d0-f279-43fe-8ac6-06c4bd13c941-mfwitten@gmail.com","subject":"Re: [PATCH 3/4] Clean: Remove unnecessary `\\' (line continuation)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-30T19:21:21Z","receivedAt":"2011-03-30T19:21:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Witten <mfwitten@gmail.com> writes:\n\n> Signed-off-by: Michael Witten <mfwitten@gmail.com>\n> ---\n>  t/t8001-annotate.sh |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/t/t8001-annotate.sh b/t/t8001-annotate.sh\n> index 45cb60e..68ac828 100755\n> --- a/t/t8001-annotate.sh\n> +++ b/t/t8001-annotate.sh\n> @@ -8,7 +8,7 @@ PROG='git annotate'\n>  \n>  test_expect_success \\\n>      'Annotating an old revision works' \\\n> -    '[ $(git annotate file master | awk \"{print \\$3}\" | grep -c \"^A$\") -eq 2 ] && \\\n> +    '[ $(git annotate file master | awk \"{print \\$3}\" | grep -c \"^A$\") -eq 2 ] &&\n>       [ $(git annotate file master | awk \"{print \\$3}\" | grep -c \"^B$\") -eq 2 ]'\n\nWhile this is not wrong per-se, I don't want to take too much half-way\nchurning.\n\nIf we were to properly do this, we should first rewrite it to use the more\nmodern style:\n\n\ttest_expect_success 'Annotating an old revision works' '\n\t\t... test script comes here ...\n        '\n\nand just run annotate once without having any downstream pipe, i.e.\n\n\tgit annotate file master >result &&\n\tawk \"{ print \\$3; }\" <result >authors &&\n\ttest 2 = $(grep A <authors | wc -l) &&\n\ttest 2 = $(grep B <authors | wc -l)\n\nso that we can catch breakage in \"git annotate\" itself more reliably\n(e.g. even if the command showed two lines for each author, it is a\nfailure if the command itself did not exit with status 0).\n"}]}