{"thread":{"id":"33819","subject":"[PATCH] diffcore-pickaxe: make error messages more consistent","startedAt":"2013-05-14T14:16:13Z","lastAt":"2013-05-14T18:15:27Z","messageCount":3,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"217328","messageId":"1368540973-25830-1-git-send-email-artagnon@gmail.com","threadId":"33819","inReplyTo":null,"subject":"[PATCH] diffcore-pickaxe: make error messages more consistent","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-14T14:16:13Z","receivedAt":"2013-05-14T14:16:13Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, diffcore-pickaxe reports two distinct errors for the same\nuser error:\n\n    $ git log --pickaxe-regex -S'\\1'\n    fatal: invalid pickaxe regex: Invalid back reference\n\n    $ git log -G'\\1' # --pickaxe-regex is implied\n    fatal: invalid log-grep regex: Invalid back reference\n\nSince the error has nothing to do with \"log-grep\", change the -G error\nmessage to match the -S error message.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Sorry I couldn't do more.  diffcore-pickaxe.c isn't at all easy to\n hack on, because there are so few tests guarding it.\n\n diffcore-pickaxe.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\nindex 63722f8..d69a7a2 100644\n--- a/diffcore-pickaxe.c\n+++ b/diffcore-pickaxe.c\n@@ -122,7 +122,7 @@ static void diffcore_pickaxe_grep(struct diff_options *o)\n \t\tchar errbuf[1024];\n \t\tregerror(err, &regex, errbuf, 1024);\n \t\tregfree(&regex);\n-\t\tdie(\"invalid log-grep regex: %s\", errbuf);\n+\t\tdie(\"invalid pickaxe regex: %s\", errbuf);\n \t}\n \n \tpickaxe(&diff_queued_diff, o, &regex, NULL, diff_grep);\n-- \n1.8.3.rc1.61.g2cacfff\n"},{"id":"217333","messageId":"7vsj1pjvyz.fsf@alter.siamese.dyndns.org","threadId":"33819","inReplyTo":"1368540973-25830-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH] diffcore-pickaxe: make error messages more consistent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-14T17:46:12Z","receivedAt":"2013-05-14T17:46:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> Currently, diffcore-pickaxe reports two distinct errors for the same\n> user error:\n>\n>     $ git log --pickaxe-regex -S'\\1'\n>     fatal: invalid pickaxe regex: Invalid back reference\n>\n>     $ git log -G'\\1' # --pickaxe-regex is implied\n>     fatal: invalid log-grep regex: Invalid back reference\n>\n> Since the error has nothing to do with \"log-grep\", change the -G error\n> message to match the -S error message.\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  Sorry I couldn't do more.  diffcore-pickaxe.c isn't at all easy to\n>  hack on, because there are so few tests guarding it.\n>\n>  diffcore-pickaxe.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c\n> index 63722f8..d69a7a2 100644\n> --- a/diffcore-pickaxe.c\n> +++ b/diffcore-pickaxe.c\n> @@ -122,7 +122,7 @@ static void diffcore_pickaxe_grep(struct diff_options *o)\n>  \t\tchar errbuf[1024];\n>  \t\tregerror(err, &regex, errbuf, 1024);\n>  \t\tregfree(&regex);\n> -\t\tdie(\"invalid log-grep regex: %s\", errbuf);\n> +\t\tdie(\"invalid pickaxe regex: %s\", errbuf);\n>  \t}\n>  \n>  \tpickaxe(&diff_queued_diff, o, &regex, NULL, diff_grep);\n\nI am debating myself if it is truly easier to explain for users that\n\"-G\" is a different variant of pickaxe.\n\nIt happens to be implemented in the same source file as pickaxe, but\nthey do logically quite different things.  -G does not even have a\nreason to pay attention to --pickaxe-regexp (it is \"grep in the log\n-p\").\n\nI suspect that it might avoid unnecessary confusion to explain them\nas totally separate operations, and not labelling this error with\n\"pickaxe regex\".  I dunno.\n"},{"id":"217337","messageId":"CALkWK0nDU4o4HT6FXc_z=5DfZzQgexFYLWMLh4+3tevW+VXwBg@mail.gmail.com","threadId":"33819","inReplyTo":"7vsj1pjvyz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] diffcore-pickaxe: make error messages more consistent","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-14T18:15:27Z","receivedAt":"2013-05-14T18:15:27Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> I am debating myself if it is truly easier to explain for users that\n> \"-G\" is a different variant of pickaxe.\n\nHm, I think it is the correct approach because readers of diffcore are\nprobably going to look at the source: it's not exactly an end-user\nmanpage.  I've not explained it as G king versus S kind in the\ndiff-options documentation.\n\nOr are you just talking about the error message?  That's simple:\n--pickaxe-regex is what triggers this off, and we've made it clear\nthat it applies to both commands (-G implicitly; see doc).\n"}]}