{"thread":{"id":"47912","subject":"[GSoC][PATCH v2] ref-filter: Make \"--contains <id>\" less chatty if <id> is invalid","startedAt":"2018-02-23T16:26:41Z","lastAt":"2018-02-24T14:12:56Z","messageCount":4,"participants":["Paul-Sebastian Ungureanu","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"340039","messageId":"20180223162557.31477-1-ungureanupaulsebastian@gmail.com","threadId":"47912","inReplyTo":null,"subject":"[GSoC][PATCH v2] ref-filter: Make \"--contains <id>\" less chatty if <id> is invalid","fromName":"Paul-Sebastian Ungureanu","fromEmail":"ungureanupaulsebastian@gmail.com","sentAt":"2018-02-23T16:25:57Z","receivedAt":"2018-02-23T16:26:41Z","isPatch":true,"sender":{"key":"ungureanupaulsebastian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24317622?v=4"},"body":"Hello,\nI have made the changes after review. This is the updated patch\nbased on what was discussed last time [1].\n\nIn this patch, I have fixed the same issue that was also seen\nin \"git branch\" and \"git for-reach-ref\". I have also removed the\ndead code that was left and updated the patches accordingly.\n\n[1] https://public-inbox.org/git/20180219212130.4217-1-ungureanupaulsebastian@gmail.com/\n\nBest regards,\nPaul Ungureanu\n\nhttps://public-inbox.org/git/20180219212130.4217-1-ungureanupaulsebastian@gmail.com/\n\n---\n\nSome git commands which use --contains <id> print the whole\nhelp text if <id> is invalid. It should only show the error\nmessage instead.\n\nThis patch applies to \"git tag\", \"git branch\", \"git for-each-ref\".\n\nThis bug was a side effect of looking up the commit in option\nparser callback. When a error occurs in the option parser, the\nfull usage is shown. To fix this bug, the part related to\nlooking up the commit was moved outside of the option parser\nto the ref-filter module.\n\nBasically, the option parser only parses strings that represent\ncommits and the ref-filter performs the commit look-up. If an\nerror occurs during the option parsing, then it must be an invalid\nargument and the user should be informed of usage, but if a error\noccurs during ref-filtering, then it is a problem with the\nargument.\n\nSigned-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>\n---\n builtin/branch.c       | 12 +++----\n builtin/for-each-ref.c |  4 +--\n builtin/tag.c          | 16 ++++-----\n parse-options.h        |  3 +-\n ref-filter.c           | 23 +++++++++++++\n ref-filter.h           |  3 ++\n t/tcontains.sh         | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 7 files changed, 136 insertions(+), 17 deletions(-)\n create mode 100755 t/tcontains.sh\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 8dcc2ed05..43442c12e 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -596,10 +596,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT__COLOR(&branch_use_color, N_(\"use colored output\")),\n \t\tOPT_SET_INT('r', \"remotes\",     &filter.kind, N_(\"act on remote-tracking branches\"),\n \t\t\tFILTER_REFS_REMOTES),\n-\t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n-\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n-\t\tOPT_WITH(&filter.with_commit, N_(\"print only branches that contain the commit\")),\n-\t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only branches that don't contain the commit\")),\n+\t\tOPT_CONTAINS(&filter.with_commit_strs, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit_strs, N_(\"print only branches that don't contain the commit\")),\n+\t\tOPT_WITH(&filter.with_commit_strs, N_(\"print only branches that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.no_commit_strs, N_(\"print only branches that don't contain the commit\")),\n \t\tOPT__ABBREV(&filter.abbrev),\n \n \t\tOPT_GROUP(N_(\"Specific git-branch actions:\")),\n@@ -657,8 +657,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \tif (!delete && !rename && !copy && !edit_description && !new_upstream && !unset_upstream && argc == 0)\n \t\tlist = 1;\n \n-\tif (filter.with_commit || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||\n-\t    filter.no_commit)\n+\tif (filter.with_commit_strs.nr || filter.merge != REF_FILTER_MERGED_NONE || filter.points_at.nr ||\n+\t    filter.no_commit_strs.nr)\n \t\tlist = 1;\n \n \tif (!!delete + !!rename + !!copy + !!new_upstream +\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex e931be9ce..deb9a779a 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -44,8 +44,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t\t\t     parse_opt_object_name),\n \t\tOPT_MERGED(&filter, N_(\"print only refs that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only refs that are not merged\")),\n-\t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only refs which contain the commit\")),\n-\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only refs which don't contain the commit\")),\n+\t\tOPT_CONTAINS(&filter.with_commit_strs, N_(\"print only refs which contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit_strs, N_(\"print only refs which don't contain the commit\")),\n \t\tOPT_BOOL(0, \"ignore-case\", &icase, N_(\"sorting and filtering are case insensitive\")),\n \t\tOPT_END(),\n \t};\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 8885e21dd..6be7f53ae 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -396,10 +396,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \t\tOPT_GROUP(N_(\"Tag listing options\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"show tag list in columns\")),\n-\t\tOPT_CONTAINS(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n-\t\tOPT_NO_CONTAINS(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n-\t\tOPT_WITH(&filter.with_commit, N_(\"print only tags that contain the commit\")),\n-\t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n+\t\tOPT_CONTAINS(&filter.with_commit_strs, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_NO_CONTAINS(&filter.no_commit_strs, N_(\"print only tags that don't contain the commit\")),\n+\t\tOPT_WITH(&filter.with_commit_strs, N_(\"print only tags that contain the commit\")),\n+\t\tOPT_WITHOUT(&filter.no_commit_strs, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_MERGED(&filter, N_(\"print only tags that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only tags that are not merged\")),\n \t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n@@ -435,8 +435,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (!cmdmode) {\n \t\tif (argc == 0)\n \t\t\tcmdmode = 'l';\n-\t\telse if (filter.with_commit || filter.no_commit ||\n-\t\t\t filter.points_at.nr || filter.merge_commit ||\n+\t\telse if (filter.points_at.nr || filter.merge_commit ||\n+\t\t\t filter.with_commit_strs.nr || filter.no_commit_strs.nr ||\n \t\t\t filter.lines != -1)\n \t\t\tcmdmode = 'l';\n \t}\n@@ -473,9 +473,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t}\n \tif (filter.lines != -1)\n \t\tdie(_(\"-n option is only allowed in list mode\"));\n-\tif (filter.with_commit)\n+\tif (filter.with_commit_strs.nr)\n \t\tdie(_(\"--contains option is only allowed in list mode\"));\n-\tif (filter.no_commit)\n+\tif (filter.no_commit_strs.nr)\n \t\tdie(_(\"--no-contains option is only allowed in list mode\"));\n \tif (filter.points_at.nr)\n \t\tdie(_(\"--points-at option is only allowed in list mode\"));\ndiff --git a/parse-options.h b/parse-options.h\nindex af711227a..4b4734f2e 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -256,8 +256,9 @@ extern int parse_opt_passthru_argv(const struct option *, const char *, int);\n #define _OPT_CONTAINS_OR_WITH(name, variable, help, flag) \\\n \t{ OPTION_CALLBACK, 0, name, (variable), N_(\"commit\"), (help), \\\n \t  PARSE_OPT_LASTARG_DEFAULT | flag, \\\n-\t  parse_opt_commits, (intptr_t) \"HEAD\" \\\n+\t  parse_opt_string_list, (intptr_t) \"HEAD\" \\\n \t}\n+\n #define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"contains\", v, h, PARSE_OPT_NONEG)\n #define OPT_NO_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(\"no-contains\", v, h, PARSE_OPT_NONEG)\n #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)\ndiff --git a/ref-filter.c b/ref-filter.c\nindex f9e25aea7..aa282a27f 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -2000,6 +2000,25 @@ static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata)\n \tfree(to_clear);\n }\n \n+int add_str_to_commit_list(struct string_list_item *item, void *commit_list)\n+{\n+\tstruct object_id oid;\n+\tstruct commit *commit;\n+\n+\tif (get_oid(item->string, &oid)) {\n+\t\terror(_(\"malformed object name %s\"), item->string);\n+\t\texit(1);\n+\t}\n+\tcommit = lookup_commit_reference(&oid);\n+\tif (!commit) {\n+\t\terror(_(\"no such commit %s\"), item->string);\n+\t\texit(1);\n+\t}\n+\tcommit_list_insert(commit, commit_list);\n+\n+\treturn 0;\n+}\n+\n /*\n  * API for filtering a set of refs. Based on the type of refs the user\n  * has requested, we iterate through those refs and apply filters\n@@ -2012,6 +2031,10 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n \tint ret = 0;\n \tunsigned int broken = 0;\n \n+\t/* Convert string representation and add to commit list. */\n+\tfor_each_string_list(&filter->with_commit_strs, add_str_to_commit_list, &filter->with_commit);\n+\tfor_each_string_list(&filter->no_commit_strs, add_str_to_commit_list, &filter->no_commit);\n+\n \tref_cbdata.array = array;\n \tref_cbdata.filter = filter;\n \ndiff --git a/ref-filter.h b/ref-filter.h\nindex 0d98342b3..62f37760f 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -55,6 +55,9 @@ struct ref_filter {\n \tstruct commit_list *with_commit;\n \tstruct commit_list *no_commit;\n \n+\tstruct string_list with_commit_strs;\n+\tstruct string_list no_commit_strs;\n+\n \tenum {\n \t\tREF_FILTER_MERGED_NONE = 0,\n \t\tREF_FILTER_MERGED_INCLUDE,\ndiff --git a/t/tcontains.sh b/t/tcontains.sh\nnew file mode 100755\nindex 000000000..4856111ff\n--- /dev/null\n+++ b/t/tcontains.sh\n@@ -0,0 +1,92 @@\n+#!/bin/sh\n+\n+test_description='Test \"contains\" argument behavior'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup ' '\n+\tgit init . &&\n+\techo \"this is a test\" >file &&\n+\tgit add -A &&\n+\tgit commit -am \"tag test\" &&\n+\tgit tag \"v1.0\" &&\n+\tgit tag \"v1.1\"\n+'\n+\n+test_expect_success 'tag --contains <existent_tag>' '\n+\tgit tag --contains \"v1.0\" >actual &&\n+\tgrep \"v1.0\" actual &&\n+\tgrep \"v1.1\" actual\n+'\n+\n+test_expect_success 'tag --contains <inexistent_tag>' '\n+\ttest_must_fail git tag --contains \"notag\" 2>actual &&\n+\ttest_i18ngrep \"error\" actual\n+'\n+\n+test_expect_success 'tag --no-contains <existent_tag>' '\n+\tgit tag --no-contains \"v1.1\" >actual &&\n+\ttest_line_count = 0 actual\n+'\n+\n+test_expect_success 'tag --no-contains <inexistent_tag>' '\n+\ttest_must_fail git tag --no-contains \"notag\" 2>actual &&\n+\ttest_i18ngrep \"error\" actual\n+'\n+\n+test_expect_success 'tag usage error' '\n+\ttest_must_fail git tag --noopt 2>actual &&\n+\ttest_i18ngrep \"usage\" actual\n+'\n+\n+test_expect_success 'branch --contains <existent_commit>' '\n+\tgit branch --contains \"master\" >actual &&\n+\ttest_i18ngrep \"master\" actual\n+'\n+\n+test_expect_success 'branch --contains <inexistent_commit>' '\n+\ttest_must_fail git branch --no-contains \"nocommit\" 2>actual &&\n+\ttest_i18ngrep \"error\" actual\n+'\n+\n+test_expect_success 'branch --no-contains <existent_commit>' '\n+\tgit branch --no-contains \"master\" >actual &&\n+\ttest_line_count = 0 actual\n+'\n+\n+test_expect_success 'branch --no-contains <inexistent_commit>' '\n+\ttest_must_fail git branch --no-contains \"nocommit\" 2>actual &&\n+\ttest_i18ngrep \"error\" actual\n+'\n+\n+test_expect_success 'branch usage error' '\n+\ttest_must_fail git branch --noopt 2>actual &&\n+\ttest_i18ngrep \"usage\" actual\n+'\n+\n+test_expect_success 'for-each-ref --contains <existent_object>' '\n+\tgit for-each-ref --contains \"master\" >actual &&\n+\ttest_line_count = 3 actual\n+'\n+\n+test_expect_success 'for-each-ref --contains <inexistent_object>' '\n+\ttest_must_fail git for-each-ref --no-contains \"noobject\" 2>actual &&\n+\ttest_i18ngrep \"error\" actual\n+'\n+\n+test_expect_success 'for-each-ref --no-contains <existent_object>' '\n+\tgit for-each-ref --no-contains \"master\" >actual &&\n+\ttest_line_count = 0 actual\n+'\n+\n+test_expect_success 'for-each-ref --no-contains <inexistent_object>' '\n+\ttest_must_fail git for-each-ref --no-contains \"noobject\" 2>actual &&\n+\ttest_i18ngrep \"error\" actual\n+'\n+\n+test_expect_success 'for-each-ref usage error' '\n+\ttest_must_fail git for-each-ref --noopt 2>actual &&\n+\ttest_i18ngrep \"usage\" actual\n+'\n+\n+test_done\n-- \n2.16.1.195.g8f471d4ad.dirty\n\n"},{"id":"340070","messageId":"xmqqwoz3s9s2.fsf@gitster-ct.c.googlers.com","threadId":"47912","inReplyTo":"20180223162557.31477-1-ungureanupaulsebastian@gmail.com","subject":"Re: [GSoC][PATCH v2] ref-filter: Make \"--contains <id>\" less chatty if <id> is invalid","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-23T20:58:37Z","receivedAt":"2018-02-23T20:58:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> writes:\n\n> Hello,\n> I have made the changes after review. This is the updated patch\n> based on what was discussed last time [1].\n>\n> In this patch, I have fixed the same issue that was also seen\n> in \"git branch\" and \"git for-reach-ref\". I have also removed the\n> dead code that was left and updated the patches accordingly.\n>\n> [1] https://public-inbox.org/git/20180219212130.4217-1-ungureanupaulsebastian@gmail.com/\n>\n> Best regards,\n> Paul Ungureanu\n>\n> https://public-inbox.org/git/20180219212130.4217-1-ungureanupaulsebastian@gmail.com/\n\nYou do not want all of the above, upto and including the \"---\" below,\nto appear in the log message of the resulting commit.  One way to\ntell the reading end that you have such preamble in your message is\nto write \"-- >8 --\" instead of \"---\" there.\n\n> ---\n>\n> Some git commands which use --contains <id> print the whole\n> help text if <id> is invalid. It should only show the error\n> message instead.\n>\n> This patch applies to \"git tag\", \"git branch\", \"git for-each-ref\".\n>\n> This bug was a side effect of looking up the commit in option\n> parser callback. When a error occurs in the option parser, the\n> full usage is shown. To fix this bug, the part related to\n> looking up the commit was moved outside of the option parser\n> to the ref-filter module.\n>\n> Basically, the option parser only parses strings that represent\n> commits and the ref-filter performs the commit look-up. If an\n> error occurs during the option parsing, then it must be an invalid\n> argument and the user should be informed of usage, but if a error\n> occurs during ref-filtering, then it is a problem with the\n> argument.\n\nThe same problem appears for \"git branch --points-at <commit>\",\ndoesn't it?\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index f9e25aea7..aa282a27f 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -2000,6 +2000,25 @@ static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata)\n>  \tfree(to_clear);\n>  }\n>  \n> +int add_str_to_commit_list(struct string_list_item *item, void *commit_list)\n\nIf this function can be static to this file (and I suspect it is),\nplease make it so.\n\n> +{\n> +\tstruct object_id oid;\n> +\tstruct commit *commit;\n> +\n> +\tif (get_oid(item->string, &oid)) {\n> +\t\terror(_(\"malformed object name %s\"), item->string);\n> +\t\texit(1);\n> +\t}\n> +\tcommit = lookup_commit_reference(&oid);\n> +\tif (!commit) {\n> +\t\terror(_(\"no such commit %s\"), item->string);\n> +\t\texit(1);\n> +\t}\n> +\tcommit_list_insert(commit, commit_list);\n\nThe original (i.e. before this patch) does commit_list_insert() in\nthe order the commits are given on the command line.  This version\ncollects the command line arguments with string_list_append() that\npreserves the order, and feeds them to commit_list_insert() here, so\nthe resulting commit_list will have the commits in the same order\nbefore or after this patch.\n\nWhich is good.\n\n> +\treturn 0;\n> +}\n\nThe code after this patch is a strict improvement (the current code\ndo not do so either), so this is outside the scope of this patch,\nbut we may want to give this function another \"const char *\" that is\nused to report which option we got a malformed object name for.\n\n> @@ -2012,6 +2031,10 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int\n>  \tint ret = 0;\n>  \tunsigned int broken = 0;\n>  \n> +\t/* Convert string representation and add to commit list. */\n> +\tfor_each_string_list(&filter->with_commit_strs, add_str_to_commit_list, &filter->with_commit);\n> +\tfor_each_string_list(&filter->no_commit_strs, add_str_to_commit_list, &filter->no_commit);\n> +\n\nAs it does not use item->util in the callback helper, this should\nuse for_each_string_list_item() instead; then you can do\n\n\tfor_each_string_list_item(item, &filter_no_commit_strs)\n\t\tcollect_commit(&filter->no_commit, item->string);\n\nwhich allows the other helper take a simple string, instead of\nrequiring a string_list_item.\n"},{"id":"340111","messageId":"xmqq371rs23z.fsf@gitster-ct.c.googlers.com","threadId":"47912","inReplyTo":"xmqqwoz3s9s2.fsf@gitster-ct.c.googlers.com","subject":"Re: [GSoC][PATCH v2] ref-filter: Make \"--contains <id>\" less chatty if <id> is invalid","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-02-23T23:44:16Z","receivedAt":"2018-02-23T23:44:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> writes:\n>\n>> Basically, the option parser only parses strings that represent\n>> commits and the ref-filter performs the commit look-up. If an\n>> error occurs during the option parsing, then it must be an invalid\n>> argument and the user should be informed of usage, but if a error\n>> occurs during ref-filtering, then it is a problem with the\n>> argument.\n\nAfter staring the code a bit longer, I started to dislike the\napproach taken by this patch quite a lot.  Isn't the problem that we\nlet parse-options machinery to show the usage help, which is\ndesigned to be shown when the user does not know what options the\ncommand supports, even when we recognised the option perfectly fine?\n\nThat is, if we added a mechanism for a callback function given to\nOPT_CALLBACK to tell the calling parse-options machinery \"I\nrecognise the option; but the value given to the option is wrong and\nthat is why I am returning an error\", and made the caller in the\nparse-options machinery to refrain from showing the usage help,\nwould it solve the issue with minimum fuss and stop the execution at\nthe very first error we detect?\n\nStepping back even further, I wonder if any error detected in a\ncustom callback handler given to OPT_CALLBACK even wants to show the\nusage help.  I offhand do not think of any situation--- the callback\nwas called only because OPT_CALLBACK item in the options[] list\nmatched what the user gave us, so at that point we know the user\ngave us one of the valid options.  The error message from the\ncallback may say \"Hey I only take commit object name\", or it could\n(theoretically) even be \"Sorry I do not take any values\", but in any\ncase, I do not think there is a reason for a failure detected in the\ncallback should lead to the usage help.\n\nSo perhaps \"if we added a machanism...to tell...\" part in the\nprevious paragraph is not even needed, and the only thing we need to\ndo is to make the caller in parse-options that calls a custom\ncallback function given to OPT_CALLBACK to stop giving the usage\nhelp.  Wouldn't that automatically fix the \"branch --points-at\ngarbage\" issue that is not addressed by this patch, too?\n"},{"id":"340171","messageId":"1519481568.32160.3.camel@gmail.com","threadId":"47912","inReplyTo":"xmqq371rs23z.fsf@gitster-ct.c.googlers.com","subject":"Re: [GSoC][PATCH v2] ref-filter: Make \"--contains <id>\" less chatty if <id> is invalid","fromName":"Paul-Sebastian Ungureanu","fromEmail":"ungureanupaulsebastian@gmail.com","sentAt":"2018-02-24T14:12:48Z","receivedAt":"2018-02-24T14:12:56Z","isPatch":true,"sender":{"key":"ungureanupaulsebastian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24317622?v=4"},"body":"Hello,\n\nYour proposed solution makes a lot more sense. I have actually\nconsidered a solution similar to this (the third solution from [1]),\nbut found it more complicated. I did not account for the fact that\nonce a callback is called, the user is already aware of the available\noptions and the user only supplied an invalid argument value.\n\nI also have to make sure that all parsers (all callbacks and standard\nones, for integer, filename, etc.) are already printing errors\nappropriately. Otherwise, some commands may fail and the user will not\nbe aware of it because nothing will be shown (no usage is shown and no\nerrors either).\n\nI will be implementing this solution and come back with another patch.\n\nThank you for your review. I really appreciate it!\n\n[1] https://public-inbox.org/git/20160118215433.GB24136@sigill.intra.pe\nff.net/\n\nBest regards,\nPaul Ungureanu\n"}]}