{"thread":{"id":"34559","subject":"[PATCH] Remove deprecated OPTION_BOOLEAN","startedAt":"2013-07-29T15:38:00Z","lastAt":"2013-07-29T18:50:35Z","messageCount":7,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"224225","messageId":"1375112281-3935-1-git-send-email-stefanbeller@googlemail.com","threadId":"34559","inReplyTo":null,"subject":"[PATCH] Remove deprecated OPTION_BOOLEAN","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-07-29T15:38:00Z","receivedAt":"2013-07-29T15:38:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi,\nthe following patch removes the OPTION_BOOLEAN from \nall commands.\n\nSo recently Junio released -rc0. Is that a reason to not\nsend out patches, which do not do bugfixes? So the following \ncommit for example doesn't fix a bug nor does it add a feature.\nIs it fine to send out such kind of commits during the -rc\ntimes or better wait for the next development cycle?\n\nStefan\n\nStefan Beller (1):\n Remove deprecated OPTION_BOOLEAN for parsing command line arguments\n\n builtin/checkout.c | 5 ++---\n builtin/clone.c | 7 +++----\n builtin/commit.c | 10 ++++------\n builtin/log.c | 4 ++--\n builtin/notes.c | 8 ++++----\n builtin/show-ref.c | 5 ++---\n parse-options.h | 5 ++---\n 7 files changed, 19 insertions(+), 25 deletions(-)\n\n-- \n1.8.4.rc0.1.g8f6a3e5\n"},{"id":"224226","messageId":"1375112281-3935-2-git-send-email-stefanbeller@googlemail.com","threadId":"34559","inReplyTo":"1375112281-3935-1-git-send-email-stefanbeller@googlemail.com","subject":"[PATCH] Remove deprecated OPTION_BOOLEAN","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-07-29T15:38:01Z","receivedAt":"2013-07-29T15:38:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"As of b04ba2bb4 OPTION_BOOLEAN was deprecated.\nThis commit removes all occurrences of OPTION_BOOLEAN.\nIn b04ba2bb4 Junio suggested to replace it with either\nOPTION_SET_INT or OPTION_COUNTUP instead. However a pattern, which\noccurred often with the OPTION_BOOLEAN was a hidden boolean parameter.\nSo I defined OPT_HIDDEN_BOOL as an additional possible parse option\nin parse-options.h to make life easy.\n\nThe OPT_HIDDEN_BOOL was used in checkout, clone, commit, show-ref.\nThe only exception, where there was need to fiddle with OPTION_SET_INT\nwas log and notes. However in these two files there is also a pattern,\nso we could think of introducing OPT_NONEG_BOOL.\n\nSigned-off-by: Stefan Beller \n---\n builtin/checkout.c | 5 ++---\n builtin/clone.c | 7 +++----\n builtin/commit.c | 10 ++++------\n builtin/log.c | 4 ++--\n builtin/notes.c | 8 ++++----\n builtin/show-ref.c | 5 ++---\n parse-options.h | 5 ++---\n 7 files changed, 19 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 7025938..646a475 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1073,9 +1073,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN('p', \"patch\", &opts.patch_mode, N_(\"select hunks interactively\")),\n \t\tOPT_BOOL(0, \"ignore-skip-worktree-bits\", &opts.ignore_skipworktree,\n \t\t\t N_(\"do not limit pathspecs to sparse entries only\")),\n-\t\t{ OPTION_BOOLEAN, 0, \"guess\", &dwim_new_local_branch, NULL,\n-\t\t N_(\"second guess 'git checkout no-such-branch'\"),\n-\t\t PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n+\t\tOPT_HIDDEN_BOOL(0, \"guess\", &dwim_new_local_branch,\n+\t\t\t\tN_(\"second guess 'git checkout no-such-branch'\")),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 430307b..e7b0b13 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -64,10 +64,9 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"force progress reporting\")),\n \tOPT_BOOLEAN('n', \"no-checkout\", &option_no_checkout,\n \t\t N_(\"don't create a checkout\")),\n-\tOPT_BOOLEAN(0, \"bare\", &option_bare, N_(\"create a bare repository\")),\n-\t{ OPTION_BOOLEAN, 0, \"naked\", &option_bare, NULL,\n-\t\tN_(\"create a bare repository\"),\n-\t\tPARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n+\tOPT_BOOL(0, \"bare\", &option_bare, N_(\"create a bare repository\")),\n+\tOPT_HIDDEN_BOOL(0, \"naked\", &option_bare,\n+\t\t\tN_(\"create a bare repository\")),\n \tOPT_BOOLEAN(0, \"mirror\", &option_mirror,\n \t\t N_(\"create a mirror repository (implies bare)\")),\n \tOPT_BOOL('l', \"local\", &option_local,\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 003bd7d..b64a083 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1448,12 +1448,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, N_(\"mode\"), N_(\"show untracked files, optional modes: all, normal, no. (Default: all)\"), PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n \t\t/* end commit contents options */\n \n-\t\t{ OPTION_BOOLEAN, 0, \"allow-empty\", &allow_empty, NULL,\n-\t\t N_(\"ok to record an empty change\"),\n-\t\t PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n-\t\t{ OPTION_BOOLEAN, 0, \"allow-empty-message\", &allow_empty_message, NULL,\n-\t\t N_(\"ok to record a change with an empty message\"),\n-\t\t PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n+\t\tOPT_HIDDEN_BOOL(0, \"allow-empty\", &allow_empty,\n+\t\t\t\tN_(\"ok to record an empty change\")),\n+\t\tOPT_HIDDEN_BOOL(0, \"allow-empty-message\", &allow_empty_message,\n+\t\t\t\tN_(\"ok to record a change with an empty message\")),\n \n \t\tOPT_END()\n \t};\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 2625f98..05e374d 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1183,9 +1183,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"don't output binary diffs\")),\n \t\tOPT_BOOLEAN(0, \"ignore-if-in-upstream\", &ignore_if_in_upstream,\n \t\t\t N_(\"don't include a patch matching a commit upstream\")),\n-\t\t{ OPTION_BOOLEAN, 'p', \"no-stat\", &use_patch_format, NULL,\n+\t\t{ OPTION_SET_INT, 'p', \"no-stat\", &use_patch_format, NULL,\n \t\t N_(\"show patch format instead of default (patch + stat)\"),\n-\t\t PARSE_OPT_NONEG | PARSE_OPT_NOARG },\n+\t\t PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1},\n \t\tOPT_GROUP(N_(\"Messaging\")),\n \t\t{ OPTION_CALLBACK, 0, \"add-header\", NULL, N_(\"header\"),\n \t\t\t N_(\"add email header\"), 0, header_callback },\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex e4100c4..8f63cb2 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -739,13 +739,13 @@ static int merge(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"resolve notes conflicts using the given strategy \"\n \t\t\t \"(manual/ours/theirs/union/cat_sort_uniq)\")),\n \t\tOPT_GROUP(N_(\"Committing unmerged notes\")),\n-\t\t{ OPTION_BOOLEAN, 0, \"commit\", &do_commit, NULL,\n+\t\t{ OPTION_SET_INT, 0, \"commit\", &do_commit, NULL,\n \t\t\tN_(\"finalize notes merge by committing unmerged notes\"),\n-\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG },\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},\n \t\tOPT_GROUP(N_(\"Aborting notes merge resolution\")),\n-\t\t{ OPTION_BOOLEAN, 0, \"abort\", &do_abort, NULL,\n+\t\t{ OPTION_SET_INT, 0, \"abort\", &do_abort, NULL,\n \t\t\tN_(\"abort notes merge\"),\n-\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG },\n+\t\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1},\n \t\tOPT_END()\n \t};\n \ndiff --git a/builtin/show-ref.c b/builtin/show-ref.c\nindex 87806ad..18680bb 100644\n--- a/builtin/show-ref.c\n+++ b/builtin/show-ref.c\n@@ -169,9 +169,8 @@ static const struct option show_ref_options[] = {\n \tOPT_BOOLEAN(0, \"heads\", &heads_only, N_(\"only show heads (can be combined with tags)\")),\n \tOPT_BOOLEAN(0, \"verify\", &verify, N_(\"stricter reference checking, \"\n \t\t \"requires exact ref path\")),\n-\t{ OPTION_BOOLEAN, 'h', NULL, &show_head, NULL,\n-\t N_(\"show the HEAD reference, even if it would be filtered out\"),\n-\t PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },\n+\tOPT_HIDDEN_BOOL('h', NULL, &show_head,\n+\t\t\tN_(\"show the HEAD reference, even if it would be filtered out\")),\n \tOPT_BOOLEAN(0, \"head\", &show_head,\n \t N_(\"show the HEAD reference, even if it would be filtered out\")),\n \tOPT_BOOLEAN('d', \"dereference\", &deref_tags,\ndiff --git a/parse-options.h b/parse-options.h\nindex c378b75..db6a6fe 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -21,9 +21,6 @@ enum parse_opt_type {\n \tOPTION_FILENAME\n };\n \n-/* Deprecated synonym */\n-#define OPTION_BOOLEAN OPTION_COUNTUP\n-\n enum parse_opt_flags {\n \tPARSE_OPT_KEEP_DASHDASH = 1,\n \tPARSE_OPT_STOP_AT_NON_OPTION = 2,\n@@ -128,6 +125,8 @@ struct option {\n #define OPT_SET_INT(s, l, v, h, i) { OPTION_SET_INT, (s), (l), (v), NULL, \\\n \t\t\t\t (h), PARSE_OPT_NOARG, NULL, (i) }\n #define OPT_BOOL(s, l, v, h) OPT_SET_INT(s, l, v, h, 1)\n+#define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \\\n+\t\t\t\t (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1}\n #define OPT_SET_PTR(s, l, v, h, p) { OPTION_SET_PTR, (s), (l), (v), NULL, \\\n \t\t\t\t (h), PARSE_OPT_NOARG, NULL, (p) }\n #define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_(\"n\"), (h) }\n-- \n1.8.4.rc0.1.g8f6a3e5\n"},{"id":"224244","messageId":"7vbo5l1c5a.fsf@alter.siamese.dyndns.org","threadId":"34559","inReplyTo":"1375112281-3935-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [PATCH] Remove deprecated OPTION_BOOLEAN","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-29T17:58:25Z","receivedAt":"2013-07-29T17:58:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> So recently Junio released -rc0. Is that a reason to not\n> send out patches, which do not do bugfixes? So the following \n> commit for example doesn't fix a bug nor does it add a feature.\n> Is it fine to send out such kind of commits during the -rc\n> times or better wait for the next development cycle?\n\nIt would be a good idea to show it to others as a preview of what to\ncome even in a pre-release freeze period, so that people holding\nreal patches in the affected area can work with you to avoid\nconflicts.\n\nBut I would not expect them to be applied to the upcoming release,\nnor even 'next'---this largely depends on the maintainer workload.\n\nThanks.\n"},{"id":"224247","messageId":"51F6AEA5.7040504@googlemail.com","threadId":"34559","inReplyTo":"7vbo5l1c5a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Remove deprecated OPTION_BOOLEAN","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-07-29T18:04:21Z","receivedAt":"2013-07-29T18:04:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 07/29/2013 07:58 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n> \n>> So recently Junio released -rc0. Is that a reason to not\n>> send out patches, which do not do bugfixes? So the following \n>> commit for example doesn't fix a bug nor does it add a feature.\n>> Is it fine to send out such kind of commits during the -rc\n>> times or better wait for the next development cycle?\n> \n> It would be a good idea to show it to others as a preview of what to\n> come even in a pre-release freeze period, so that people holding\n> real patches in the affected area can work with you to avoid\n> conflicts.\n> \n> But I would not expect them to be applied to the upcoming release,\n> nor even 'next'---this largely depends on the maintainer workload.\n> \n> Thanks.\n> \n\nThanks for the answer. Let me rephrase the question a little bit:\nWould you rather encourage to send it out now, or later?\n(\"I'll apply it ot a local branch and eventually remember it next\nrelease to merge.\" or \"I'll forget about such stuff, you need to\nresend it anyway.\")\n\nRegarding possible merge conflicts I'd be happy to resolve them\nand not add a burden to people doing features.\n\nStefan\n\n\n"},{"id":"224248","messageId":"7v4nbd1bjj.fsf@alter.siamese.dyndns.org","threadId":"34559","inReplyTo":"1375112281-3935-2-git-send-email-stefanbeller@googlemail.com","subject":"Re: [PATCH] Remove deprecated OPTION_BOOLEAN","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-29T18:11:28Z","receivedAt":"2013-07-29T18:11:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> As of b04ba2bb4 OPTION_BOOLEAN was deprecated.\n\nThe primary purpose of b04ba2bb (parse-options: deprecate\nOPT_BOOLEAN, 2011-09-27) is to deprecate OPT_BOOLEAN(), which was\nhard to use correctly.\n\nOPT_BOOLEAN() is not touched at all with this patch, it seems. Do\nthey want count-up semantics?\n\n> This commit removes all occurrences of OPTION_BOOLEAN.\n> In b04ba2bb4 Junio suggested to replace it with either\n> OPTION_SET_INT or OPTION_COUNTUP instead. However a pattern, which\n> occurred often with the OPTION_BOOLEAN was a hidden boolean parameter.\n> So I defined OPT_HIDDEN_BOOL as an additional possible parse option\n> in parse-options.h to make life easy.\n>\n> The OPT_HIDDEN_BOOL was used in checkout, clone, commit, show-ref.\n> The only exception, where there was need to fiddle with OPTION_SET_INT\n> was log and notes. However in these two files there is also a pattern,\n> so we could think of introducing OPT_NONEG_BOOL.\n>\n> Signed-off-by: Stefan Beller \n\nAt first glance, it looked to me that OPT_HIDDEN_BOOL was a good\naddition, given how often we have PARSE_OPT_HIDDEN. While I think\nsome of the hidden ones are justified, I am not sure if the hiding\nof many options are. If we stop hiding many of them, HIDDEN_BOOL\nmay become not so useful. I dunno.\n"},{"id":"224250","messageId":"51F6B20B.1010709@googlemail.com","threadId":"34559","inReplyTo":"7v4nbd1bjj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Remove deprecated OPTION_BOOLEAN","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-07-29T18:18:51Z","receivedAt":"2013-07-29T18:18:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 07/29/2013 08:11 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n> \n>> As of b04ba2bb4 OPTION_BOOLEAN was deprecated.\n> \n> The primary purpose of b04ba2bb (parse-options: deprecate\n> OPT_BOOLEAN, 2011-09-27) is to deprecate OPT_BOOLEAN(), which was\n> hard to use correctly.\n> \n> OPT_BOOLEAN() is not touched at all with this patch, it seems. Do\n> they want count-up semantics?\n> \n\nI'm working on the OPT_BOOLEAN replacement as well, but the \nOPTION_BOOLEAN seemed as an easier start (way less until completion)\n\n> At first glance, it looked to me that OPT_HIDDEN_BOOL was a good\n> addition, given how often we have PARSE_OPT_HIDDEN. While I think\n> some of the hidden ones are justified, I am not sure if the hiding\n> of many options are. If we stop hiding many of them, HIDDEN_BOOL\n> may become not so useful. I dunno.\n\nThere should be no change in semantics.\nAll those which now use the OPT_HIDDEN_BOOL have had \n{OPTION_BOOLEAN,..., PARSE_OPT_HIDDEN}\nbefore.\n\nBut your concern sounds more like you'd dislike the\nappearance of OPT_HIDDEN_BOOL because you'd want to remove the\nhidden options. The options being hidden there sound to me as if\nthose were hidden due to their historical nature and there is always\na non-hidden equivalent being better worded.\n\nStefan\n\n"},{"id":"224254","messageId":"7vzjt5yzd0.fsf@alter.siamese.dyndns.org","threadId":"34559","inReplyTo":"51F6AEA5.7040504@googlemail.com","subject":"Re: [PATCH] Remove deprecated OPTION_BOOLEAN","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-29T18:50:35Z","receivedAt":"2013-07-29T18:50:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On 07/29/2013 07:58 PM, Junio C Hamano wrote:\n>> Stefan Beller writes:\n>> \n>>> So recently Junio released -rc0. Is that a reason to not\n>>> send out patches, which do not do bugfixes? So the following \n>>> commit for example doesn't fix a bug nor does it add a feature.\n>>> Is it fine to send out such kind of commits during the -rc\n>>> times or better wait for the next development cycle?\n>> \n>> It would be a good idea to show it to others as a preview of what to\n>> come even in a pre-release freeze period, so that people holding\n>> real patches in the affected area can work with you to avoid\n>> conflicts.\n>> \n>> But I would not expect them to be applied to the upcoming release,\n>> nor even 'next'---this largely depends on the maintainer workload.\n>> \n>> Thanks.\n>> \n>\n> Thanks for the answer. Let me rephrase the question a little bit:\n> Would you rather encourage to send it out now, or later?\n> (\"I'll apply it ot a local branch and eventually remember it next\n> release to merge.\" or \"I'll forget about such stuff, you need to\n> resend it anyway.\")\n\nIt is closer to the latter (\"I'd appreciate if submitters let me\nforget about them\"), but still you may want to send them out for\nearly reviews from others.\n\n>\n> Regarding possible merge conflicts I'd be happy to resolve them\n> and not add a burden to people doing features.\n>\n> Stefan\n"}]}