{"thread":{"id":"25807","subject":"[PATCH] rebase -i: remove undocumented '--verify' flag","startedAt":"2010-11-22T06:48:24Z","lastAt":"2010-11-24T01:09:06Z","messageCount":11,"participants":["Martin von Zweigbergk","Matthieu Moy","Thomas Rast","Kevin Ballard","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"156309","messageId":"1290408504-14639-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"25807","inReplyTo":null,"subject":"[PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-22T06:48:24Z","receivedAt":"2010-11-22T06:48:24Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Remove the undocumented and unused '--verify' flag from interactive\nrebase.\n\nSigned-off-by: Martin von Zweigbergk \n---\nThis tiny and almost pointless patch has been in my refactor-rebase\nbranch for quite some time. As it's not really related to the\nrefactoring, I'm sending it off separately instead.\n\n git-rebase--interactive.sh | 2 --\n 1 files changed, 0 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex a27952d..f7171e8 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -726,8 +726,6 @@ do\n \t--no-verify)\n \t\tOK_TO_SKIP_PRE_REBASE=yes\n \t\t;;\n-\t--verify)\n-\t\t;;\n \t--continue)\n \t\tis_standalone \"$@\" || usage\n \t\tget_saved_options\n-- \n1.7.3.2.190.gfb4ae\n"},{"id":"156310","messageId":"vpqoc9hsemy.fsf@bauges.imag.fr","threadId":"25807","inReplyTo":"1290408504-14639-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-11-22T12:53:57Z","receivedAt":"2010-11-22T12:53:57Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Martin von Zweigbergk writes:\n\n> Remove the undocumented and unused '--verify' flag from interactive\n> rebase.\n\nI don't think this change is good. If a command has a --no-whatever\nflag, one expects the --whatever flag to exist too, even if it's a\nno-op.\n\n> \t--no-verify)\n> \t\tOK_TO_SKIP_PRE_REBASE=yes\n> \t\t;;\n\n--no-verify exists, so\n\n> -\t--verify)\n> -\t\t;;\n\n--verify exists too.\n\nI think a better change would be to add a comment like\n\n--verify)\n\t# no-op, exists because --no-verify exists too.\n\t;;\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"156311","messageId":"201011221414.15982.trast@student.ethz.ch","threadId":"25807","inReplyTo":"vpqoc9hsemy.fsf@bauges.imag.fr","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-11-22T13:14:15Z","receivedAt":"2010-11-22T13:14:15Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Matthieu Moy wrote:\n> Martin von Zweigbergk writes:\n> \n> > Remove the undocumented and unused '--verify' flag from interactive\n> > rebase.\n> \n> I don't think this change is good. If a command has a --no-whatever\n> flag, one expects the --whatever flag to exist too, even if it's a\n> no-op.\n[...]\n> I think a better change would be to add a comment like\n> \n> --verify)\n> \t# no-op, exists because --no-verify exists too.\n\nShouldn't that be\n\n OK_TO_SKIP_PRE_REBASE=\n\ninstead, so that it undoes the effect of an earlier --no-verify?\n\n> \t;;\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"156314","messageId":"vpq8w0lqyf7.fsf@bauges.imag.fr","threadId":"25807","inReplyTo":"201011221414.15982.trast@student.ethz.ch","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-11-22T13:29:32Z","receivedAt":"2010-11-22T13:29:32Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Thomas Rast writes:\n\n> Matthieu Moy wrote:\n>> Martin von Zweigbergk writes:\n>> \n>> > Remove the undocumented and unused '--verify' flag from interactive\n>> > rebase.\n>> \n>> I don't think this change is good. If a command has a --no-whatever\n>> flag, one expects the --whatever flag to exist too, even if it's a\n>> no-op.\n> [...]\n>> I think a better change would be to add a comment like\n>> \n>> --verify)\n>> \t# no-op, exists because --no-verify exists too.\n>\n> Shouldn't that be\n>\n> OK_TO_SKIP_PRE_REBASE=\n>\n> instead, so that it undoes the effect of an earlier --no-verify?\n\nYes, right. Useful when an alias contains --no-whatever in particular.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"156316","messageId":"AANLkTi=VXpT0PZvm4by0CvaLQ=EfuhMywQiHszYyQ_v_@mail.gmail.com","threadId":"25807","inReplyTo":"201011221414.15982.trast@student.ethz.ch","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-22T13:49:47Z","receivedAt":"2010-11-22T13:49:47Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, Nov 22, 2010 at 8:14 AM, Thomas Rast wrote:\n> Matthieu Moy wrote:\n>> Martin von Zweigbergk writes:\n>>\n>> > Remove the undocumented and unused '--verify' flag from interactive\n>> > rebase.\n>>\n>> I don't think this change is good. If a command has a --no-whatever\n>> flag, one expects the --whatever flag to exist too, even if it's a\n>> no-op.\n> [...]\n>> I think a better change would be to add a comment like\n>>\n>> --verify)\n>>       # no-op, exists because --no-verify exists too.\n>\n> Shouldn't that be\n>\n>  OK_TO_SKIP_PRE_REBASE=\n>\n> instead, so that it undoes the effect of an earlier --no-verify?\n>\n\nYes. But because it did not work like that, it was not documented and it\nwas only accepted by interactive rebase, I thought it was best to just\nremove it.\n\nHowever, I do understand Matthieu's point about having a '--whatever'\noption for every '--no-whatever' option. So, if that is the general\nopinion, I wouldn't mind adding the flag to non-interactive rebase as\nwell. As long as it is consistent, it would simplify things for the user\n(and, which is more important to me right now :-), for me while\nrefactoring this code.)\n\n/Martin\n"},{"id":"156347","messageId":"alpine.DEB.1.10.1011222103290.17721@debian","threadId":"25807","inReplyTo":"vpq8w0lqyf7.fsf@bauges.imag.fr","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-22T20:21:01Z","receivedAt":"2010-11-22T20:21:01Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\n\nOn Mon, 22 Nov 2010, Matthieu Moy wrote:\n\n> Thomas Rast writes:\n>\n>> Matthieu Moy wrote:\n>>> Martin von Zweigbergk writes:\n>>>\n>>>> Remove the undocumented and unused '--verify' flag from interactive\n>>>> rebase.\n>>>\n>>> I don't think this change is good. If a command has a --no-whatever\n>>> flag, one expects the --whatever flag to exist too, even if it's a\n>>> no-op.\n>> [...]\n>>> I think a better change would be to add a comment like\n>>>\n>>> --verify)\n>>> \t# no-op, exists because --no-verify exists too.\n>>\n>> Shouldn't that be\n>>\n>> OK_TO_SKIP_PRE_REBASE=\n>>\n>> instead, so that it undoes the effect of an earlier --no-verify?\n>\n> Yes, right. Useful when an alias contains --no-whatever in particular.\n\n\nAlright, how about something like this instead?\n\n(I hope this is the correct way of including a patch. I have only used\n'git send-email before'. I noticed that Jeff seems to remove the first\nthree lines and put a '-- 8> --' before, but others do not. What does\nthe mysterious header mean?)\n\n\n----\nFrom 90c14fe48ab921ae60000e4f9de02f97f867e273 Mon Sep 17 00:00:00 2001\nFrom: Martin von Zweigbergk \nDate: Mon, 22 Nov 2010 20:42:50 +0100\nSubject: [PATCH] rebase: support --verify\n\nInteractive rebase allows the '--verify' option to be passed, but it will\nbe ignored. Implement proper support for the option for both interactive\nand non-interactive rebase by making it override any previous\n'--no-verify'.\n\nSigned-off-by: Martin von Zweigbergk \n---\n Documentation/git-rebase.txt | 4 ++++\n git-rebase--interactive.sh | 2 ++\n git-rebase.sh | 3 +++\n 3 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex f3753a8..1f5ce74 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -279,6 +279,10 @@ which makes little sense.\n --no-verify::\n \tThis option bypasses the pre-rebase hook. See also linkgit:githooks[5].\n\n+--verify::\n+\tAllows the pre-rebase hook to run, which is the default. This option can\n+\tbe used to override --no-verify. See also linkgit:githooks[5].\n+\n -C::\n \tEnsure at least lines of surrounding context match before\n \tand after each change. When fewer lines of surrounding\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex a27952d..4eabe54 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -28,6 +28,7 @@ continue continue rebasing process\n abort abort rebasing process and restore original branch\n skip skip current patch and continue rebasing process\n no-verify override pre-rebase hook from stopping the operation\n+verify allow pre-rebase hook to run\n root rebase all reachable commmits up to the root(s)\n autosquash move commits that begin with squash!/fixup! under -i\n \"\n@@ -727,6 +728,7 @@ do\n \t\tOK_TO_SKIP_PRE_REBASE=yes\n \t\t;;\n \t--verify)\n+\t\tOK_TO_SKIP_PRE_REBASE=\n \t\t;;\n \t--continue)\n \t\tis_standalone \"$@\" || usage\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 3d194b1..595fca2 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -206,6 +206,9 @@ do\n \t--no-verify)\n \t\tOK_TO_SKIP_PRE_REBASE=yes\n \t\t;;\n+\t--verify)\n+\t\tOK_TO_SKIP_PRE_REBASE=\n+\t\t;;\n \t--continue)\n \t\ttest -d \"$dotest\" -o -d \"$GIT_DIR\"/rebase-apply ||\n \t\t\tdie \"No rebase in progress?\"\n-- \n1.7.3.2.190.gfb4ae\n"},{"id":"156348","messageId":"CA8E4FB8-70F7-415C-85DE-746B1113AE4C@sb.org","threadId":"25807","inReplyTo":"alpine.DEB.1.10.1011222103290.17721@debian","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Kevin Ballard","fromEmail":"kevin@sb.org","sentAt":"2010-11-23T02:24:10Z","receivedAt":"2010-11-23T02:24:10Z","isPatch":true,"sender":{"key":"kevin@sb.org","avatar":"https://avatars.githubusercontent.com/u/714?v=4"},"body":"On Nov 22, 2010, at 12:21 PM, Martin von Zweigbergk wrote:\n\n> (I hope this is the correct way of including a patch. I have only used\n> 'git send-email before'. I noticed that Jeff seems to remove the first\n> three lines and put a '-- 8> --' before, but others do not. What does\n> the mysterious header mean?)\n\nIt's actually 8< or >8, and it's a little ASCII icon of a pair of scissors.\nIf a line consists mainly of dashes and scissors then `git am --scissors`\ncan split the mail on that line and treat the rest of the body after that\nline as a patch.\n\n-Kevin Ballard\n"},{"id":"156389","messageId":"20101123144133.GA3145@sigill.intra.peff.net","threadId":"25807","inReplyTo":"CA8E4FB8-70F7-415C-85DE-746B1113AE4C@sb.org","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-11-23T14:41:33Z","receivedAt":"2010-11-23T14:41:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 22, 2010 at 06:24:10PM -0800, Kevin Ballard wrote:\n\n> On Nov 22, 2010, at 12:21 PM, Martin von Zweigbergk wrote:\n> \n> > (I hope this is the correct way of including a patch. I have only used\n> > 'git send-email before'. I noticed that Jeff seems to remove the first\n> > three lines and put a '-- 8> --' before, but others do not. What does\n> > the mysterious header mean?)\n> \n> It's actually 8< or >8, and it's a little ASCII icon of a pair of scissors.\n> If a line consists mainly of dashes and scissors then `git am --scissors`\n> can split the mail on that line and treat the rest of the body after that\n> line as a patch.\n\nYep. I don't know if Junio actually uses --scissors these days. Before\nit existed, he would just snip the parts above the marker manually,\nwhich was not a big deal because he reads his mail in emacs. If that is\nstill the case, then it doesn't really matter what the marker is, as\nlong as he recognizes it, and it is not \"---\", which is the special\nmarker for splitting commit message from cover letter.\n\nThat being said, people other than Junio may apply your patch, so if you\nare going to use such a marker, making it look scissor-like is probably\nthe best thing to do.\n\n-Peff\n"},{"id":"156441","messageId":"7vk4k3vm4a.fsf@alter.siamese.dyndns.org","threadId":"25807","inReplyTo":"20101123144133.GA3145@sigill.intra.peff.net","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-23T20:08:37Z","receivedAt":"2010-11-23T20:08:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> That being said, people other than Junio may apply your patch, so if you\n> are going to use such a marker, making it look scissor-like is probably\n> the best thing to do.\n\nFWIW, I do run \"am -sc3\" from inside Emacs. It is a good idea to use what\nis already accepted by the tool than coming up with a random convention of\nyour own anyway, so if somebody really wants to do \"introductory comments\nin discussion and then a patch\", \"-- >8 --\" without quotes alone on a line\nwould be an appropriate thing to use.\n"},{"id":"156443","messageId":"7vfwurvluh.fsf@alter.siamese.dyndns.org","threadId":"25807","inReplyTo":"alpine.DEB.1.10.1011222103290.17721@debian","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-23T20:14:30Z","receivedAt":"2010-11-23T20:14:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> (I hope this is the correct way of including a patch. I have only used\n> 'git send-email before'. I noticed that Jeff seems to remove the first\n> three lines and put a '-- 8> --' before, but others do not. What does\n> the mysterious header mean?)\n> -- >8 --\n> Subject: [PATCH] rebase: support --verify\n>\n> Interactive rebase allows the '--verify' option to be passed, but it will\n> be ignored. Implement proper support for the option for both interactive\n> and non-interactive rebase by making it override any previous\n> '--no-verify'.\n>\n> Signed-off-by: Martin von Zweigbergk \n\nSounds like a sane thing to do.\n\n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index f3753a8..1f5ce74 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -279,6 +279,10 @@ which makes little sense.\n> --no-verify::\n> \tThis option bypasses the pre-rebase hook. See also linkgit:githooks[5].\n>\n> +--verify::\n> +\tAllows the pre-rebase hook to run, which is the default. This option can\n> +\tbe used to override --no-verify. See also linkgit:githooks[5].\n> +\n> -C::\n> \tEnsure at least lines of surrounding context match before\n> \tand after each change. When fewer lines of surrounding\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index a27952d..4eabe54 100755\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -28,6 +28,7 @@ continue continue rebasing process\n> abort abort rebasing process and restore original branch\n> skip skip current patch and continue rebasing process\n> no-verify override pre-rebase hook from stopping the operation\n> +verify allow pre-rebase hook to run\n\nSomehow this patch seems severely whitespace mangled---please check your\nMUA. I think I've seen Alpine send patches sanely; there should be a way\nto tell it to behave.\n\nNo need to resend; I can unmunge this patch by hand.\n"},{"id":"156461","messageId":"AANLkTinF8kTRS+MoxLhnVNNN2+jdev5N9cRM-TwHv9np@mail.gmail.com","threadId":"25807","inReplyTo":"7vfwurvluh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rebase -i: remove undocumented '--verify' flag","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2010-11-24T01:09:06Z","receivedAt":"2010-11-24T01:09:06Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Tue, Nov 23, 2010 at 3:14 PM, Junio C Hamano wrote:\n> Somehow this patch seems severely whitespace mangled---please check your\n> MUA.  I think I've seen Alpine send patches sanely; there should be a way\n> to tell it to behave.\n\nHmm... It must be the format=flowed stuff, which should be turned off\naccording to SubmittingPatches. It is apparently on by default in\nAlpine. I have now turned it off, so it will hopefully look\nbetter next time. I tried sending the patch to myself, but I couldn't\nsee any difference either in Gmail or in Alpine. As far as I can\nunderstand, this option only affects the presentation. I didn't see\nanything wrong with the raw message.\n\nSorry about the inconvenience :-(.\n\n> No need to resend; I can unmunge this patch by hand.\n\nThanks.\n"}]}