{"thread":{"id":"60186","subject":"[PATCH] name-rev: use OPT_HIDDEN_BOOL for --peel-tag","startedAt":"2023-09-02T18:38:39Z","lastAt":"2023-09-07T20:21:12Z","messageCount":3,"participants":["René Scharfe","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"481330","messageId":"5a86c8f8-fcdc-fee9-8af5-aa5ecb036d2e@web.de","threadId":"60186","inReplyTo":null,"subject":"[PATCH] name-rev: use OPT_HIDDEN_BOOL for --peel-tag","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-09-02T18:38:34Z","receivedAt":"2023-09-02T18:38:39Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"adfc1857bd (describe: fix --contains when a tag is given as input,\n2013-07-18) added the option --peel-tag, defining it using a positional\nstruct option initializer and a comment indicating that it's intended to\nbe a hidden OPT_BOOL.  4741edd549 (Remove deprecated OPTION_BOOLEAN for\nparsing arguments, 2013-08-03) added the macro OPT_HIDDEN_BOOL, which\nallows to express this more succinctly.  Use it.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n builtin/name-rev.c | 8 ++------\n 1 file changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex c706fa3720..2dd1807c4e 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -582,12 +582,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"undefined\", &allow_undefined, N_(\"allow to print `undefined` names (default)\")),\n \t\tOPT_BOOL(0, \"always\",     &always,\n \t\t\t   N_(\"show abbreviated commit object as fallback\")),\n-\t\t{\n-\t\t\t/* A Hidden OPT_BOOL */\n-\t\t\tOPTION_SET_INT, 0, \"peel-tag\", &peel_tag, NULL,\n-\t\t\tN_(\"dereference tags in the input (internal use)\"),\n-\t\t\tPARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1,\n-\t\t},\n+\t\tOPT_HIDDEN_BOOL(0, \"peel-tag\", &peel_tag,\n+\t\t\t   N_(\"dereference tags in the input (internal use)\")),\n \t\tOPT_END(),\n \t};\n\n--\n2.42.0\n"},{"id":"481386","messageId":"20230905071719.GF199565@coredump.intra.peff.net","threadId":"60186","inReplyTo":"5a86c8f8-fcdc-fee9-8af5-aa5ecb036d2e@web.de","subject":"Re: [PATCH] name-rev: use OPT_HIDDEN_BOOL for --peel-tag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-09-05T07:17:19Z","receivedAt":"2023-09-05T16:00:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 02, 2023 at 08:38:34PM +0200, René Scharfe wrote:\n\n> adfc1857bd (describe: fix --contains when a tag is given as input,\n> 2013-07-18) added the option --peel-tag, defining it using a positional\n> struct option initializer and a comment indicating that it's intended to\n> be a hidden OPT_BOOL.  4741edd549 (Remove deprecated OPTION_BOOLEAN for\n> parsing arguments, 2013-08-03) added the macro OPT_HIDDEN_BOOL, which\n> allows to express this more succinctly.  Use it.\n\nYeah, this is a definite improvement.\n\nOPT_HIDDEN_BOOL() itself is a little funny to me. I guess back then we\ndid not have the \"_F\" variants, but really it is just:\n\n  OPT_BOOL_F(0, \"peel-tag\", &peel_tag,\n\t     N_(\"dereference tags in the input (internal use)\")),\n\t     PARSE_OPT_HIDDEN);\n\nwhich would remove one more special case (after all, being hidden is\northogonal to the type). But there are enough of them that maybe having\na special name for this is worth it. I dunno. But we could probably\nsimplify the definition, at least. :)\n\n-Peff\n"},{"id":"481514","messageId":"e4cec1b9-fb9e-3b12-355f-d4375f2da161@web.de","threadId":"60186","inReplyTo":"20230905071719.GF199565@coredump.intra.peff.net","subject":"Re: [PATCH] name-rev: use OPT_HIDDEN_BOOL for --peel-tag","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2023-09-07T20:20:48Z","receivedAt":"2023-09-07T20:21:12Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 05.09.23 um 09:17 schrieb Jeff King:\n> On Sat, Sep 02, 2023 at 08:38:34PM +0200, René Scharfe wrote:\n>\n> OPT_HIDDEN_BOOL() itself is a little funny to me. I guess back then we\n> did not have the \"_F\" variants, but really it is just:\n>\n>   OPT_BOOL_F(0, \"peel-tag\", &peel_tag,\n> \t     N_(\"dereference tags in the input (internal use)\")),\n> \t     PARSE_OPT_HIDDEN);\n>\n> which would remove one more special case (after all, being hidden is\n> orthogonal to the type).\n\nGood point!\n\n> But there are enough of them that maybe having\n> a special name for this is worth it. I dunno.\n\nA special case just for a flag feels wasteful.  _F is more general at a\nreasonable price of some verbosity:\n\n\tOPT_HIDDEN_BOOL(s, l, v, h),\n\tOPT_BOOL_F(s, l, v, h, PARSE_OPT_HIDDEN),\n\nHindsight..\n\n> But we could probably simplify the definition, at least. :)\n\nSure, that would be some kind of consolation price.\n\nRené\n"}]}