{"thread":{"id":"36012","subject":"[PATCH] commit.c:record_author_date() use skip_prefix() instead of starts_with()","startedAt":"2014-03-01T19:48:19Z","lastAt":"2014-03-03T19:52:56Z","messageCount":4,"participants":["Tanay Abhra","Michael Haggerty","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"235760","messageId":"1393703299-7977-1-git-send-email-tanayabh@gmail.com","threadId":"36012","inReplyTo":null,"subject":"[PATCH] commit.c:record_author_date() use skip_prefix() instead of starts_with()","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-03-01T19:48:19Z","receivedAt":"2014-03-01T19:48:19Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n commit.c |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/commit.c b/commit.c\nindex 6bf4fe0..c954ecb 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -566,7 +566,7 @@ static void record_author_date(struct author_date_slab *author_date,\n \t     buf;\n \t     buf = line_end + 1) {\n \t\tline_end = strchrnul(buf, '\\n');\n-\t\tif (!starts_with(buf, \"author \")) {\n+\t\tif (!skip_prefix(buf, \"author \")) {\n \t\t\tif (!line_end[0] || line_end[1] == '\\n')\n \t\t\t\treturn; /* end of header */\n \t\t\tcontinue;\n-- \n1.7.9.5\n\nHello,\n\nThis is my patch for the GSoC microproject #10:\n\nRewrite commit.c:record_author_date() to use skip_prefix(). \nAre there other places in this file where skip_prefix() would be more \nreadable than starts_with()?\n\nSince skip_prefix() and starts_with() implement the same functionality with different\nreturn values, they can be interchanged easily.\n\nOther usage of starts_with() in the same file can be found with\n\n$ grep -n starts_with commit.c\n\n1116:\t\telse if (starts_with(line, gpg_sig_header) &&\n1196:\t\tif (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {\n\nI have a query,should I tackle a bug from the mailing lists or research about the proposal \nand present a rough draft?\n\nCheers,\nTanay Abhra.\n"},{"id":"235851","messageId":"53144450.3080006@alum.mit.edu","threadId":"36012","inReplyTo":"1393703299-7977-1-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH] commit.c:record_author_date() use skip_prefix() instead of starts_with()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-03T08:58:56Z","receivedAt":"2014-03-03T08:58:56Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The format of this email is wrong.  The non-commit-message notes should\ncome between the \"---\" line (<- note, there are three minus signs here)\nand the patch itself.\n\nOn 03/01/2014 08:48 PM, Tanay Abhra wrote:\n> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>\n> ---\n>  commit.c |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/commit.c b/commit.c\n> index 6bf4fe0..c954ecb 100644\n> --- a/commit.c\n> +++ b/commit.c\n> @@ -566,7 +566,7 @@ static void record_author_date(struct author_date_slab *author_date,\n>  \t     buf;\n>  \t     buf = line_end + 1) {\n>  \t\tline_end = strchrnul(buf, '\\n');\n> -\t\tif (!starts_with(buf, \"author \")) {\n> +\t\tif (!skip_prefix(buf, \"author \")) {\n\nIf this is the only change, there is not much point, is there?  How does\nthis help?  Perhaps there is some way to take advantage of the\ndifference between starts_with() and skip_prefix() to simplify the rest\nof the function?\n\n>  \t\t\tif (!line_end[0] || line_end[1] == '\\n')\n>  \t\t\t\treturn; /* end of header */\n>  \t\t\tcontinue;\n> -- \n> 1.7.9.5\n> \n> Hello,\n> \n> This is my patch for the GSoC microproject #10:\n> \n> Rewrite commit.c:record_author_date() to use skip_prefix(). \n> Are there other places in this file where skip_prefix() would be more \n> readable than starts_with()?\n> \n> Since skip_prefix() and starts_with() implement the same functionality with different\n> return values, they can be interchanged easily.\n> \n> Other usage of starts_with() in the same file can be found with\n> \n> $ grep -n starts_with commit.c\n> \n> 1116:\t\telse if (starts_with(line, gpg_sig_header) &&\n> 1196:\t\tif (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {\n\nThe rhetorical question that was part of this microproject was meant to\ninspire you to actually *FIX* the other spots, at least if the change\nmakes sense.\n\n> I have a query,should I tackle a bug from the mailing lists or research about the proposal \n> and present a rough draft?\n\nMy suggestion is that you follow up on this microproject until it is\nperfect before worrying too much about the next step.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"235900","messageId":"xmqqmwh7qg6v.fsf@gitster.dls.corp.google.com","threadId":"36012","inReplyTo":"53144450.3080006@alum.mit.edu","subject":"Re: [PATCH] commit.c:record_author_date() use skip_prefix() instead of starts_with()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-03T18:40:08Z","receivedAt":"2014-03-03T18:40:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n>> -\t\tif (!starts_with(buf, \"author \")) {\n>> +\t\tif (!skip_prefix(buf, \"author \")) {\n>\n> If this is the only change, there is not much point, is there?  How does\n> this help?  Perhaps there is some way to take advantage of the\n> difference between starts_with() and skip_prefix() to simplify the rest\n> of the function?\n\nI admit I lost track, but wasn't there a discussion to use\nstarts_with/ends_with when appropriate (namely, the caller is\nabsolutely not interested in what the remainder of the string is\nafter skipping the prefix), moving away from skip_prefix()?  Isn't\nthis change going in the wrong direction?\n"},{"id":"235913","messageId":"CAPig+cQrnKRDo88qq=3Ujjjwv0rn4H+eovAeAnmODUDPr00m8g@mail.gmail.com","threadId":"36012","inReplyTo":"xmqqmwh7qg6v.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] commit.c:record_author_date() use skip_prefix() instead of starts_with()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-03T19:52:56Z","receivedAt":"2014-03-03T19:52:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 3, 2014 at 1:40 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n>\n>>> -            if (!starts_with(buf, \"author \")) {\n>>> +            if (!skip_prefix(buf, \"author \")) {\n>>\n>> If this is the only change, there is not much point, is there?  How does\n>> this help?  Perhaps there is some way to take advantage of the\n>> difference between starts_with() and skip_prefix() to simplify the rest\n>> of the function?\n>\n> I admit I lost track, but wasn't there a discussion to use\n> starts_with/ends_with when appropriate (namely, the caller is\n> absolutely not interested in what the remainder of the string is\n> after skipping the prefix), moving away from skip_prefix()?  Isn't\n> this change going in the wrong direction?\n\nYes, it would be going in the wrong direction if this was all there\nwas to it, but the particular GSoC microproject [1] which inspired\nthis (incomplete) submission expects that the potential student will\ndig deeper and discover how skip_prefix() can be used to achieve\ngreater simplification in record_author_date() and in other places in\nthe same file.\n\n[1]: https://github.com/git/git.github.io/blob/master/SoC-2014-Microprojects.md\n"}]}