{"thread":{"id":"11289","subject":"[PATCH 3/3] Test interaction between diff --check and --exit-code","startedAt":"2007-12-14T11:23:41Z","lastAt":"2007-12-17T16:29:08Z","messageCount":24,"participants":["Wincent Colaiuta","Junio C Hamano","Pierre Habouzit","Johannes Schindelin","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"63156","messageId":"1197631424-52586-1-git-send-email-win@wincent.com","threadId":"11289","inReplyTo":null,"subject":"[PATCH 0/3] diff topic tweaks","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-14T11:23:41Z","receivedAt":"2007-12-14T11:23:41Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"I've eyeballed and done some testing of the changes that Junio\npushed out to \"next\" and they all look good to me.\n\nSome minor adjustments:\n\n[1/3] Revert changes and extend diff option documentation\n\nSeeing as Junio's adjustments changed the behaviour, the\ndocumentation needs to be changed as well.\n\n[2/3] Use shorter error messages for whitespace problems\n\nCosmetic only.\n\n[3/3] Test interaction between diff --check and --exit-code\n\nJust to make sure it does what we think it does (and it does).\n\nCheers,\nWincent\n"},{"id":"63157","messageId":"1197631424-52586-2-git-send-email-win@wincent.com","threadId":"11289","inReplyTo":"1197631424-52586-1-git-send-email-win@wincent.com","subject":"[PATCH 1/3] Revert changes and extend diff option documentation","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-14T11:23:42Z","receivedAt":"2007-12-14T11:23:42Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"Revert the part of 62c6489 which says that --check affects\nthe exit code of \"git diff\"; as of da31b35 this is only true\nif you pass --exit-code as well or disable the pager with\n--no-pager or GIT_PAGER=cat.\n\nInstead extend the discussion of the --exit-code switch.\n\nAlso, instead of hard-coding \"what whitespace errors are\",\nmention that their classification can be controlled via\nconfig and per-path attributes.\n\nSigned-off-by: Wincent Colaiuta \n---\n Documentation/diff-options.txt | 13 +++++++------\n 1 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 9ecc1d7..54207f0 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -92,10 +92,10 @@ endif::git-format-patch[]\n \tfile gives the default to do so.\n \n --check::\n-\tWarn if changes introduce trailing whitespace\n-\tor an indent that uses a space before a tab. Exits with\n-\tnon-zero status if problems are found. Not compatible with\n-\t--exit-code.\n+\tWarn if changes introduce whitespace problems (such as\n+\ttrailing whitespace). Configuration and per-path attributes\n+\tcontrol what git classifies as a whitespace problem (see\n+\tgitlink:git-config[1] and gitlink:gitattributes[5]).\n \n --full-index::\n \tInstead of the first handful characters, show full\n@@ -197,8 +197,9 @@ endif::git-format-patch[]\n \n --exit-code::\n \tMake the program exit with codes similar to diff(1).\n-\tThat is, it exits with 1 if there were differences and\n-\t0 means no differences.\n+\tThat is, it exits with 0 if there were no differences\n+\tand 1 if there were. If --check is used and the\n+\tdifferences introduce whitespace problems exits with 3.\n \n --quiet::\n \tDisable all output of the program. Implies --exit-code.\n-- \n1.5.4.rc0.1099.g76fa0-dirty\n"},{"id":"63158","messageId":"1197631424-52586-3-git-send-email-win@wincent.com","threadId":"11289","inReplyTo":"1197631424-52586-2-git-send-email-win@wincent.com","subject":"[PATCH 2/3] Use shorter error messages for whitespace problems","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-14T11:23:43Z","receivedAt":"2007-12-14T11:23:43Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"The initial version of the whitespace_error_string() function took the\nmessages from builtin-apply.c rather than the shorter messages from\ndiff.c.\n\nThis commit addresses Junio's concern that these messages might be too\nlong (now that we can emit multiple warnings per line).\n\nSigned-off-by: Wincent Colaiuta \n---\n t/t4015-diff-whitespace.sh | 2 +-\n ws.c | 6 +++---\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 05ef78b..9bff8f5 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -121,7 +121,7 @@ test_expect_success 'check mixed spaces and tabs in indent' '\n \n \t# This is indented with SP HT SP.\n \techo \" \t foo();\" > x &&\n-\tgit diff --check | grep \"Space in indent is followed by a tab\"\n+\tgit diff --check | grep \"space before tab in indent\"\n \n '\n \ndiff --git a/ws.c b/ws.c\nindex d7d1522..46cbdd6 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -101,16 +101,16 @@ char *whitespace_error_string(unsigned ws)\n \tstruct strbuf err;\n \tstrbuf_init(&err, 0);\n \tif (ws & WS_TRAILING_SPACE)\n-\t\tstrbuf_addstr(&err, \"Adds trailing whitespace\");\n+\t\tstrbuf_addstr(&err, \"trailing whitespace\");\n \tif (ws & WS_SPACE_BEFORE_TAB) {\n \t\tif (err.len)\n \t\t\tstrbuf_addstr(&err, \", \");\n-\t\tstrbuf_addstr(&err, \"Space in indent is followed by a tab\");\n+\t\tstrbuf_addstr(&err, \"space before tab in indent\");\n \t}\n \tif (ws & WS_INDENT_WITH_NON_TAB) {\n \t\tif (err.len)\n \t\t\tstrbuf_addstr(&err, \", \");\n-\t\tstrbuf_addstr(&err, \"Indent more than 8 places with spaces\");\n+\t\tstrbuf_addstr(&err, \"indent with spaces\");\n \t}\n \treturn strbuf_detach(&err, NULL);\n }\n-- \n1.5.4.rc0.1099.g76fa0-dirty\n"},{"id":"63155","messageId":"1197631424-52586-4-git-send-email-win@wincent.com","threadId":"11289","inReplyTo":"1197631424-52586-3-git-send-email-win@wincent.com","subject":"[PATCH 3/3] Test interaction between diff --check and --exit-code","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-14T11:23:44Z","receivedAt":"2007-12-14T11:23:44Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"Make sure that it works as advertised in the man page.\n\nSigned-off-by: Wincent Colaiuta \n---\n t/t4017-diff-retval.sh | 29 +++++++++++++++++++++++++++++\n 1 files changed, 29 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t4017-diff-retval.sh b/t/t4017-diff-retval.sh\nindex 6873190..dc0b712 100755\n--- a/t/t4017-diff-retval.sh\n+++ b/t/t4017-diff-retval.sh\n@@ -76,4 +76,33 @@ test_expect_success 'git diff-index --cached HEAD' '\n \t}\n '\n \n+test_expect_success '--check --exit-code returns 0 for no difference' '\n+\n+\tgit diff --check --exit-code\n+\n+'\n+\n+test_expect_success '--check --exit-code returns 1 for a clean difference' '\n+\n+\techo \"good\" > a &&\n+\tgit diff --check --exit-code\n+\ttest $? = 1\n+\n+'\n+\n+test_expect_success '--check --exit-code returns 3 for a dirty difference' '\n+\n+\techo \"bad \" >> a &&\n+\tgit diff --check --exit-code\n+\ttest $? = 3\n+\n+'\n+\n+test_expect_success '--check with --no-pager returns 2 for dirty difference' '\n+\n+\tgit --no-pager diff --check\n+\ttest $? = 2\n+\n+'\n+\n test_done\n-- \n1.5.4.rc0.1099.g76fa0-dirty\n"},{"id":"63339","messageId":"7vejdml92c.fsf@gitster.siamese.dyndns.org","threadId":"11289","inReplyTo":"1197631424-52586-2-git-send-email-win@wincent.com","subject":"Re: [PATCH 1/3] Revert changes and extend diff option documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-16T19:43:23Z","receivedAt":"2007-12-16T19:43:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wincent Colaiuta writes:\n\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 9ecc1d7..54207f0 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -92,10 +92,10 @@ endif::git-format-patch[]\n> \tfile gives the default to do so.\n> \n> --check::\n> -\tWarn if changes introduce trailing whitespace\n> -\tor an indent that uses a space before a tab. Exits with\n> -\tnon-zero status if problems are found. Not compatible with\n> -\t--exit-code.\n> +\tWarn if changes introduce whitespace problems (such as\n> +\ttrailing whitespace). Configuration and per-path attributes\n> +\tcontrol what git classifies as a whitespace problem (see\n> +\tgitlink:git-config[1] and gitlink:gitattributes[5]).\n\nThis is not quite right, is it? The command still exits with exit code.\nIt is just that the calling process does not see it if you let it spawn\nthe pager.\n\n> @@ -197,8 +197,9 @@ endif::git-format-patch[]\n> \n> --exit-code::\n> \tMake the program exit with codes similar to diff(1).\n> -\tThat is, it exits with 1 if there were differences and\n> -\t0 means no differences.\n> +\tThat is, it exits with 0 if there were no differences\n> +\tand 1 if there were. If --check is used and the\n> +\tdifferences introduce whitespace problems exits with 3.\n\nThis side is correct.\n"},{"id":"63377","messageId":"F6F3247E-4E71-4977-9626-F0571278E1E6@wincent.com","threadId":"11289","inReplyTo":"7vejdml92c.fsf@gitster.siamese.dyndns.org","subject":"git.c option parsing (was: [PATCH 1/3] Revert changes and extend diff option documentation)","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-17T08:27:12Z","receivedAt":"2007-12-17T08:27:12Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 16/12/2007, a las 20:43, Junio C Hamano escribió:\n\n> Wincent Colaiuta writes:\n>\n>> diff --git a/Documentation/diff-options.txt b/Documentation/diff- \n>> options.txt\n>> index 9ecc1d7..54207f0 100644\n>> --- a/Documentation/diff-options.txt\n>> +++ b/Documentation/diff-options.txt\n>> @@ -92,10 +92,10 @@ endif::git-format-patch[]\n>> \tfile gives the default to do so.\n>>\n>> --check::\n>> -\tWarn if changes introduce trailing whitespace\n>> -\tor an indent that uses a space before a tab. Exits with\n>> -\tnon-zero status if problems are found. Not compatible with\n>> -\t--exit-code.\n>> +\tWarn if changes introduce whitespace problems (such as\n>> +\ttrailing whitespace). Configuration and per-path attributes\n>> +\tcontrol what git classifies as a whitespace problem (see\n>> +\tgitlink:git-config[1] and gitlink:gitattributes[5]).\n>\n> This is not quite right, is it? The command still exits with exit \n> code.\n> It is just that the calling process does not see it if you let it \n> spawn\n> the pager.\n\nYes, I know, but I thought that in the interests of brevity it would \nbe best not to clutter up the man page with that kind of detail. If a \nuser is interested in the exit code they will probably do a search of \nthe man page for \"exit\" and find the --exit-code section, which gives \nthem the answer they want. Adding a line like \"Also, if you pass --no- \npager will exit with a non-zero exit code\" doesn't really add much \nother than clutter IMO.\n\nIncidentally, and this is really tangential, I find the whole \"--no- \npager\" UI pretty horrendous, along with all the other switches parsed \nby git.c. Basically, understanding these switches requires the user to \nknow internal implementation details that he/she should most \ndefinitely not have to know.\n\nFor example, \"git --no-pager diff --check\" works as expected but \"git \ndiff --no-pager --check\" carps with \"error: invalid option: --no- \npager\". To understand why this is so, the user needs to know too many \nthings which should be internal implementation details:\n\n- that \"git\" is just a wrapper tool that chooses a low level command \nto run and runs it\n\n- that there are some parameters that affect the way all the other \ncommands run and should be passed to \"git\" only, prior to the \n\"subcommand\" part\n\n- that those parameters can't be passed after the \"subcommand\" \nspecification\n\nAnd seeing as we are actively encouraging users to always use the \"git \ndiff\" form rather than \"git-diff\", in a sense we are actively \nencourage them to think of Git as a single tool rather than a \ncollection of commands, it seems like an ugly wart that we then have \nto teach them that some parameters must go *between* the \"git\" and the \n\"diff\" but not after. And of course, if you always use the \"git-diff\" \nthen there's no way to use the --no-pager switch at all; you instead \nhave to use an environment variable.\n\nI've been thinking about ways to iron out these wrinkles and unify \nthings. Here's my thinking about a possible cause of action:\n\n1. Factor out the code in git.c which handles the common arguments \ninto separate functions; at this stage it's just a refactoring and the \nbehaviour is unchanged, but those separate functions will later be \nused from the builtins.\n\n2. Teach the builtin commands to handle those common arguments by \nusing the common functions. For example, we teach \"git diff\" to \nunderstand the --no-pager option by leveraging the functions we just \nfactored out in the previous step.\n\n3. Now, when git.c sees an argument that would normally go before a \n\"subcommand\" it no longer needs to handle it directly itself but can \ninstead pass it along to the subcommand. In other words, where it \nformerly saw \"git --no-pager diff args...\" and handled the --no-pager \npart itself, it can instead just pass \"--no-pager args...\" to the \nbuiltin diff. This gives us backward compatibility with the old \nformat, but the new format (user types \"git diff --no-pager args...\") \nwill work also.\n\n4. Update the docs: make a common-options.txt page which is included \nin all other manual pages either listing the options explicitly or \ndirecting users to the git(7) manpage (which should probably become \ngit(1) if we are talking about Git as a single tool) to learn about \nthem.\n\nBenefits of this approach: we'd have a consistent UI which didn't \nrequire too much knowledge of the internals of Git, and all of the \nfollowing would work:\n\ngit diff --no-pager args...\ngit-diff --no-pager args...\ngit --no-pager diff args...\n\nLooking at the options currently parsed by git.c, I think most of them \nwould be very straightforward to support in the way described above:\n\n--paginate\n--no-pager\n--git-dir\n--work-tree\n--bare\n\nExisting special cases:\n\n--help (calls \"git help foo\")\n--version\n\nProblematic:\n\n-p (short form of --paginate: seeing as -p is already used to mean \nother things by other commands, I think this should be deprecated as a \nsynonym for --paginate)\n\nTricky:\n\n--exec-path (this is the only one which git.c *must* handle before \npassing control to the \"subcommand\", so it actually has to look ahead \npast the \"subcommand\" part in order to see it and act upon it. \nBasically it would just have to look at all the arguments up to but \nnot including a \"--\" pathspec marker checking to see if --exec-path is \nsupplied)\n\nOf course, the above plan will only work for builtins, not for \nscripts. An additional step would be needed to enable scripts to \nhandle these options; perhaps teaching \"git rev-parse\" something...\n\nCheers,\nWincent\n"},{"id":"63381","messageId":"7vsl21aeqw.fsf@gitster.siamese.dyndns.org","threadId":"11289","inReplyTo":"F6F3247E-4E71-4977-9626-F0571278E1E6@wincent.com","subject":"Re: git.c option parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-17T08:48:23Z","receivedAt":"2007-12-17T08:48:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wincent Colaiuta writes:\n\n> Of course, the above plan will only work for builtins, not for \n> scripts. An additional step would be needed to enable scripts to \n> handle these options; perhaps teaching \"git rev-parse\" something...\n\nAs long as special options stay special and we make a rule not to allow\nany subcommand to assign its own meaning to them, the git wrapper can\nlookahead and reorder, when seeing a command line:\n\n\tgit scripted-command --special\n\ninto\n\n\tgit --special scripted-command\n\nAnd that approach would work well for built-ins as well, I would\nimagine.\n\nThere is one minor detail, though. There could be an option-parameter\nthat is literally --special. E.g.\n\n\tgit grep -e --no-paginate\n\nshould not be reordered to\n\n\tgit --no-paginate grep -e\n"},{"id":"63384","messageId":"25018761-80E8-410C-BB18-DD799F58308A@wincent.com","threadId":"11289","inReplyTo":"7vsl21aeqw.fsf@gitster.siamese.dyndns.org","subject":"Re: git.c option parsing","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-17T09:01:56Z","receivedAt":"2007-12-17T09:01:56Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 17/12/2007, a las 9:48, Junio C Hamano escribió:\n\n> Wincent Colaiuta writes:\n>\n>> Of course, the above plan will only work for builtins, not for\n>> scripts. An additional step would be needed to enable scripts to\n>> handle these options; perhaps teaching \"git rev-parse\" something...\n>\n> As long as special options stay special and we make a rule not to \n> allow\n> any subcommand to assign its own meaning to them, the git wrapper can\n> lookahead and reorder, when seeing a command line:\n>\n> \tgit scripted-command --special\n>\n> into\n>\n> \tgit --special scripted-command\n>\n> And that approach would work well for built-ins as well, I would\n> imagine.\n\nYes, and it would be simpler to implement also. The only downside is \nthat without all the other proposed changes things like \"git-dashed -- \nspecial\" wouldn't work; only teaching the builtins to actually handle \nthe special options would work in that case. And in the interests of \nconsistency I think it's pretty important that \"git subcommand -- \nspecial\" and \"git-subcommand --special\" both work the same as the user \nwould (reasonably) expect them to...\n\n> There is one minor detail, though. There could be an option-parameter\n> that is literally --special. E.g.\n>\n> \tgit grep -e --no-paginate\n>\n> should not be reordered to\n>\n> \tgit --no-paginate grep -e\n\n\nYes, that's definitely one that would need to be treated as a special \ncase.\n\nCheers,\nWincent\n"},{"id":"63387","messageId":"20071217092013.GD7453@artemis.madism.org","threadId":"11289","inReplyTo":"25018761-80E8-410C-BB18-DD799F58308A@wincent.com","subject":"Re: git.c option parsing","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T09:20:13Z","receivedAt":"2007-12-17T09:20:13Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 09:01:56AM +0000, Wincent Colaiuta wrote:\n> El 17/12/2007, a las 9:48, Junio C Hamano escribió:\n> \n> >Wincent Colaiuta writes:\n> >\n> >>Of course, the above plan will only work for builtins, not for\n> >>scripts. An additional step would be needed to enable scripts to\n> >>handle these options; perhaps teaching \"git rev-parse\" something...\n> >\n> >As long as special options stay special and we make a rule not to allow\n> >any subcommand to assign its own meaning to them, the git wrapper can\n> >lookahead and reorder, when seeing a command line:\n> >\n> >\tgit scripted-command --special\n> >\n> >into\n> >\n> >\tgit --special scripted-command\n> >\n> >And that approach would work well for built-ins as well, I would\n> >imagine.\n> \n> Yes, and it would be simpler to implement also. The only downside is that \n> without all the other proposed changes things like \"git-dashed --special\" \n> wouldn't work; only teaching the builtins to actually handle the special \n> options would work in that case. And in the interests of consistency I \n> think it's pretty important that \"git subcommand --special\" and \n> \"git-subcommand --special\" both work the same as the user would \n> (reasonably) expect them to...\n\n There is a simple way to do that, that wouldn't conflict with the git\ngrep -e one, that would be to define an array of \"Special\" flags, in a\nmacro, and have every builtin using parseopt adding that macro at the\nend.\n\n Then in git.c, you would have to scan for the command name when called\nthrough `git --opt1 --opt2 foo`, and pass it to parseopt with as argc\nthe position of `foo` in the argument array. Parseopt will trust you for\nit, and if it returns a non zero count, you have to barf, then you just\nneed to work on the rest of the array. That would mean in pseudo code\nthat this would work like:\n\n // ...\n /* when in `git --opt1 --opt2 foo -a -b -c` mode: */\n int cmd_pos = git_find_builtin_command_name(argc, argv);\n int count = parse_options(cmd_pos, argv, git_generic_options,\n\t\"git [special-options] cmd [options]\", 0);\n if (count)\n\tdie(\"unknown git command: `%s`\", argv[0]);\n argv += cmd_pos;\n argc -= cmd_pos;\n /* here we simulate an argv of {\"foo\", \"-a\", \"-b\", \"-c\"} */\n\n Of course this only works on builtins that do support parseopt other\nones will need the massage you describe. For them the sole thing to\nchange would be to replace the OPT_END() with a GIT_SPECIAL_OPTS() or\nsth alike.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63388","messageId":"20071217092656.GE7453@artemis.madism.org","threadId":"11289","inReplyTo":"20071217092013.GD7453@artemis.madism.org","subject":"Re: git.c option parsing","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T09:26:56Z","receivedAt":"2007-12-17T09:26:56Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 09:20:13AM +0000, Pierre Habouzit wrote:\n> There is a simple way to do that, that wouldn't conflict with the git\n> grep -e one, that would be to define an array of \"Special\" flags, in a\n> macro, and have every builtin using parseopt adding that macro at the\n> end.\n> \n> Then in git.c, you would have to scan for the command name when called\n> through `git --opt1 --opt2 foo`, and pass it to parseopt with as argc\n> the position of `foo` in the argument array. Parseopt will trust you for\n> it, and if it returns a non zero count, you have to barf, then you just\n> need to work on the rest of the array. That would mean in pseudo code\n> that this would work like:\n> \n> // ...\n> /* when in `git --opt1 --opt2 foo -a -b -c` mode: */\n> int cmd_pos = git_find_builtin_command_name(argc, argv);\n> int count = parse_options(cmd_pos, argv, git_generic_options,\n> \t\"git [special-options] cmd [options]\", 0);\n> if (count)\n> \tdie(\"unknown git command: `%s`\", argv[0]);\n> argv += cmd_pos;\n> argc -= cmd_pos;\n> /* here we simulate an argv of {\"foo\", \"-a\", \"-b\", \"-c\"} */\n> \n> Of course this only works on builtins that do support parseopt other\n> ones will need the massage you describe. For them the sole thing to\n> change would be to replace the OPT_END() with a GIT_SPECIAL_OPTS() or\n> sth alike.\n\n Oh and I forgot to say that I find this is a brilliant idea, because\nthere is quite a _lot_ of options that are very pervasive: --quiet,\n--verbose, … are pretty obvious candidates, but things like:\n`--abbrev=12` does make a lot of sense to me, and it would factor quite\na lot of option structures :) Okay `git --abbrev=12 status` probably\nmakes no real sense, but who cares ?\n\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63390","messageId":"20071217095014.GF7453@artemis.madism.org","threadId":"11289","inReplyTo":"20071217092013.GD7453@artemis.madism.org","subject":"[PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T09:50:14Z","receivedAt":"2007-12-17T09:50:14Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Signed-off-by: Pierre Habouzit \n---\n > // ...\n > /* when in `git --opt1 --opt2 foo -a -b -c` mode: */\n > int cmd_pos = git_find_builtin_command_name(argc, argv);\n > int count = parse_options(cmd_pos, argv, git_generic_options,\n > \t\"git [special-options] cmd [options]\", 0);\n > if (count)\n > \tdie(\"unknown git command: `%s`\", argv[0]);\n > argv += cmd_pos;\n > argc -= cmd_pos;\n > /* here we simulate an argv of {\"foo\", \"-a\", \"-b\", \"-c\"} */\n\n Or even simpler, with the following specifically tailored patch you\n can directly write:\n\n argc = parse_options(argc, argv, git_generic_options,\n\t\"git [generic-options] [cmd-options]\",\n\tPARSE_OPT_STOP_AT_ARG);\n\n and then {argc, argv} will exactly be the NULL-terminated array\n starting with the builtin command. Kind of nice :)\n\n parse-options.c | 2 ++\n parse-options.h | 1 +\n 2 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 7a08a0c..4f5c55e 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -229,6 +229,8 @@ int parse_options(int argc, const char **argv, const struct option *options,\n \t\tconst char *arg = args.argv[0];\n \n \t\tif (*arg != '-' || !arg[1]) {\n+\t\t\tif (flags & PARSE_OPT_STOP_AT_ARG)\n+\t\t\t\tbreak;\n \t\t\targv[j++] = args.argv[0];\n \t\t\tcontinue;\n \t\t}\ndiff --git a/parse-options.h b/parse-options.h\nindex 102ac31..7c636b9 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -18,6 +18,7 @@ enum parse_opt_type {\n \n enum parse_opt_flags {\n \tPARSE_OPT_KEEP_DASHDASH = 1,\n+\tPARSE_OPT_STOP_AT_ARG = 2,\n };\n \n enum parse_opt_option_flags {\n-- \n1.5.4.rc0.1147.gfd22d\n\n"},{"id":"63392","messageId":"Pine.LNX.4.64.0712171033550.9446@racer.site","threadId":"11289","inReplyTo":"20071217095014.GF7453@artemis.madism.org","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-17T10:34:20Z","receivedAt":"2007-12-17T10:34:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Dec 2007, Pierre Habouzit wrote:\n\n> diff --git a/parse-options.c b/parse-options.c\n> index 7a08a0c..4f5c55e 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -229,6 +229,8 @@ int parse_options(int argc, const char **argv, const struct option *options,\n> \t\tconst char *arg = args.argv[0];\n> \n> \t\tif (*arg != '-' || !arg[1]) {\n> +\t\t\tif (flags & PARSE_OPT_STOP_AT_ARG)\n> +\t\t\t\tbreak;\n> \t\t\targv[j++] = args.argv[0];\n> \t\t\tcontinue;\n> \t\t}\n> diff --git a/parse-options.h b/parse-options.h\n> index 102ac31..7c636b9 100644\n> --- a/parse-options.h\n> +++ b/parse-options.h\n> @@ -18,6 +18,7 @@ enum parse_opt_type {\n> \n> enum parse_opt_flags {\n> \tPARSE_OPT_KEEP_DASHDASH = 1,\n> +\tPARSE_OPT_STOP_AT_ARG = 2,\n> };\n> \n> enum parse_opt_option_flags {\n\nFunny. I already posted this:\n\nhttp://repo.or.cz/w/git/dscho.git?a=commitdiff;h=504f763a28b3109fce258b36f9e94e7c54be6f3d\n\nCiao,\nDscho\n"},{"id":"63402","messageId":"30351C09-8BED-4D81-ABDD-2E079B4D54D2@wincent.com","threadId":"11289","inReplyTo":"20071217095014.GF7453@artemis.madism.org","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-17T11:10:00Z","receivedAt":"2007-12-17T11:10:00Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 17/12/2007, a las 10:50, Pierre Habouzit escribió:\n\n> Signed-off-by: Pierre Habouzit \n> ---\n>> // ...\n>> /* when in `git --opt1 --opt2 foo -a -b -c` mode: */\n>> int cmd_pos = git_find_builtin_command_name(argc, argv);\n>> int count = parse_options(cmd_pos, argv, git_generic_options,\n>> \t\"git [special-options] cmd [options]\", 0);\n>> if (count)\n>> \tdie(\"unknown git command: `%s`\", argv[0]);\n>> argv += cmd_pos;\n>> argc -= cmd_pos;\n>> /* here we simulate an argv of {\"foo\", \"-a\", \"-b\", \"-c\"} */\n>\n> Or even simpler, with the following specifically tailored patch you\n> can directly write:\n>\n> argc = parse_options(argc, argv, git_generic_options,\n> \t\"git [generic-options] [cmd-options]\",\n> \tPARSE_OPT_STOP_AT_ARG);\n>\n> and then {argc, argv} will exactly be the NULL-terminated array\n> starting with the builtin command. Kind of nice :)\n\nIndeed, nice ideas. I think all this will lead to a nice UI \nimprovement post-1.5.4.\n\nAbout the only thing that I think would merit action *prior* to 1.5.4 \nis marking the \"-p\" switch to git (synonym for --paginate) as \ndeprecated, see as it clashes with other commands' uses of that switch \n(\"git log -p\" for example). Are there any other conflicting specials \nthat a currently parsed in git.c?\n\nCheers,\nWincent\n"},{"id":"63405","messageId":"20071217114703.GH7453@artemis.madism.org","threadId":"11289","inReplyTo":"30351C09-8BED-4D81-ABDD-2E079B4D54D2@wincent.com","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T11:47:03Z","receivedAt":"2007-12-17T11:47:03Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 11:10:00AM +0000, Wincent Colaiuta wrote:\n> El 17/12/2007, a las 10:50, Pierre Habouzit escribió:\n> \n> >Signed-off-by: Pierre Habouzit \n> >---\n> >> // ...\n> >> /* when in `git --opt1 --opt2 foo -a -b -c` mode: */\n> >> int cmd_pos = git_find_builtin_command_name(argc, argv);\n> >> int count = parse_options(cmd_pos, argv, git_generic_options,\n> >>\t\"git [special-options] cmd [options]\", 0);\n> >> if (count)\n> >>\tdie(\"unknown git command: `%s`\", argv[0]);\n> >> argv += cmd_pos;\n> >> argc -= cmd_pos;\n> >> /* here we simulate an argv of {\"foo\", \"-a\", \"-b\", \"-c\"} */\n> >\n> > Or even simpler, with the following specifically tailored patch you\n> > can directly write:\n> >\n> > argc = parse_options(argc, argv, git_generic_options,\n> >\t\"git [generic-options] [cmd-options]\",\n> >\tPARSE_OPT_STOP_AT_ARG);\n> >\n> > and then {argc, argv} will exactly be the NULL-terminated array\n> > starting with the builtin command. Kind of nice :)\n> \n> Indeed, nice ideas. I think all this will lead to a nice UI improvement \n> post-1.5.4.\n> \n> About the only thing that I think would merit action *prior* to 1.5.4 is \n> marking the \"-p\" switch to git (synonym for --paginate) as deprecated, \n> see as it clashes with other commands' uses of that switch (\"git log -p\" \n> for example). Are there any other conflicting specials that a currently \n> parsed in git.c?\n\n You don't need to, and I'd see that as a regression. With my proposal,\nthere isn't any kind of need that git commands do not clash with git\nones. The parse-option mechanism will properly hide options that are\nmasked this way, dscho wrote the patch for that.\n\n git -p log -p ...\n\njust makes sense to me. CVS or SVN e.g. (don't hit me !) have the same\nkind of \"issues\", and I never found that weird.\n\n In fact I see this the other way around: git status -p that is in fact\nthe same as git -p status, is the conveniency, git -p status is the\ncanonical form.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"63406","messageId":"Pine.LNX.4.64.0712171149540.9446@racer.site","threadId":"11289","inReplyTo":"20071217114703.GH7453@artemis.madism.org","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-17T11:50:15Z","receivedAt":"2007-12-17T11:50:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Dec 2007, Pierre Habouzit wrote:\n\n> On Mon, Dec 17, 2007 at 11:10:00AM +0000, Wincent Colaiuta wrote:\n> > El 17/12/2007, a las 10:50, Pierre Habouzit escribió:\n> > \n> > >Signed-off-by: Pierre Habouzit \n> > >---\n> > >> // ...\n> > >> /* when in `git --opt1 --opt2 foo -a -b -c` mode: */\n> > >> int cmd_pos = git_find_builtin_command_name(argc, argv);\n> > >> int count = parse_options(cmd_pos, argv, git_generic_options,\n> > >>\t\"git [special-options] cmd [options]\", 0);\n> > >> if (count)\n> > >>\tdie(\"unknown git command: `%s`\", argv[0]);\n> > >> argv += cmd_pos;\n> > >> argc -= cmd_pos;\n> > >> /* here we simulate an argv of {\"foo\", \"-a\", \"-b\", \"-c\"} */\n> > >\n> > > Or even simpler, with the following specifically tailored patch you\n> > > can directly write:\n> > >\n> > > argc = parse_options(argc, argv, git_generic_options,\n> > >\t\"git [generic-options] [cmd-options]\",\n> > >\tPARSE_OPT_STOP_AT_ARG);\n> > >\n> > > and then {argc, argv} will exactly be the NULL-terminated array\n> > > starting with the builtin command. Kind of nice :)\n> > \n> > Indeed, nice ideas. I think all this will lead to a nice UI improvement \n> > post-1.5.4.\n> > \n> > About the only thing that I think would merit action *prior* to 1.5.4 is \n> > marking the \"-p\" switch to git (synonym for --paginate) as deprecated, \n> > see as it clashes with other commands' uses of that switch (\"git log -p\" \n> > for example). Are there any other conflicting specials that a currently \n> > parsed in git.c?\n> \n> You don't need to, and I'd see that as a regression. With my proposal,\n> there isn't any kind of need that git commands do not clash with git\n> ones. The parse-option mechanism will properly hide options that are\n> masked this way, dscho wrote the patch for that.\n> \n> git -p log -p ...\n> \n> just makes sense to me. CVS or SVN e.g. (don't hit me !) have the same\n> kind of \"issues\", and I never found that weird.\n> \n> In fact I see this the other way around: git status -p that is in fact\n> the same as git -p status, is the conveniency, git -p status is the\n> canonical form.\n\nI would even go further: \"git status -p\" looks utterly wrong to me.\n\nCiao,\nDscho\n"},{"id":"63410","messageId":"3CF3CEA5-72F1-47D1-ADB9-37F5C2E292A8@wincent.com","threadId":"11289","inReplyTo":"Pine.LNX.4.64.0712171149540.9446@racer.site","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-17T12:06:30Z","receivedAt":"2007-12-17T12:06:30Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 17/12/2007, a las 12:50, Johannes Schindelin escribió:\n\n> On Mon, 17 Dec 2007, Pierre Habouzit wrote:\n>\n>> On Mon, Dec 17, 2007 at 11:10:00AM +0000, Wincent Colaiuta wrote:\n>>> El 17/12/2007, a las 10:50, Pierre Habouzit escribió:\n>>>\n>>>> Signed-off-by: Pierre Habouzit \n>>>> ---\n>>>>> // ...\n>>>>> /* when in `git --opt1 --opt2 foo -a -b -c` mode: */\n>>>>> int cmd_pos = git_find_builtin_command_name(argc, argv);\n>>>>> int count = parse_options(cmd_pos, argv, git_generic_options,\n>>>>> \t\"git [special-options] cmd [options]\", 0);\n>>>>> if (count)\n>>>>> \tdie(\"unknown git command: `%s`\", argv[0]);\n>>>>> argv += cmd_pos;\n>>>>> argc -= cmd_pos;\n>>>>> /* here we simulate an argv of {\"foo\", \"-a\", \"-b\", \"-c\"} */\n>>>>\n>>>> Or even simpler, with the following specifically tailored patch \n>>>> you\n>>>> can directly write:\n>>>>\n>>>> argc = parse_options(argc, argv, git_generic_options,\n>>>> \t\"git [generic-options] [cmd-options]\",\n>>>> \tPARSE_OPT_STOP_AT_ARG);\n>>>>\n>>>> and then {argc, argv} will exactly be the NULL-terminated array\n>>>> starting with the builtin command. Kind of nice :)\n>>>\n>>> Indeed, nice ideas. I think all this will lead to a nice UI \n>>> improvement\n>>> post-1.5.4.\n>>>\n>>> About the only thing that I think would merit action *prior* to \n>>> 1.5.4 is\n>>> marking the \"-p\" switch to git (synonym for --paginate) as \n>>> deprecated,\n>>> see as it clashes with other commands' uses of that switch (\"git \n>>> log -p\"\n>>> for example). Are there any other conflicting specials that a \n>>> currently\n>>> parsed in git.c?\n>>\n>> You don't need to, and I'd see that as a regression. With my \n>> proposal,\n>> there isn't any kind of need that git commands do not clash with git\n>> ones. The parse-option mechanism will properly hide options that are\n>> masked this way, dscho wrote the patch for that.\n>>\n>> git -p log -p ...\n>>\n>> just makes sense to me. CVS or SVN e.g. (don't hit me !) have the \n>> same\n>> kind of \"issues\", and I never found that weird.\n\nYes, we know what it does because we know that \"git ... log ...\" is \nactually two commands and each one handles one of the -p switches, but \nit is much easier to present git as a single tool to the newcomer (and \nI guess I don't need to argue that case here seeing as the decision \nhas already been taken long ago to talk using dashless forms), and it \nis much easier to explain to a newcomer something like:\n\ngit log --paginate -p\n\nThan:\n\ngit -p log -p\n\n(Incidentally, who actually uses \"git -p log -p\"? Doesn't everybody \nhave a pager set-up by default?)\n\n>> In fact I see this the other way around: git status -p that is in \n>> fact\n>> the same as git -p status, is the conveniency, git -p status is the\n>> canonical form.\n>\n> I would even go further: \"git status -p\" looks utterly wrong to me.\n\nThis is exactly the kind of insider knowledge that will baffle \nnewcomers. The canonical form is canonical only because it relies on \nan historical (and continuing) implementation detail, but we shouldn't \nexpect newcomers to have to learn such details in order to know where \nto stick their command line arguments; they already have enough things \nto learn about, I think.\n\nBut it doesn't really matter. The proposed changes allow old-timers to \ncontinue putting their special options between the \"git\" and the \n\"command\". If you don't want to deprecate the -p special because of \nthe confusion it might cause, I think we should at least not give it a \nvery prominent place in the documentation, nor use it any examples.\n\nCheers,\nWincent\n"},{"id":"63415","messageId":"Pine.LNX.4.64.0712171223210.9446@racer.site","threadId":"11289","inReplyTo":"3CF3CEA5-72F1-47D1-ADB9-37F5C2E292A8@wincent.com","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-17T12:26:39Z","receivedAt":"2007-12-17T12:26:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Dec 2007, Wincent Colaiuta wrote:\n\n> Yes, we know what it does because we know that \"git ... log ...\" is \n> actually two commands and each one handles one of the -p switches, but \n> it is much easier to present git as a single tool to the newcomer (and I \n> guess I don't need to argue that case here seeing as the decision has \n> already been taken long ago to talk using dashless forms), and it is \n> much easier to explain to a newcomer something like:\n> \n> git log --paginate -p\n> \n> Than:\n> \n> git -p log -p\n\nHow about\n\n\tgit log -p\n\nHmm?\n\nFact is: you make the tool easier by having sane defaults. Not by moving \naround command line options. The option \"-p\" for git is an option that \nholds for _all_ subcommands. That's why it belongs _before_ the \nsubcommand.\n\n> But it doesn't really matter. The proposed changes allow old-timers to \n> continue putting their special options between the \"git\" and the \n> \"command\". If you don't want to deprecate the -p special because of the \n> confusion it might cause, I think we should at least not give it a very \n> prominent place in the documentation, nor use it any examples.\n\nI think it is wrong to go out of our way to support \"git status -p\" as a \nsynonym to \"git -p status\". I simply do not believe that newcomers are \nnot intelligent enough to understand that \"git -p \" means that \nthe output goes into their pager.\n\nCiao,\nDscho\n"},{"id":"63419","messageId":"26962818-F702-44D2-BD26-95D74CE21F0D@wincent.com","threadId":"11289","inReplyTo":"Pine.LNX.4.64.0712171223210.9446@racer.site","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-17T12:40:18Z","receivedAt":"2007-12-17T12:40:18Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 17/12/2007, a las 13:26, Johannes Schindelin escribió:\n\n> Hi,\n>\n> On Mon, 17 Dec 2007, Wincent Colaiuta wrote:\n>\n>> Yes, we know what it does because we know that \"git ... log ...\" is\n>> actually two commands and each one handles one of the -p switches, \n>> but\n>> it is much easier to present git as a single tool to the newcomer \n>> (and I\n>> guess I don't need to argue that case here seeing as the decision has\n>> already been taken long ago to talk using dashless forms), and it is\n>> much easier to explain to a newcomer something like:\n>>\n>> git log --paginate -p\n>>\n>> Than:\n>>\n>> git -p log -p\n>\n> How about\n>\n> \tgit log -p\n>\n> Hmm?\n>\n> Fact is: you make the tool easier by having sane defaults. Not by \n> moving\n> around command line options. The option \"-p\" for git is an option \n> that\n> holds for _all_ subcommands. That's why it belongs _before_ the\n> subcommand.\n>\n>> But it doesn't really matter. The proposed changes allow old-timers \n>> to\n>> continue putting their special options between the \"git\" and the\n>> \"command\". If you don't want to deprecate the -p special because of \n>> the\n>> confusion it might cause, I think we should at least not give it a \n>> very\n>> prominent place in the documentation, nor use it any examples.\n>\n> I think it is wrong to go out of our way to support \"git status -p\" \n> as a\n> synonym to \"git -p status\". I simply do not believe that newcomers \n> are\n> not intelligent enough to understand that \"git -p \" \n> means that\n> the output goes into their pager.\n\nBut the point is, of all the special options, -p is the *only* that \ncan't unambiguously go after the subcommand.\n\nI'm arguing that the world would be a simpler place, friendlier to \nnewcomers if *all* git commands looked like \"git subcommand opts...\", \nand at the moment -p is the only obstacle to making this so. And just \nin case it's necessary to restate this, I am not proposing removing \nsupport for the git specials subcommand opts...\" form; I'm just trying \nto make it so that we don't have to advertise it so prominently. This \nseems to be the natural complement to the move to dashless forms.\n\nCheers,\nWincent\n"},{"id":"63421","messageId":"Pine.LNX.4.64.0712171253290.9446@racer.site","threadId":"11289","inReplyTo":"26962818-F702-44D2-BD26-95D74CE21F0D@wincent.com","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-17T12:55:52Z","receivedAt":"2007-12-17T12:55:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Dec 2007, Wincent Colaiuta wrote:\n\n> El 17/12/2007, a las 13:26, Johannes Schindelin escribi?:\n> \n> > I think it is wrong to go out of our way to support \"git status -p\" as \n> > a synonym to \"git -p status\". I simply do not believe that newcomers \n> > are not intelligent enough to understand that \"git -p \" \n> > means that the output goes into their pager.\n> \n> But the point is, of all the special options, -p is the *only* that \n> can't unambiguously go after the subcommand.\n\nIt should not be put after the subcommand. That's my point. Exactly \nbecause it is -- even conceptually -- no subcommand option.\n\nCVS has many shortcomings, but one lesson here is that people had no \nproblems with \"cvs -z3 update -d -P\". See, the \"-z3\" is an option that \nhas nothing to do with the subcommand.\n\nExactly the same situation here. I never had any problems explaining why \n\"-p\" goes before the subcommand here. Never.\n\nCiao,\nDscho\n"},{"id":"63422","messageId":"223E3B44-92DA-4861-83D6-67E56F70E784@wincent.com","threadId":"11289","inReplyTo":"Pine.LNX.4.64.0712171253290.9446@racer.site","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-17T13:12:28Z","receivedAt":"2007-12-17T13:12:28Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 17/12/2007, a las 13:55, Johannes Schindelin escribió:\n\n> On Mon, 17 Dec 2007, Wincent Colaiuta wrote:\n>\n>> El 17/12/2007, a las 13:26, Johannes Schindelin escribi?:\n>>\n>>> I think it is wrong to go out of our way to support \"git status - \n>>> p\" as\n>>> a synonym to \"git -p status\". I simply do not believe that \n>>> newcomers\n>>> are not intelligent enough to understand that \"git -p \"\n>>> means that the output goes into their pager.\n>>\n>> But the point is, of all the special options, -p is the *only* that\n>> can't unambiguously go after the subcommand.\n>\n> It should not be put after the subcommand. That's my point. Exactly\n> because it is -- even conceptually -- no subcommand option.\n>\n> CVS has many shortcomings, but one lesson here is that people had no\n> problems with \"cvs -z3 update -d -P\". See, the \"-z3\" is an option \n> that\n> has nothing to do with the subcommand.\n>\n> Exactly the same situation here. I never had any problems \n> explaining why\n> \"-p\" goes before the subcommand here. Never.\n\nWould be even better if there was nothing to explain and it all just \nworked, which is what I'm proposing here. As I said, -p is the only \nspecial option which clashes with any non-special uses.\n\nBut leaving -p aside, will you oppose any patches that make it \npossible for people to write stuff like:\n\ngit init --bare\n\nPersonally, I think this is an obvious usability improvement worth \nstriving for. Given that \"git --bare init\" will continue to work under \nwhat I'm proposing, I really can't see any worthwhile argument against \nit. Because we're talking about a UI improvement for newcomers at no \ncost to old timers.\n\nCheers,\nWincent\n"},{"id":"63423","messageId":"Pine.LNX.4.64.0712171328020.9446@racer.site","threadId":"11289","inReplyTo":"223E3B44-92DA-4861-83D6-67E56F70E784@wincent.com","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-17T13:30:17Z","receivedAt":"2007-12-17T13:30:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 17 Dec 2007, Wincent Colaiuta wrote:\n\n> El 17/12/2007, a las 13:55, Johannes Schindelin escribi?:\n> \n> > I never had any problems explaining why \"-p\" goes before the \n> > subcommand here. Never.\n> \n> Would be even better if there was nothing to explain and it all just \n> worked, which is what I'm proposing here. As I said, -p is the only \n> special option which clashes with any non-special uses.\n\nEven in the best of circumstances, you have to teach people a little.\n\n> But leaving -p aside, will you oppose any patches that make it possible \n> for people to write stuff like:\n> \n> git init --bare\n> \n> Personally, I think this is an obvious usability improvement worth \n> striving for. Given that \"git --bare init\" will continue to work under \n> what I'm proposing, I really can't see any worthwhile argument against \n> it. Because we're talking about a UI improvement for newcomers at no \n> cost to old timers.\n\nMy reasoning is like as for \"-p\". \"--bare\" is not special to \"init\". It \nmakes git not guess, but work on the current directory as a bare \nrepository.\n\nIn my experience, it is easier to give people a clear-cut distinction \nbetween different concepts. Then way fewer surprises will hassle them \nlater.\n\nCiao,\nDscho\n"},{"id":"63425","messageId":"ECF98EB9-DE57-4DFF-9F5B-A7FEC3BEB022@wincent.com","threadId":"11289","inReplyTo":"Pine.LNX.4.64.0712171328020.9446@racer.site","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-12-17T13:57:53Z","receivedAt":"2007-12-17T13:57:53Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 17/12/2007, a las 14:30, Johannes Schindelin escribió:\n\n>> But leaving -p aside, will you oppose any patches that make it \n>> possible\n>> for people to write stuff like:\n>>\n>> git init --bare\n>>\n>> Personally, I think this is an obvious usability improvement worth\n>> striving for. Given that \"git --bare init\" will continue to work \n>> under\n>> what I'm proposing, I really can't see any worthwhile argument \n>> against\n>> it. Because we're talking about a UI improvement for newcomers at no\n>> cost to old timers.\n>\n> My reasoning is like as for \"-p\". \"--bare\" is not special to \n> \"init\". It\n> makes git not guess, but work on the current directory as a bare\n> repository.\n\nYes, I know what --bare does. It seems to me your argument is as \nfollows:\n\n- \"git subcommand\" is actually \"git\" (which does setup) followed by \n\"git-subcommand\" (which performs an action)[*]\n\n- the newcomer should be taught this, and know which options go \nbetween \"git\" and \"subcommand\" and which options go after\n\n- even though there is a straightforward way to have \"git subcommand \nspecials args\" work without breaking compatibility, you oppose it on \nthe grounds that users should know how Git works\n\n- the fact that CVS also has some options that go before the \nsubcommand makes it OK for git to do the same\n\n- even though we could have a better UI that didn't require any of \nthis explanation at all, we should keep the UI we have seeing as it is \neasy to explain\n\nAnd possibly some combination of these two as well:\n\n- requiring arguments to be inserted between \"git\" and \"subcommand\" \ndoesn't undermine the effort to make git appear like a single tool to \nusers\n\n- the effort to move to dashless forms has nothing to do with \npresenting git as a single tool to users\n\nWell, I basically disagree with you on most of these points; all but \nthe first one(*), really. I think it's fairly clear what both you and \nI think on this and I'm not very interested in debating it further; I \nam more interested in hearing other people's points on this to get a \nfeeling for whether there is a consensus (or lack thereof).\n\nCheers,\nWincent\n"},{"id":"63429","messageId":"47669234.7020202@viscovery.net","threadId":"11289","inReplyTo":"223E3B44-92DA-4861-83D6-67E56F70E784@wincent.com","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2007-12-17T15:13:56Z","receivedAt":"2007-12-17T15:13:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Wincent Colaiuta schrieb:\n> But leaving -p aside, will you oppose any patches that make it possible\n> for people to write stuff like:\n> \n> git init --bare\n> \n> Personally, I think this is an obvious usability improvement worth\n> striving for. Given that \"git --bare init\" will continue to work under\n> what I'm proposing, I really can't see any worthwhile argument against\n> it. Because we're talking about a UI improvement for newcomers at no\n> cost to old timers.\n\nYour point. I hate to have to think hard each time whether it's \"git --bare\ninit\" or \"git init --bare\" and \"git clone --bare\" or \"git --bare clone\" and\nwouldn't mind if I no longer needed to.\n\n-- Hannes\n"},{"id":"63436","messageId":"20071217162907.GC22554@artemis.madism.org","threadId":"11289","inReplyTo":"47669234.7020202@viscovery.net","subject":"Re: [PATCH] Have a flag to stop the option parsing at the first argument.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-12-17T16:29:08Z","receivedAt":"2007-12-17T16:29:08Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Dec 17, 2007 at 03:13:56PM +0000, Johannes Sixt wrote:\n> Wincent Colaiuta schrieb:\n> > But leaving -p aside, will you oppose any patches that make it possible\n> > for people to write stuff like:\n> > \n> > git init --bare\n> > \n> > Personally, I think this is an obvious usability improvement worth\n> > striving for. Given that \"git --bare init\" will continue to work under\n> > what I'm proposing, I really can't see any worthwhile argument against\n> > it. Because we're talking about a UI improvement for newcomers at no\n> > cost to old timers.\n> \n> Your point. I hate to have to think hard each time whether it's \"git --bare\n> init\" or \"git init --bare\" and \"git clone --bare\" or \"git --bare clone\" and\n> wouldn't mind if I no longer needed to.\n\nSeconded.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"}]}