{"thread":{"id":"24530","subject":"[RFC PATCH 0/2] Allow detached forms (--option arg) for git log options.","startedAt":"2010-07-26T18:14:36Z","lastAt":"2010-08-01T05:24:37Z","messageCount":20,"participants":["Matthieu Moy","Jonathan Nieder","Sverre Rabbelier","Miles Bader","Ævar Arnfjörð Bjarmason","Jakub Narebski","Pierre Habouzit"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"146403","messageId":"1280168078-31147-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24530","inReplyTo":null,"subject":"[RFC PATCH 0/2] Allow detached forms (--option arg) for git log options.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-26T18:14:36Z","receivedAt":"2010-07-26T18:14:36Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Hi,\n\nThis has been bothering me for a while: many commands accept detached\nform (like \"git commit -m message\" instead of \"git commit -mmessage\"),\nbut others don't, in particular, git log options like\n\ngit log -S\ngit log --grep=\n\ndo not accept spaces.\n\nThis small patch serie is a very early RFC: it implements the feature\nfor just two options. There are at least 4 ways towards a real\nimplementations:\n\n1) nobody except me likes the feature, drop the RFC.\n\n2) Implement the same for other options. That's very repetitive (for\n each option, there are two ifs: a prefixcmp and a strcmp), I don't\n like it much.\n\n3) Write a function or macro that accepts both variants, and use it\n everywhere.\n\n4) use parse-option for \"git log\" options and then get the feature for\n free.\n\nHence my question: is there any reason why \"git log\" hasn't been\nmigrated to parse-option? Or is it only that nobody did it yet?\n\nWhat do you think?\n\nThanks,\n\nMatthieu Moy (2):\n Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".\n Allow \"git log -S string\" as synonym for \"git log -Sstring\".\n\n diff.c | 5 +++++\n revision.c | 4 ++++\n 2 files changed, 9 insertions(+), 0 deletions(-)\n\n-- \n1.7.2.23.g58c3b.dirty\n"},{"id":"146404","messageId":"1280168078-31147-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"24530","inReplyTo":"1280168078-31147-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-26T18:14:37Z","receivedAt":"2010-07-26T18:14:37Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy \n---\n revision.c | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 7e82efd..e93bbd9 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1148,6 +1148,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\t\t int *unkc, const char **unkv)\n {\n \tconst char *arg = argv[0];\n+\tconst char *optarg = argv[1];\n \n \t/* pseudo revision arguments */\n \tif (!strcmp(arg, \"--all\") || !strcmp(arg, \"--branches\") ||\n@@ -1374,6 +1375,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\tadd_header_grep(revs, GREP_HEADER_COMMITTER, arg+12);\n \t} else if (!prefixcmp(arg, \"--grep=\")) {\n \t\tadd_message_grep(revs, arg+7);\n+\t} else if (!strcmp(arg, \"--grep\")) {\n+\t\tadd_message_grep(revs, optarg);\n+\t\treturn 2;\n \t} else if (!strcmp(arg, \"--extended-regexp\") || !strcmp(arg, \"-E\")) {\n \t\trevs->grep_filter.regflags |= REG_EXTENDED;\n \t} else if (!strcmp(arg, \"--regexp-ignore-case\") || !strcmp(arg, \"-i\")) {\n-- \n1.7.2.23.g58c3b.dirty\n"},{"id":"146405","messageId":"1280168078-31147-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"24530","inReplyTo":"1280168078-31147-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[RFC PATCH 2/2] Allow \"git log -S string\" as synonym for \"git log -Sstring\".","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-07-26T18:14:38Z","receivedAt":"2010-07-26T18:14:38Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy \n---\n diff.c | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 17873f3..4e3be89 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2993,6 +2993,7 @@ static int diff_scoreopt_parse(const char *opt);\n int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n {\n \tconst char *arg = av[0];\n+\tconst char *optarg = av[1];\n \n \t/* Output format options */\n \tif (!strcmp(arg, \"-p\") || !strcmp(arg, \"-u\") || !strcmp(arg, \"--patch\"))\n@@ -3182,6 +3183,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->line_termination = 0;\n \telse if (!prefixcmp(arg, \"-l\"))\n \t\toptions->rename_limit = strtoul(arg+2, NULL, 10);\n+\telse if (!strcmp(arg, \"-S\")) {\n+\t\toptions->pickaxe = optarg;\n+\t\treturn 2;\n+\t}\n \telse if (!prefixcmp(arg, \"-S\"))\n \t\toptions->pickaxe = arg + 2;\n \telse if (!strcmp(arg, \"--pickaxe-all\"))\n-- \n1.7.2.23.g58c3b.dirty\n"},{"id":"146423","messageId":"20100726193109.GA1043@burratino","threadId":"24530","inReplyTo":"1280168078-31147-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [RFC PATCH 0/2] Allow detached forms (--option arg) for git log options.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-26T19:31:09Z","receivedAt":"2010-07-26T19:31:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Matthieu,\n\nMatthieu Moy wrote:\n\n> is there any reason why \"git log\" hasn't been\n> migrated to parse-option? Or is it only that nobody did it yet?\n\nPlease go ahead. :)\n\nThe difficult piece is that the diff and revision handling options are\nshared by a large number of commands.\n\nI think my favorite idea is to provide macros to include the\nappropriate entries in option tables[1]. Plus side: very easy for\ncallers to use. Downside: bloats the option tables, though I think\nthat can be worked around.\n\nJunio seemed to suggest that adapting the current multi-pass procedure\nmight be easier[2].\n\nThat said, in the meantime, something like this series (just for -S\nit would be a big improvement already) would make sense to me.\n\nThanks.\nJonathan\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/85354/focus=85391\n[2] http://thread.gmane.org/gmane.comp.version-control.git/85354/focus=85362\n"},{"id":"146451","messageId":"AANLkTik-5FVwrFz+5hzqT4_u7MLOPg352+G3XoDcgdKs@mail.gmail.com","threadId":"24530","inReplyTo":"1280168078-31147-3-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [RFC PATCH 2/2] Allow \"git log -S string\" as synonym for \"git log -Sstring\".","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-07-27T06:42:03Z","receivedAt":"2010-07-27T06:42:03Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jul 26, 2010 at 13:14, Matthieu Moy wrote:\n>\n\n> +       else if (!strcmp(arg, \"-S\")) {\n> +               options->pickaxe = optarg;\n> +               return 2;\n> +       }\n\nTYVM.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"146452","messageId":"AANLkTikGPMDvxQKjpKOBge8UwrC_GuC36_=C_tYR_ngr@mail.gmail.com","threadId":"24530","inReplyTo":"1280168078-31147-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2010-07-27T06:43:22Z","receivedAt":"2010-07-27T06:43:22Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Jul 26, 2010 at 13:14, Matthieu Moy wrote:\n> +       } else if (!strcmp(arg, \"--grep\")) {\n> +               add_message_grep(revs, optarg);\n> +               return 2;\n\nThis one makes a little less sense since to me '--flag' are always\nbooleans, whereas '-m' can take an argument (such as '-m' from 'git\ncommit'.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"146455","messageId":"buohbjll3l9.fsf@dhlpc061.dev.necel.com","threadId":"24530","inReplyTo":"AANLkTikGPMDvxQKjpKOBge8UwrC_GuC36_=C_tYR_ngr@mail.gmail.com","subject":"Re: [RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2010-07-27T08:40:50Z","receivedAt":"2010-07-27T08:40:50Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Sverre Rabbelier writes:\n>> +       } else if (!strcmp(arg, \"--grep\")) {\n>> +               add_message_grep(revs, optarg);\n>> +               return 2;\n>\n> This one makes a little less sense since to me '--flag' are always\n> booleans, whereas '-m' can take an argument (such as '-m' from 'git\n> commit'.\n\nThe fact that --grep requires the \"=\" is amazingly confusing if you're\nused to standard GNU long-argument parsing (which many standard\nutilities use, and which git's argument syntax is clearly modelled\nafter), where both forms are equivalent, and documentation typically\nonly refers to the \"=\" form, but implicitly allows the separate-args\nform.\n\nI'm continually getting tripped up by git's idiosynchratic argument\nparsing, and it's nice to see it getting cleaned up a bit...\n\n-Miles\n\n-- \nThe trouble with most people is that they think with their hopes or\nfears or wishes rather than with their minds. -- Will Durant\n"},{"id":"146456","messageId":"vpqbp9t9uta.fsf@bauges.imag.fr","threadId":"24530","inReplyTo":"AANLkTikGPMDvxQKjpKOBge8UwrC_GuC36_=C_tYR_ngr@mail.gmail.com","subject":"Re: [RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-27T08:45:53Z","receivedAt":"2010-07-27T08:45:53Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Sverre Rabbelier writes:\n\n> Heya,\n>\n> On Mon, Jul 26, 2010 at 13:14, Matthieu Moy wrote:\n>> +       } else if (!strcmp(arg, \"--grep\")) {\n>> +               add_message_grep(revs, optarg);\n>> +               return 2;\n>\n> This one makes a little less sense since to me '--flag' are always\n> booleans, whereas '-m' can take an argument (such as '-m' from 'git\n> commit'.\n\nTry this:\n\n git commit --message foo\n\n... it works. Parse-options allows it, but handwritten option parsing\nfor git log doesn't.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146462","messageId":"AANLkTim1S_IYbPArQqX91OOPtoh2-rIWmTRon50_j2p3@mail.gmail.com","threadId":"24530","inReplyTo":"1280168078-31147-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-27T10:18:03Z","receivedAt":"2010-07-27T10:18:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Jul 26, 2010 at 18:14, Matthieu Moy wrote:\n\n> +       } else if (!strcmp(arg, \"--grep\")) {\n> +               add_message_grep(revs, optarg);\n> +               return 2;\n\nThis looks good. I've been bitten by git-log's non-standard option\nparsing. But there's still a lot of options that need the =, no?:\n\n 21 matches for \"=\"\" in buffer: revision.c\n\n 1163: if (!prefixcmp(arg, \"--max-count=\")) {\n 1166: } else if (!prefixcmp(arg, \"--skip=\")) {\n 1181: } else if (!prefixcmp(arg, \"--max-age=\")) {\n 1183: } else if (!prefixcmp(arg, \"--since=\")) {\n 1185: } else if (!prefixcmp(arg, \"--after=\")) {\n 1187: } else if (!prefixcmp(arg, \"--min-age=\")) {\n 1189: } else if (!prefixcmp(arg, \"--before=\")) {\n 1191: } else if (!prefixcmp(arg, \"--until=\")) {\n 1272: } else if (!prefixcmp(arg, \"--unpacked=\")) {\n 1297: } else if (!prefixcmp(arg, \"--pretty=\") ||\n!prefixcmp(arg, \"--format=\")) {\n 1304: } else if (!prefixcmp(arg, \"--show-notes=\")) {\n 1346: } else if (!prefixcmp(arg, \"--abbrev=\")) {\n 1362: } else if (!strncmp(arg, \"--date=\", 7)) {\n 1371: else if (!prefixcmp(arg, \"--author=\")) {\n 1373: } else if (!prefixcmp(arg, \"--committer=\")) {\n 1375: } else if (!prefixcmp(arg, \"--grep=\")) {\n 1385: } else if (!prefixcmp(arg, \"--encoding=\")) {\n 1515: if (!prefixcmp(arg, \"--glob=\")) {\n 1521: if (!prefixcmp(arg, \"--branches=\")) {\n 1527: if (!prefixcmp(arg, \"--tags=\")) {\n 1533: if (!prefixcmp(arg, \"--remotes=\")) {\n\nI think changing the option parsing so that it handles all the long\noptions consistently would be very nice (along with some tests). But\njust making --grep a special case is more confusing than requiring =\neverywhere.\n"},{"id":"146464","messageId":"m3ocdtkytn.fsf@localhost.localdomain","threadId":"24530","inReplyTo":"buohbjll3l9.fsf@dhlpc061.dev.necel.com","subject":"Re: [RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-07-27T10:24:51Z","receivedAt":"2010-07-27T10:24:51Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Miles Bader writes:\n> Sverre Rabbelier writes:\n\n>>> +       } else if (!strcmp(arg, \"--grep\")) {\n>>> +               add_message_grep(revs, optarg);\n>>> +               return 2;\n>>\n>> This one makes a little less sense since to me '--flag' are always\n>> booleans, whereas '-m' can take an argument (such as '-m' from 'git\n>> commit'.\n> \n> The fact that --grep requires the \"=\" is amazingly confusing if you're\n> used to standard GNU long-argument parsing (which many standard\n> utilities use, and which git's argument syntax is clearly modelled\n> after), where both forms are equivalent, and documentation typically\n> only refers to the \"=\" form, but implicitly allows the separate-args\n> form.\n\nI think that parseopt allows both sticky (-mfoo, --message=foo) and\nnon-sticky (-m foo, --message foo) forms, if I remember it correctly\nwith exception of arguments with *optional* parameters which require\nsticky form.\n \n> I'm continually getting tripped up by git's idiosynchratic argument\n> parsing, and it's nice to see it getting cleaned up a bit...\n\nI guess that this solution is simpler than moving to parseopt... is\nthat because log options and diff options crop everywhere? Do\nparseopt have no support for sub-parsers, like e.g. argp from libc:\n\n (libc.info.gz)Argp\n (libc.info.gz)Argp Children \n http://www.gnu.org/s/libc/manual/html_node/Argp-Children.html\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"146470","messageId":"vpqsk355ea6.fsf@bauges.imag.fr","threadId":"24530","inReplyTo":"AANLkTim1S_IYbPArQqX91OOPtoh2-rIWmTRon50_j2p3@mail.gmail.com","subject":"Re: [RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-27T11:56:33Z","receivedAt":"2010-07-27T11:56:33Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Mon, Jul 26, 2010 at 18:14, Matthieu Moy wrote:\n>\n>> +       } else if (!strcmp(arg, \"--grep\")) {\n>> +               add_message_grep(revs, optarg);\n>> +               return 2;\n>\n> This looks good. I've been bitten by git-log's non-standard option\n> parsing. But there's still a lot of options that need the =, no?:\n\nSure, that's why the patch is just an RFC. I wanted to start the\ndiscussion before diving into the repetitive task or migration to\nparse-option for others, and I picked --grep and -S because they're\nthe ones which annoys me the most.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146472","messageId":"AANLkTikcKd4nEZuot5fyZyiLqwAWl4gQyqtNg2512SKM@mail.gmail.com","threadId":"24530","inReplyTo":"vpqsk355ea6.fsf@bauges.imag.fr","subject":"Re: [RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-27T12:21:47Z","receivedAt":"2010-07-27T12:21:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 27, 2010 at 11:56, Matthieu Moy\n wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> On Mon, Jul 26, 2010 at 18:14, Matthieu Moy wrote:\n>>\n>>> +       } else if (!strcmp(arg, \"--grep\")) {\n>>> +               add_message_grep(revs, optarg);\n>>> +               return 2;\n>>\n>> This looks good. I've been bitten by git-log's non-standard option\n>> parsing. But there's still a lot of options that need the =, no?:\n>\n> Sure, that's why the patch is just an RFC. I wanted to start the\n> discussion before diving into the repetitive task or migration to\n> parse-option for others, and I picked --grep and -S because they're\n> the ones which annoys me the most.\n\nAh, there was nothing to indicate that, so I thought I'd mention the\nrest. I look forward to seeing what you'll come up with to consolidate\nthe option parsing later on.\n\nJakub's suggestion seems particularly interesting, maybe we can use\nthe optparse lib here with some modifications to allow it to parse a\nsubset of the total number of options.\n"},{"id":"146478","messageId":"vpqmxtd3vj7.fsf@bauges.imag.fr","threadId":"24530","inReplyTo":"AANLkTikcKd4nEZuot5fyZyiLqwAWl4gQyqtNg2512SKM@mail.gmail.com","subject":"Re: [RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-07-27T13:26:52Z","receivedAt":"2010-07-27T13:26:52Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Tue, Jul 27, 2010 at 11:56, Matthieu Moy\n> wrote:\n>>\n>> Sure, that's why the patch is just an RFC. I wanted to start the\n>> discussion before diving into the repetitive task or migration to\n>> parse-option for others, and I picked --grep and -S because they're\n>> the ones which annoys me the most.\n>\n> Ah, there was nothing to indicate that,\n\nExcept if you read carefully ;-)\n\nMatthieu Moy writes:\n\n> This small patch serie is a very early RFC: it implements the feature\n> for just two options.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"146479","messageId":"AANLkTilUTblhzPKiIsugHMDeK-W4gL4eUyyP6KSZbIb5@mail.gmail.com","threadId":"24530","inReplyTo":"vpqmxtd3vj7.fsf@bauges.imag.fr","subject":"Re: [RFC PATCH 1/2] Allow \"git log --grep foo\" as synonym for \"git log --grep=foo\".","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-27T13:46:46Z","receivedAt":"2010-07-27T13:46:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Jul 27, 2010 at 13:26, Matthieu Moy\n wrote:\n> Ęvar Arnfjörš Bjarmason writes:\n>\n>> On Tue, Jul 27, 2010 at 11:56, Matthieu Moy\n>> wrote:\n>>>\n>>> Sure, that's why the patch is just an RFC. I wanted to start the\n>>> discussion before diving into the repetitive task or migration to\n>>> parse-option for others, and I picked --grep and -S because they're\n>>> the ones which annoys me the most.\n>>\n>> Ah, there was nothing to indicate that,\n>\n> Except if you read carefully ;-)\n>\n> Matthieu Moy writes:\n>\n>> This small patch serie is a very early RFC: it implements the feature\n>> for just two options.\n\nAh, missed that. Sorry for the noise.\n"},{"id":"146482","messageId":"20100727144639.GU2504@madism.org","threadId":"24530","inReplyTo":"20100726193109.GA1043@burratino","subject":"Re: [RFC PATCH 0/2] Allow detached forms (--option arg) for git log options.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2010-07-27T14:46:39Z","receivedAt":"2010-07-27T14:46:39Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Jul 26, 2010 at 02:31:09PM -0500, Jonathan Nieder wrote:\n> Hi Matthieu,\n> \n> Matthieu Moy wrote:\n> \n> > is there any reason why \"git log\" hasn't been\n> > migrated to parse-option? Or is it only that nobody did it yet?\n> \n> Please go ahead. :)\n\nI started it in the past, but never went around to actually do it.\n\nI started to get rid of most of the bitfields to use explicit or-ed\nfields, but stopped at that, I don't even remember if those patches got\nmerged or not.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"146483","messageId":"m37hkhklll.fsf@localhost.localdomain","threadId":"24530","inReplyTo":"20100727144639.GU2504@madism.org","subject":"Re: [RFC PATCH 0/2] Allow detached forms (--option arg) for git log options.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-07-27T15:10:35Z","receivedAt":"2010-07-27T15:10:35Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Pierre Habouzit writes:\n\n> On Mon, Jul 26, 2010 at 02:31:09PM -0500, Jonathan Nieder wrote:\n> > Hi Matthieu,\n> > \n> > Matthieu Moy wrote:\n> > \n> > > is there any reason why \"git log\" hasn't been\n> > > migrated to parse-option? Or is it only that nobody did it yet?\n> > \n> > Please go ahead. :)\n> \n> I started it in the past, but never went around to actually do it.\n> \n> I started to get rid of most of the bitfields to use explicit or-ed\n> fields, but stopped at that, I don't even remember if those patches got\n> merged or not.\n\nWhy did you feel this change was needed / necessary? Was it\nlimitation of parseopt? Or perhaps it was for portability reasons?\nOr was it just the matter of code elegance?\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"146617","messageId":"20100728130610.GG6895@madism.org","threadId":"24530","inReplyTo":"m37hkhklll.fsf@localhost.localdomain","subject":"Re: [RFC PATCH 0/2] Allow detached forms (--option arg) for git log options.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2010-07-28T13:06:11Z","receivedAt":"2010-07-28T13:06:11Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Jul 27, 2010 at 08:10:35AM -0700, Jakub Narebski wrote:\n> Pierre Habouzit writes:\n> \n> > On Mon, Jul 26, 2010 at 02:31:09PM -0500, Jonathan Nieder wrote:\n> > > Hi Matthieu,\n> > > \n> > > Matthieu Moy wrote:\n> > > \n> > > > is there any reason why \"git log\" hasn't been\n> > > > migrated to parse-option? Or is it only that nobody did it yet?\n> > > \n> > > Please go ahead. :)\n> > \n> > I started it in the past, but never went around to actually do it.\n> > \n> > I started to get rid of most of the bitfields to use explicit or-ed\n> > fields, but stopped at that, I don't even remember if those patches got\n> > merged or not.\n> \n> Why did you feel this change was needed / necessary? Was it\n> limitation of parseopt? Or perhaps it was for portability reasons?\n> Or was it just the matter of code elegance?\n\nyou cannot take the address of a bit portably in C, so you can't let\nparseopt set/clear bits through bitfields (as in unsigned field : 1 in a\nstruct in C I mean).\n\nSo to use parseopt OPTION_BIT feature, you have to convert them to C\nflags as in \"unsigned flags\" and explicit masks defines/enums.\n\nIOW:\n\n struct foo {\n unsigned bar : 1,\n\t\t...\n\t\tbaz : 1;\n };\n\nMust be converted into:\n\n struct foo {\n #define FOO_FLAG_BAR (1U << 1)\n ...\n #define FOO_FLAG_BAZ (1U << 18)\n unsigned flags;\n }\n\nso that you can use parseopt. that's what I meant.\n\n\nThis was done for the rev-list parsing stuff e.g.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"146705","messageId":"201007291116.44859.jnareb@gmail.com","threadId":"24530","inReplyTo":"20100728130610.GG6895@madism.org","subject":"Re: [RFC PATCH 0/2] Allow detached forms (--option arg) for git log options.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-07-29T09:16:42Z","receivedAt":"2010-07-29T09:16:42Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 28 Jul 2010, Pierre Habouzit wrote:\n\n> you cannot take the address of a bit portably in C, so you can't let\n> parseopt set/clear bits through bitfields (as in unsigned field : 1 in a\n> struct in C I mean).\n> \n> So to use parseopt OPTION_BIT feature, you have to convert them to C\n> flags as in \"unsigned flags\" and explicit masks defines/enums.\n> \n> IOW:\n> \n> struct foo {\n> unsigned bar : 1,\n> \t\t...\n> \t\t baz : 1;\n> };\n> \n> Must be converted into:\n> \n> struct foo {\n> #define FOO_FLAG_BAR (1U << 1)\n> ...\n> #define FOO_FLAG_BAZ (1U << 18)\n> unsigned flags;\n> }\n> \n> so that you can use parseopt. that's what I meant.\n> \n> \n> This was done for the rev-list parsing stuff e.g.\n\ne.g. what?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"146728","messageId":"20100729183310.GA3891@madism.org","threadId":"24530","inReplyTo":"201007291116.44859.jnareb@gmail.com","subject":"Re: [RFC PATCH 0/2] Allow detached forms (--option arg) for git log options.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2010-07-29T18:33:10Z","receivedAt":"2010-07-29T18:33:10Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Jul 29, 2010 at 11:16:42AM +0200, Jakub Narebski wrote:\n> On Wed, 28 Jul 2010, Pierre Habouzit wrote:\n> \n> > you cannot take the address of a bit portably in C, so you can't let\n> > parseopt set/clear bits through bitfields (as in unsigned field : 1 in a\n> > struct in C I mean).\n> > \n> > So to use parseopt OPTION_BIT feature, you have to convert them to C\n> > flags as in \"unsigned flags\" and explicit masks defines/enums.\n> > \n> > IOW:\n> > \n> > struct foo {\n> > unsigned bar : 1,\n> > \t\t...\n> > \t\t baz : 1;\n> > };\n> > \n> > Must be converted into:\n> > \n> > struct foo {\n> > #define FOO_FLAG_BAR (1U << 1)\n> > ...\n> > #define FOO_FLAG_BAZ (1U << 18)\n> > unsigned flags;\n> > }\n> > \n> > so that you can use parseopt. that's what I meant.\n> > \n> > \n> > This was done for the rev-list parsing stuff e.g.\n> \n> e.g. what?\n\nerr no, not rev-list, diff options: struct diff_options::flags and the\nDIFF_OPT_* defines\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"146902","messageId":"20100801052437.GA10438@burratino","threadId":"24530","inReplyTo":"20100728130610.GG6895@madism.org","subject":"Re: [RFC PATCH 0/2] Allow detached forms (--option arg) for git log options.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-01T05:24:37Z","receivedAt":"2010-08-01T05:24:37Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Pierre Habouzit wrote:\n\n> you cannot take the address of a bit portably in C, so you can't let\n> parseopt set/clear bits through bitfields (as in unsigned field : 1 in a\n> struct in C I mean).\n\nFor the curious: I think this means doing something like\nv1.5.4-rc0~186^2~1 (Make the diff_options bitfields be an unsigned\nwith explicit masks, 2007-11-10), which means instead of writing\n\n\trevs->topo_order = 1;\n\none would write something like\n\n\tREV_TRAV_SET(revs, TOPO_ORDER);\n\nSee [1] and [2]. Looks simple and reasonable.\n\nWhile we are exploring ancient history, I find[3]:\n\n\t I came up with the relocation thing because I feared\n\tthat the msys port (and maybe other ?) that are about to\n\tuse (or already do) threads would step on each other toes\n\twhile recursing into a sub-array of options.\n\n\t Johannes thinks that this never happens in our\n\tcodebase, hence that my patches are an overkill.\n\n\t The likely users of this feature are currently diff\n\toptions (diff.c diff_opt_parse) and revisions\n\t(builtin-log.c setup_revisions).\n\n\t Using Johannes patch, we will have to export a global\n\tstruct diff_option (resp. struct rev_info) from diff.c\n\t(resp. revisions.c) and no function (or almost) would\n\ttake struct diff_option (resp struct rev_info) as an\n\targument because everyone would work on the global\n\tvariable[0].\n\n\t With my patches, we can work like we do now, with a\n\tmore functional approach.\n\nIs the relocation thing worth thinking about? (Mind you, I was not\nthere, so I do not know what it is nor whether it was a dead end.) If\nso, is it documented anywhere?\n\nThe table-inclusion method[4] still appeals to me very much. Well,\nwhatever seems to work best.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/63797/focus=63937\n[2] http://thread.gmane.org/gmane.comp.version-control.git/83083/focus=83114\n[3] http://thread.gmane.org/gmane.comp.version-control.git/63502/focus=63506\n[4] http://thread.gmane.org/gmane.comp.version-control.git/63505/focus=63517\n"}]}