{"thread":{"id":"28339","subject":"git-p4.skipSubmitEdit","startedAt":"2011-09-08T20:40:52Z","lastAt":"2011-12-17T17:39:03Z","messageCount":17,"participants":["L. A. Linden Levy","Vitor Antunes","Luke Diamand","Pete Wyckoff","Michael Horowitz"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"175135","messageId":"1315514452.10046.0.camel@uncle-pecos","threadId":"28339","inReplyTo":null,"subject":"git-p4.skipSubmitEdit","fromName":"L. A. Linden Levy","fromEmail":"alevy@mobitv.com","sentAt":"2011-09-08T20:40:52Z","receivedAt":"2011-09-08T20:40:52Z","isPatch":false,"sender":{"key":"alevy@mobitv.com","avatar":null},"body":"Hi All,\n\nI have been using git-p4 for a while and it has allowed me to completely\nchange the way I develop and still be able to use perforce which my\ncompany has for its main VCS. One thing that was driving me nuts was\nthat \"git p4 submit\" cycles through all of my individual commits and\nasks me if I want to change them. The way I develop I often am checking\nin 20 to 50 different small commits each with a descriptive git comment.\nI felt like I was doing double duty by having emacs open on every commit\ninto perforce. So I modified git-p4 to have an option to skip the\neditor. This option coupled with git-p4.skipSubmitEditCheck will make\nthe submission non-interactive for \"git p4 submit\".\n\nBelow are the patch and environment results:\n\n\n$ git config -l\n...\nuser.name=Loren A. Linden Levy\ngit-p4.skipsubmitedit=true\ngit-p4.skipsubmiteditcheck=true\n...\n\n$ git format-patch origin/master --stdout\n\nFrom 16c4344de0047cbaf3381eca590a3e59b0d0a25c Mon Sep 17 00:00:00 2001\nFrom: \"Loren A. Linden Levy\" \nDate: Thu, 8 Sep 2011 13:37:22 -0700\nSubject: [PATCH] changed git-p4\n\n---\n contrib/fast-import/git-p4 | 15 ++++++++++-----\n 1 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 2f7b270..a438b3e 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -935,18 +935,23 @@ class P4Submit(Command, P4UserMap):\n tmpFile.write(submitTemplate + separatorLine + diff +\nnewdiff)\n tmpFile.close()\n mtime = os.stat(fileName).st_mtime\n- if os.environ.has_key(\"P4EDITOR\"):\n- editor = os.environ.get(\"P4EDITOR\")\n+ if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n+ pass\n else:\n- editor = read_pipe(\"git var GIT_EDITOR\").strip()\n- system(editor + \" \" + fileName)\n-\n+ if os.environ.has_key(\"P4EDITOR\"):\n+ editor = os.environ.get(\"P4EDITOR\")\n+ else:\n+ editor = read_pipe(\"git var GIT_EDITOR\").strip()\n+ \n+ system(editor + \" \" + fileName)\n+ \n if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n checkModTime = False\n else:\n checkModTime = True\n \n response = \"y\"\n+\n if checkModTime and (os.stat(fileName).st_mtime <= mtime):\n response = \"x\"\n while response != \"y\" and response != \"n\":\n-- \n1.7.6.347.g4db0d\n\n\n-- \nAlex Linden Levy\nSenior Software Engineer\nMobiTV, Inc.\n6425 Christie Avenue, 5th Floor, Emeryville, CA 94608\nphone 510.450.5190 mobile 720.352.8394\nemail alevy@mobitv.com web www.mobitv.com\n\n\n"},{"id":"175163","messageId":"loom.20110909T115356-849@post.gmane.org","threadId":"28339","inReplyTo":"1315514452.10046.0.camel@uncle-pecos","subject":"Re: git-p4.skipSubmitEdit","fromName":"Vitor Antunes","fromEmail":"vitor.hda@gmail.com","sentAt":"2011-09-09T10:05:09Z","receivedAt":"2011-09-09T10:05:09Z","isPatch":false,"sender":{"key":"vitor.hda@gmail.com","avatar":"https://avatars.githubusercontent.com/u/606876?v=4"},"body":"L. A. Linden Levy mobitv.com> writes:\n\n> \n> Hi All,\n> \n> I have been using git-p4 for a while and it has allowed me to completely\n> change the way I develop and still be able to use perforce which my\n> company has for its main VCS. One thing that was driving me nuts was\n> that \"git p4 submit\" cycles through all of my individual commits and\n> asks me if I want to change them. The way I develop I often am checking\n> in 20 to 50 different small commits each with a descriptive git comment.\n> I felt like I was doing double duty by having emacs open on every commit\n> into perforce. So I modified git-p4 to have an option to skip the\n> editor. This option coupled with git-p4.skipSubmitEditCheck will make\n> the submission non-interactive for \"git p4 submit\".\n\nHi Loren,\n\nThis option was already included in a recent commit. The name that was\nused is \"skipSubmitEditCheck\". Please make sure you are using the most\nrecent version of the script.\n\nBut don't let this discourage you from submitting patches. Just makesure\nyou clone git's repository and apply your patch over \"maint\" or \"master\"\nbranches. For more details on how to submit patches you can read\nDocumentation/SubmittingPatches.\n\nVitor\n"},{"id":"175218","messageId":"4E6A514C.5080200@diamand.org","threadId":"28339","inReplyTo":"loom.20110909T115356-849@post.gmane.org","subject":"Re: git-p4.skipSubmitEdit","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-09-09T17:47:56Z","receivedAt":"2011-09-09T17:47:56Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 09/09/11 11:05, Vitor Antunes wrote:\n> L. A. Linden Levy mobitv.com> writes:\n>\n>>\n>> Hi All,\n>>\n>> I have been using git-p4 for a while and it has allowed me to completely\n>> change the way I develop and still be able to use perforce which my\n>> company has for its main VCS. One thing that was driving me nuts was\n>> that \"git p4 submit\" cycles through all of my individual commits and\n>> asks me if I want to change them. The way I develop I often am checking\n>> in 20 to 50 different small commits each with a descriptive git comment.\n>> I felt like I was doing double duty by having emacs open on every commit\n>> into perforce. So I modified git-p4 to have an option to skip the\n>> editor. This option coupled with git-p4.skipSubmitEditCheck will make\n>> the submission non-interactive for \"git p4 submit\".\n>\n> Hi Loren,\n>\n> This option was already included in a recent commit. The name that was\n> used is \"skipSubmitEditCheck\". Please make sure you are using the most\n> recent version of the script.\n\nI put that option in - glad it's of use!\n\nThat option actually just skips the check of 'did the user edit the \nfile'. git-p4 will still go ahead and bring up the file in the editor first.\n\nI get around this myself by setting EDITOR=/bin/true. That works for me \nbecause I'm only using it in a script.\n\nBut it's possible that an additional option would actually be useful.\n\n\n\n>\n> But don't let this discourage you from submitting patches. Just makesure\n> you clone git's repository and apply your patch over \"maint\" or \"master\"\n> branches. For more details on how to submit patches you can read\n> Documentation/SubmittingPatches.\n>\n> Vitor\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"175220","messageId":"1315590747.10046.5.camel@uncle-pecos","threadId":"28339","inReplyTo":"4E6A514C.5080200@diamand.org","subject":"Re: git-p4.skipSubmitEdit","fromName":"L. A. Linden Levy","fromEmail":"alevy@mobitv.com","sentAt":"2011-09-09T17:52:27Z","receivedAt":"2011-09-09T17:52:27Z","isPatch":false,"sender":{"key":"alevy@mobitv.com","avatar":null},"body":"I noticed that it only skipped the edit check. That is why I added the\nskipSubmitEdit option. If they are both true then it never opens the\neditor and never checks for an edit. Probably they should just be one\noption. I think it should probably also be a command line option to skip\nthe editor.\n\n- Alex\n\nOn Fri, 2011-09-09 at 13:47 -0400, Luke Diamand wrote:\n> On 09/09/11 11:05, Vitor Antunes wrote:\n> > L. A. Linden Levy mobitv.com> writes:\n> >\n> >>\n> >> Hi All,\n> >>\n> >> I have been using git-p4 for a while and it has allowed me to completely\n> >> change the way I develop and still be able to use perforce which my\n> >> company has for its main VCS. One thing that was driving me nuts was\n> >> that \"git p4 submit\" cycles through all of my individual commits and\n> >> asks me if I want to change them. The way I develop I often am checking\n> >> in 20 to 50 different small commits each with a descriptive git comment.\n> >> I felt like I was doing double duty by having emacs open on every commit\n> >> into perforce. So I modified git-p4 to have an option to skip the\n> >> editor. This option coupled with git-p4.skipSubmitEditCheck will make\n> >> the submission non-interactive for \"git p4 submit\".\n> >\n> > Hi Loren,\n> >\n> > This option was already included in a recent commit. The name that was\n> > used is \"skipSubmitEditCheck\". Please make sure you are using the most\n> > recent version of the script.\n> \n> I put that option in - glad it's of use!\n> \n> That option actually just skips the check of 'did the user edit the \n> file'. git-p4 will still go ahead and bring up the file in the editor first.\n> \n> I get around this myself by setting EDITOR=/bin/true. That works for me \n> because I'm only using it in a script.\n> \n> But it's possible that an additional option would actually be useful.\n> \n> \n> \n> >\n> > But don't let this discourage you from submitting patches. Just makesure\n> > you clone git's repository and apply your patch over \"maint\" or \"master\"\n> > branches. For more details on how to submit patches you can read\n> > Documentation/SubmittingPatches.\n> >\n> > Vitor\n> >\n> > --\n> > To unsubscribe from this list: send the line \"unsubscribe git\" in\n> > the body of a message to majordomo@vger.kernel.org\n> > More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n\n-- \nAlex Linden Levy\nSenior Software Engineer\nMobiTV, Inc.\n6425 Christie Avenue, 5th Floor, Emeryville, CA 94608\nphone 510.450.5190 mobile 720.352.8394\nemail alevy@mobitv.com web www.mobitv.com\n\n"},{"id":"175260","messageId":"4E6AFF5F.6090805@diamand.org","threadId":"28339","inReplyTo":"1315590747.10046.5.camel@uncle-pecos","subject":"Re: git-p4.skipSubmitEdit","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-09-10T06:10:39Z","receivedAt":"2011-09-10T06:10:39Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 09/09/11 18:52, L. A. Linden Levy wrote:\n> I noticed that it only skipped the edit check. That is why I added the\n> skipSubmitEdit option. If they are both true then it never opens the\n> editor and never checks for an edit. Probably they should just be one\n> option. I think it should probably also be a command line option to skip\n> the editor.\n\nSounds plausible. If the skipSubmitEdit is true then the edit check \nshould always be skipped.\n\n\n>\n> - Alex\n>\n> On Fri, 2011-09-09 at 13:47 -0400, Luke Diamand wrote:\n>> On 09/09/11 11:05, Vitor Antunes wrote:\n>>> L. A. Linden Levy mobitv.com> writes:\n>>>\n>>>>\n>>>> Hi All,\n>>>>\n>>>> I have been using git-p4 for a while and it has allowed me to completely\n>>>> change the way I develop and still be able to use perforce which my\n>>>> company has for its main VCS. One thing that was driving me nuts was\n>>>> that \"git p4 submit\" cycles through all of my individual commits and\n>>>> asks me if I want to change them. The way I develop I often am checking\n>>>> in 20 to 50 different small commits each with a descriptive git comment.\n>>>> I felt like I was doing double duty by having emacs open on every commit\n>>>> into perforce. So I modified git-p4 to have an option to skip the\n>>>> editor. This option coupled with git-p4.skipSubmitEditCheck will make\n>>>> the submission non-interactive for \"git p4 submit\".\n>>>\n>>> Hi Loren,\n>>>\n>>> This option was already included in a recent commit. The name that was\n>>> used is \"skipSubmitEditCheck\". Please make sure you are using the most\n>>> recent version of the script.\n>>\n>> I put that option in - glad it's of use!\n>>\n>> That option actually just skips the check of 'did the user edit the\n>> file'. git-p4 will still go ahead and bring up the file in the editor first.\n>>\n>> I get around this myself by setting EDITOR=/bin/true. That works for me\n>> because I'm only using it in a script.\n>>\n>> But it's possible that an additional option would actually be useful.\n>>\n>>\n>>\n>>>\n>>> But don't let this discourage you from submitting patches. Just makesure\n>>> you clone git's repository and apply your patch over \"maint\" or \"master\"\n>>> branches. For more details on how to submit patches you can read\n>>> Documentation/SubmittingPatches.\n>>>\n>>> Vitor\n>>>\n>>> --\n>>> To unsubscribe from this list: send the line \"unsubscribe git\" in\n>>> the body of a message to majordomo@vger.kernel.org\n>>> More majordomo info at http://vger.kernel.org/majordomo-info.html\n>>\n>\n"},{"id":"175331","messageId":"4E6DB5F0.7080303@diamand.org","threadId":"28339","inReplyTo":"1315514452.10046.0.camel@uncle-pecos","subject":"Re: git-p4.skipSubmitEdit","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-09-12T07:34:08Z","receivedAt":"2011-09-12T07:34:08Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 08/09/11 21:40, L. A. Linden Levy wrote:\n> Hi All,\n>\n> I have been using git-p4 for a while and it has allowed me to completely\n> change the way I develop and still be able to use perforce which my\n> company has for its main VCS. One thing that was driving me nuts was\n> that \"git p4 submit\" cycles through all of my individual commits and\n> asks me if I want to change them. The way I develop I often am checking\n> in 20 to 50 different small commits each with a descriptive git comment.\n> I felt like I was doing double duty by having emacs open on every commit\n> into perforce. So I modified git-p4 to have an option to skip the\n> editor. This option coupled with git-p4.skipSubmitEditCheck will make\n> the submission non-interactive for \"git p4 submit\".\n\n\nSorry - I've not had a chance to look at this before now. But a couple \nof comments:\n\n - Is there a line wrap problem in the patch? It doesn't seem to want \nto apply for me.\n\n - needs an update to the documentation in git-p4.txt\n\n - is there any way to eliminate the slightly ugly \n\"if/pass/else/do-stuff\" construct?\n\n - I'd think if you turned off the editor completely then there's no \npoint doing the submit-edit-check.\n\n - You probably need to follow the instructions in \nDocumentation/SubmittingPatches so that Junio will pay attention to the \npatch.\n\nRegards!\nLuke\n\n\n>\n> Below are the patch and environment results:\n>\n>\n> $ git config -l\n> ...\n> user.name=Loren A. Linden Levy\n> git-p4.skipsubmitedit=true\n> git-p4.skipsubmiteditcheck=true\n> ...\n>\n> $ git format-patch origin/master --stdout\n>\n> From 16c4344de0047cbaf3381eca590a3e59b0d0a25c Mon Sep 17 00:00:00 2001\n> From: \"Loren A. Linden Levy\"\n> Date: Thu, 8 Sep 2011 13:37:22 -0700\n> Subject: [PATCH] changed git-p4\n>\n> ---\n> contrib/fast-import/git-p4 | 15 ++++++++++-----\n> 1 files changed, 10 insertions(+), 5 deletions(-)\n>\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> index 2f7b270..a438b3e 100755\n> --- a/contrib/fast-import/git-p4\n> +++ b/contrib/fast-import/git-p4\n> @@ -935,18 +935,23 @@ class P4Submit(Command, P4UserMap):\n> tmpFile.write(submitTemplate + separatorLine + diff +\n> newdiff)\n> tmpFile.close()\n> mtime = os.stat(fileName).st_mtime\n> - if os.environ.has_key(\"P4EDITOR\"):\n> - editor = os.environ.get(\"P4EDITOR\")\n> + if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n> + pass\n> else:\n> - editor = read_pipe(\"git var GIT_EDITOR\").strip()\n> - system(editor + \" \" + fileName)\n> -\n> + if os.environ.has_key(\"P4EDITOR\"):\n> + editor = os.environ.get(\"P4EDITOR\")\n> + else:\n> + editor = read_pipe(\"git var GIT_EDITOR\").strip()\n> +\n> + system(editor + \" \" + fileName)\n> +\n> if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n> checkModTime = False\n> else:\n> checkModTime = True\n>\n> response = \"y\"\n> +\n> if checkModTime and (os.stat(fileName).st_mtime<= mtime):\n> response = \"x\"\n> while response != \"y\" and response != \"n\":\n"},{"id":"175343","messageId":"1315847540.10046.29.camel@uncle-pecos","threadId":"28339","inReplyTo":"4E6DB5F0.7080303@diamand.org","subject":"Re: git-p4.skipSubmitEdit","fromName":"L. A. Linden Levy","fromEmail":"alevy@mobitv.com","sentAt":"2011-09-12T17:12:19Z","receivedAt":"2011-09-12T17:12:19Z","isPatch":false,"sender":{"key":"alevy@mobitv.com","avatar":null},"body":"I agree with almost all your points. I have answered them each in-line\nbelow.\n\nOn Mon, 2011-09-12 at 03:34 -0400, Luke Diamand wrote:\n> On 08/09/11 21:40, L. A. Linden Levy wrote:\n> > Hi All,\n> >\n> > I have been using git-p4 for a while and it has allowed me to completely\n> > change the way I develop and still be able to use perforce which my\n> > company has for its main VCS. One thing that was driving me nuts was\n> > that \"git p4 submit\" cycles through all of my individual commits and\n> > asks me if I want to change them. The way I develop I often am checking\n> > in 20 to 50 different small commits each with a descriptive git comment.\n> > I felt like I was doing double duty by having emacs open on every commit\n> > into perforce. So I modified git-p4 to have an option to skip the\n> > editor. This option coupled with git-p4.skipSubmitEditCheck will make\n> > the submission non-interactive for \"git p4 submit\".\n> \n> \n> Sorry - I've not had a chance to look at this before now. But a couple \n> of comments:\n> \n> - Is there a line wrap problem in the patch? It doesn't seem to want \n> to apply for me.\n\nProbably. Below are the results from following the patch submission\ninstructions. \n\nFrom 16c4344de0047cbaf3381eca590a3e59b0d0a25c Mon Sep 17 00:00:00 2001\nFrom: \"Loren A. Linden Levy\" \nDate: Thu, 8 Sep 2011 13:37:22 -0700\nSubject: [PATCH 1/5] changed git-p4\n\n---\n contrib/fast-import/git-p4 | 15 ++++++++++-----\n 1 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 2f7b270..a438b3e 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -935,18 +935,23 @@ class P4Submit(Command, P4UserMap):\n tmpFile.write(submitTemplate + separatorLine + diff +\nnewdiff)\n tmpFile.close()\n mtime = os.stat(fileName).st_mtime\n- if os.environ.has_key(\"P4EDITOR\"):\n- editor = os.environ.get(\"P4EDITOR\")\n+ if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n+ pass\n else:\n- editor = read_pipe(\"git var GIT_EDITOR\").strip()\n- system(editor + \" \" + fileName)\n-\n+ if os.environ.has_key(\"P4EDITOR\"):\n+ editor = os.environ.get(\"P4EDITOR\")\n+ else:\n+ editor = read_pipe(\"git var GIT_EDITOR\").strip()\n+ \n+ system(editor + \" \" + fileName)\n+ \n if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n checkModTime = False\n else:\n checkModTime = True\n \n response = \"y\"\n+\n if checkModTime and (os.stat(fileName).st_mtime <= mtime):\n response = \"x\"\n while response != \"y\" and response != \"n\":\n-- \n1.7.7.rc0.73.g16c43\n\nFrom ed60bb737b89eea4b84719c458aeebebd666a2ae Mon Sep 17 00:00:00 2001\nFrom: \"Loren A. Linden Levy\" \nDate: Mon, 12 Sep 2011 09:52:52 -0700\nSubject: [PATCH 2/5] removed white space for patch\n\n---\n contrib/fast-import/git-p4 | 2 --\n 1 files changed, 0 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex a438b3e..edd2525 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -942,9 +942,7 @@ class P4Submit(Command, P4UserMap):\n editor = os.environ.get(\"P4EDITOR\")\n else:\n editor = read_pipe(\"git var GIT_EDITOR\").strip()\n- \n system(editor + \" \" + fileName)\n- \n if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n checkModTime = False\n else:\n-- \n1.7.7.rc0.73.g16c43\n\nFrom ee7eaf73cf52100359b08600eab1279e03ef777b Mon Sep 17 00:00:00 2001\nFrom: \"Loren A. Linden Levy\" \nDate: Mon, 12 Sep 2011 09:55:11 -0700\nSubject: [PATCH 3/5] modified the documentation\n\n---\n contrib/fast-import/git-p4.txt | 8 +++++++-\n 1 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4.txt\nb/contrib/fast-import/git-p4.txt\nindex 52003ae..3dba636 100644\n--- a/contrib/fast-import/git-p4.txt\n+++ b/contrib/fast-import/git-p4.txt\n@@ -202,10 +202,16 @@ able to find the relevant client. This client\nspec will be used to\n both filter the files cloned by git and set the directory layout as\n specified in the client (this implies --keep-path style semantics).\n \n-git-p4.skipSubmitModTimeCheck\n+git-p4.skipSubmitEdit\n \n git config [--global] git-p4.skipSubmitModTimeCheck false\n \n+If true, submit will not invoke the editor to modify the p4 change\ntemplate.\n+\n+git-p4.skipSubmitEditCheck \n+\n+ git config [--global] git-p4.skipSubmitEditCheck false\n+\n If true, submit will not check if the p4 change template has been\nmodified.\n \n git-p4.preserveUser\n-- \n1.7.7.rc0.73.g16c43\n\nFrom ee7eaf73cf52100359b08600eab1279e03ef777b Mon Sep 17 00:00:00 2001\nFrom: \"Loren A. Linden Levy\" \nDate: Mon, 12 Sep 2011 09:55:11 -0700\nSubject: [PATCH 3/6] modified the documentation\n\n---\n contrib/fast-import/git-p4.txt | 8 +++++++-\n 1 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4.txt\nb/contrib/fast-import/git-p4.txt\nindex 52003ae..3dba636 100644\n--- a/contrib/fast-import/git-p4.txt\n+++ b/contrib/fast-import/git-p4.txt\n@@ -202,10 +202,16 @@ able to find the relevant client. This client\nspec will be used to\n both filter the files cloned by git and set the directory layout as\n specified in the client (this implies --keep-path style semantics).\n \n-git-p4.skipSubmitModTimeCheck\n+git-p4.skipSubmitEdit\n \n git config [--global] git-p4.skipSubmitModTimeCheck false\n \n+If true, submit will not invoke the editor to modify the p4 change\ntemplate.\n+\n+git-p4.skipSubmitEditCheck \n+\n+ git config [--global] git-p4.skipSubmitEditCheck false\n+\n If true, submit will not check if the p4 change template has been\nmodified.\n \n git-p4.preserveUser\n-- \n1.7.7.rc0.73.g16c43\n\nFrom c9d3b4ea4dcf1f5fd489dfacde550dd4c648afc7 Mon Sep 17 00:00:00 2001\nFrom: \"Loren A. Linden Levy\" \nDate: Mon, 12 Sep 2011 10:01:08 -0700\nSubject: [PATCH 4/6] changed the order a bit\n\n---\n contrib/fast-import/git-p4 | 11 ++++++-----\n 1 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex edd2525..80bcc90 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -935,18 +935,19 @@ class P4Submit(Command, P4UserMap):\n tmpFile.write(submitTemplate + separatorLine + diff +\nnewdiff)\n tmpFile.close()\n mtime = os.stat(fileName).st_mtime\n+ if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n+ checkModTime = False\n+ else:\n+ checkModTime = True\n if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n- pass\n+ checkModTime = False\n+ pass \n else:\n if os.environ.has_key(\"P4EDITOR\"):\n editor = os.environ.get(\"P4EDITOR\")\n else:\n editor = read_pipe(\"git var GIT_EDITOR\").strip()\n system(editor + \" \" + fileName)\n- if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n- checkModTime = False\n- else:\n- checkModTime = True\n \n response = \"y\"\n \n-- \n1.7.7.rc0.73.g16c43\n\nFrom de3d67a301b49e4d986e83727b822aa5fd257527 Mon Sep 17 00:00:00 2001\nFrom: \"Loren A. Linden Levy\" \nDate: Mon, 12 Sep 2011 10:03:03 -0700\nSubject: [PATCH 5/6] cleaned up whitespace for making a patch\n\n---\n contrib/fast-import/git-p4 | 2 +-\n contrib/fast-import/git-p4.txt | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 80bcc90..1cd65a3 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -941,7 +941,7 @@ class P4Submit(Command, P4UserMap):\n checkModTime = True\n if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n checkModTime = False\n- pass \n+ pass\n else:\n if os.environ.has_key(\"P4EDITOR\"):\n editor = os.environ.get(\"P4EDITOR\")\ndiff --git a/contrib/fast-import/git-p4.txt\nb/contrib/fast-import/git-p4.txt\nindex 3dba636..8291e7a 100644\n--- a/contrib/fast-import/git-p4.txt\n+++ b/contrib/fast-import/git-p4.txt\n@@ -208,7 +208,7 @@ git-p4.skipSubmitEdit\n \n If true, submit will not invoke the editor to modify the p4 change\ntemplate.\n \n-git-p4.skipSubmitEditCheck \n+git-p4.skipSubmitEditCheck\n \n git config [--global] git-p4.skipSubmitEditCheck false\n \n-- \n1.7.7.rc0.73.g16c43\n\nFrom 64c4878d12748daeda637b3d7d2f018c316b4239 Mon Sep 17 00:00:00 2001\nFrom: \"Loren A. Linden Levy\" \nDate: Mon, 12 Sep 2011 10:10:22 -0700\nSubject: [PATCH 6/6] made the docs correct for the new option\n\n---\n contrib/fast-import/git-p4.txt | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4.txt\nb/contrib/fast-import/git-p4.txt\nindex 8291e7a..f1c1d0c 100644\n--- a/contrib/fast-import/git-p4.txt\n+++ b/contrib/fast-import/git-p4.txt\n@@ -204,7 +204,7 @@ specified in the client (this implies --keep-path\nstyle semantics).\n \n git-p4.skipSubmitEdit\n \n- git config [--global] git-p4.skipSubmitModTimeCheck false\n+ git config [--global] git-p4.skipSubmitEdit false\n \n If true, submit will not invoke the editor to modify the p4 change\ntemplate.\n \n-- \n1.7.7.rc0.73.g16c43\n\n\n\n> - needs an update to the documentation in git-p4.txt\n\nDone. Also I think the skipSubmitEditCheck was incorrectly name\ngit-p4.skipSubmitModTimeCheck in the git-p4.txt file. I updated this as\nwell.\n\n> - is there any way to eliminate the slightly ugly \n> \"if/pass/else/do-stuff\" construct?\n\nI think it is logically correct and describes the situation well. We are\nskipping the editor so we pass it. If you still do not agree then\nperhaps a suggestion of a better logical flow would help me. \n\n> - I'd think if you turned off the editor completely then there's no \n> point doing the submit-edit-check.\n\nI have changed the ordering so that at the moment this happens because I\nagree with you.\n\n> - You probably need to follow the instructions in \n> Documentation/SubmittingPatches so that Junio will pay attention to the \n> patch.\n\nSee point #1.\n\n> Regards!\n> Luke\n> \n> \n> >\n> > Below are the patch and environment results:\n> >\n> >\n> > $ git config -l\n> > ...\n> > user.name=Loren A. Linden Levy\n> > git-p4.skipsubmitedit=true\n> > git-p4.skipsubmiteditcheck=true\n> > ...\n> >\n> > $ git format-patch origin/master --stdout\n> >\n> > From 16c4344de0047cbaf3381eca590a3e59b0d0a25c Mon Sep 17 00:00:00 2001\n> > From: \"Loren A. Linden Levy\"\n> > Date: Thu, 8 Sep 2011 13:37:22 -0700\n> > Subject: [PATCH] changed git-p4\n> >\n> > ---\n> > contrib/fast-import/git-p4 | 15 ++++++++++-----\n> > 1 files changed, 10 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> > index 2f7b270..a438b3e 100755\n> > --- a/contrib/fast-import/git-p4\n> > +++ b/contrib/fast-import/git-p4\n> > @@ -935,18 +935,23 @@ class P4Submit(Command, P4UserMap):\n> > tmpFile.write(submitTemplate + separatorLine + diff +\n> > newdiff)\n> > tmpFile.close()\n> > mtime = os.stat(fileName).st_mtime\n> > - if os.environ.has_key(\"P4EDITOR\"):\n> > - editor = os.environ.get(\"P4EDITOR\")\n> > + if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n> > + pass\n> > else:\n> > - editor = read_pipe(\"git var GIT_EDITOR\").strip()\n> > - system(editor + \" \" + fileName)\n> > -\n> > + if os.environ.has_key(\"P4EDITOR\"):\n> > + editor = os.environ.get(\"P4EDITOR\")\n> > + else:\n> > + editor = read_pipe(\"git var GIT_EDITOR\").strip()\n> > +\n> > + system(editor + \" \" + fileName)\n> > +\n> > if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n> > checkModTime = False\n> > else:\n> > checkModTime = True\n> >\n> > response = \"y\"\n> > +\n> > if checkModTime and (os.stat(fileName).st_mtime<= mtime):\n> > response = \"x\"\n> > while response != \"y\" and response != \"n\":\n> \n\n-- \nAlex Linden Levy\nSenior Software Engineer\nMobiTV, Inc.\n6425 Christie Avenue, 5th Floor, Emeryville, CA 94608\nphone 510.450.5190 mobile 720.352.8394\nemail alevy@mobitv.com web www.mobitv.com\n\n"},{"id":"177913","messageId":"20111018004500.GA31768@arf.padd.com","threadId":"28339","inReplyTo":"1315847540.10046.29.camel@uncle-pecos","subject":"Re: git-p4.skipSubmitEdit","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-10-18T00:45:00Z","receivedAt":"2011-10-18T00:45:00Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"alevy@mobitv.com wrote on Mon, 12 Sep 2011 10:12 -0700:\n> I agree with almost all your points. I have answered them each in-line\n> below.\n> \n> On Mon, 2011-09-12 at 03:34 -0400, Luke Diamand wrote:\n> > On 08/09/11 21:40, L. A. Linden Levy wrote:\n> > > Hi All,\n> > >\n> > > I have been using git-p4 for a while and it has allowed me to completely\n> > > change the way I develop and still be able to use perforce which my\n> > > company has for its main VCS. One thing that was driving me nuts was\n> > > that \"git p4 submit\" cycles through all of my individual commits and\n> > > asks me if I want to change them. The way I develop I often am checking\n> > > in 20 to 50 different small commits each with a descriptive git comment.\n> > > I felt like I was doing double duty by having emacs open on every commit\n> > > into perforce. So I modified git-p4 to have an option to skip the\n> > > editor. This option coupled with git-p4.skipSubmitEditCheck will make\n> > > the submission non-interactive for \"git p4 submit\".\n> > \n> > \n> > Sorry - I've not had a chance to look at this before now. But a couple \n> > of comments:\n> > \n> > - Is there a line wrap problem in the patch? It doesn't seem to want \n> > to apply for me.\n> \n> Probably. Below are the results from following the patch submission\n> instructions. \n\nSorry I sat on this for a month. It is a good idea. Your\npatches were good in content, but they didn't apply well due to\nbeing line-wrapped and having one duplicate.\n\nReading the code, though, I decided that the whole\nskipSubmitEdit* checking was a bit convoluted even before you got\nto it. So I moved it all to a separate function. And added a\nunit test.\n\nTell me if you think this is okay instead. If I got a bit too\nwordy in the doc, please help with that too.\n\n\t\t-- Pete\n\n--- 8< ---\n\nSubject: [PATCH] git-p4: introduce skipSubmitEdit\n\nAdd a configuration variable to skip invoking the editor in the\nsubmit path.\n\nThe existing variable skipSubmitEditCheck continues to make sure\nthat the submit template was indeed modified by the editor; but,\nit is not considered is skipSubmitEdit is true.\n\nReported-by: Loren A. Linden Levy \nSigned-off-by: Pete Wyckoff \n---\n contrib/fast-import/git-p4 | 60 +++++++++++++++++++----------\n contrib/fast-import/git-p4.txt | 19 ++++++++-\n t/t9804-skip-submit-edit.sh | 82 ++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 137 insertions(+), 24 deletions(-)\n create mode 100755 t/t9804-skip-submit-edit.sh\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex f885d70..abd6778 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -847,6 +847,39 @@ class P4Submit(Command, P4UserMap):\n \n return template\n \n+ def edit_template(self, template_file):\n+ \"\"\"Invoke the editor to let the user change the submission\n+ message. Return true if okay to continue with the submit.\"\"\"\n+\n+ # if configured to skip the editing part, just submit\n+ if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n+ return True\n+\n+ # look at the modification time, to check later if the user saved\n+ # the file\n+ mtime = os.stat(template_file).st_mtime\n+\n+ # invoke the editor\n+ if os.environ.has_key(\"P4EDITOR\"):\n+ editor = os.environ.get(\"P4EDITOR\")\n+ else:\n+ editor = read_pipe(\"git var GIT_EDITOR\").strip()\n+ system(editor + \" \" + template_file)\n+\n+ # If the file was not saved, prompt to see if this patch should\n+ # be skipped. But skip this verification step if configured so.\n+ if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n+ print \"return true for skipSubmitEditCheck\"\n+ return True\n+\n+ if os.stat(template_file).st_mtime <= mtime:\n+ while True:\n+ response = raw_input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n+ if response == 'y':\n+ return True\n+ if response == 'n':\n+ return False\n+\n def applyCommit(self, id):\n print \"Applying %s\" % (read_pipe(\"git log --max-count=1 --pretty=oneline %s\" % id))\n \n@@ -1001,7 +1034,7 @@ class P4Submit(Command, P4UserMap):\n \n separatorLine = \"######## everything below this line is just the diff #######\\n\"\n \n- [handle, fileName] = tempfile.mkstemp()\n+ (handle, fileName) = tempfile.mkstemp()\n tmpFile = os.fdopen(handle, \"w+\")\n if self.isWindows:\n submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n@@ -1009,25 +1042,9 @@ class P4Submit(Command, P4UserMap):\n newdiff = newdiff.replace(\"\\n\", \"\\r\\n\")\n tmpFile.write(submitTemplate + separatorLine + diff + newdiff)\n tmpFile.close()\n- mtime = os.stat(fileName).st_mtime\n- if os.environ.has_key(\"P4EDITOR\"):\n- editor = os.environ.get(\"P4EDITOR\")\n- else:\n- editor = read_pipe(\"git var GIT_EDITOR\").strip()\n- system(editor + \" \" + fileName)\n \n- if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n- checkModTime = False\n- else:\n- checkModTime = True\n-\n- response = \"y\"\n- if checkModTime and (os.stat(fileName).st_mtime <= mtime):\n- response = \"x\"\n- while response != \"y\" and response != \"n\":\n- response = raw_input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n-\n- if response == \"y\":\n+ if self.edit_template(fileName):\n+ # read the edited message and submit\n tmpFile = open(fileName, \"rb\")\n message = tmpFile.read()\n tmpFile.close()\n@@ -1039,11 +1056,12 @@ class P4Submit(Command, P4UserMap):\n if self.preserveUser:\n if p4User:\n # Get last changelist number. Cannot easily get it from\n- # the submit command output as the output is unmarshalled.\n+ # the submit command output as the output is\n+ # unmarshalled.\n changelist = self.lastP4Changelist()\n self.modifyChangelistUser(changelist, p4User)\n-\n else:\n+ # skip this patch\n for f in editedFiles:\n p4_revert(f)\n for f in filesToAdd:\ndiff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt\nindex 52003ae..5044a12 100644\n--- a/contrib/fast-import/git-p4.txt\n+++ b/contrib/fast-import/git-p4.txt\n@@ -202,11 +202,24 @@ able to find the relevant client. This client spec will be used to\n both filter the files cloned by git and set the directory layout as\n specified in the client (this implies --keep-path style semantics).\n \n-git-p4.skipSubmitModTimeCheck\n+git-p4.skipSubmitEdit\n \n- git config [--global] git-p4.skipSubmitModTimeCheck false\n+ git config [--global] git-p4.skipSubmitEdit false\n \n-If true, submit will not check if the p4 change template has been modified.\n+Normally, git-p4 invokes an editor after each commit is applied so\n+that you can make changes to the submit message. Setting this\n+variable to true will skip the editing step, submitting the change as is.\n+\n+git-p4.skipSubmitEditCheck\n+\n+ git config [--global] git-p4.skipSubmitEditCheck false\n+\n+After the editor is invoked, git-p4 normally makes sure you saved the\n+change description, as an indication that you did indeed read it over\n+and edit it. You can quit without saving to abort the submit (or skip\n+this change and continue). Setting this variable to true will cause\n+git-p4 not to check if you saved the change description. This variable\n+only matters if git-p4.skipSubmitEdit has not been set to true.\n \n git-p4.preserveUser\n \ndiff --git a/t/t9804-skip-submit-edit.sh b/t/t9804-skip-submit-edit.sh\nnew file mode 100755\nindex 0000000..734ccf2\n--- /dev/null\n+++ b/t/t9804-skip-submit-edit.sh\n@@ -0,0 +1,82 @@\n+#!/bin/sh\n+\n+test_description='git-p4 skipSubmitEdit config variables'\n+\n+. ./lib-git-p4.sh\n+\n+test_expect_success 'start p4d' '\n+\tstart_p4d\n+'\n+\n+test_expect_success 'init depot' '\n+\t(\n+\t\tcd \"$cli\" &&\n+\t\techo file1 >file1 &&\n+\t\tp4 add file1 &&\n+\t\tp4 submit -d \"change 1\"\n+\t)\n+'\n+\n+# this works because EDITOR is set to :\n+test_expect_success 'no config, unedited, say yes' '\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo line >>file1 &&\n+\t\tgit commit -a -m \"change 2\" &&\n+\t\techo y | \"$GITP4\" submit &&\n+\t\tp4 changes //depot/... >wc &&\n+\t\ttest_line_count = 2 wc\n+\t)\n+'\n+\n+test_expect_success 'no config, unedited, say no' '\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo line >>file1 &&\n+\t\tgit commit -a -m \"change 3 (not really)\" &&\n+\t\tprintf \"bad response\\nn\\n\" | \"$GITP4\" submit\n+\t\tp4 changes //depot/... >wc &&\n+\t\ttest_line_count = 2 wc\n+\t)\n+'\n+\n+test_expect_success 'skipSubmitEdit' '\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEdit true &&\n+\t\t# will fail if editor is even invoked\n+\t\tgit config core.editor /bin/false &&\n+\t\techo line >>file1 &&\n+\t\tgit commit -a -m \"change 3\" &&\n+\t\t\"$GITP4\" submit &&\n+\t\tp4 changes //depot/... >wc &&\n+\t\ttest_line_count = 3 wc\n+\t)\n+'\n+\n+test_expect_success 'skipSubmitEditCheck' '\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\tgit config git-p4.skipSubmitEditCheck true &&\n+\t\techo line >>file1 &&\n+\t\tgit commit -a -m \"change 4\" &&\n+\t\t\"$GITP4\" submit &&\n+\t\tp4 changes //depot/... >wc &&\n+\t\ttest_line_count = 4 wc\n+\t)\n+'\n+\n+\n+test_expect_success 'kill p4d' '\n+\tkill_p4d\n+'\n+\n+test_done\n-- \n1.7.7\n"},{"id":"177953","messageId":"1318956684.2717.7.camel@uncle-pecos","threadId":"28339","inReplyTo":"20111018004500.GA31768@arf.padd.com","subject":"Re: git-p4.skipSubmitEdit","fromName":"L. A. Linden Levy","fromEmail":"alevy@mobitv.com","sentAt":"2011-10-18T16:51:24Z","receivedAt":"2011-10-18T16:51:24Z","isPatch":false,"sender":{"key":"alevy@mobitv.com","avatar":null},"body":"Hi Peter,\n\nThis is just fine for me. Like I said initially I did not approach the\nproblem from an engineering perspective, I just changed it to prevent\nthe patch questions so I could commit faster. Looking forward to seeing\nthe update in the repo.\n\nCheers,\nAlex\n\nOn Mon, 2011-10-17 at 20:45 -0400, Pete Wyckoff wrote:\n> alevy@mobitv.com wrote on Mon, 12 Sep 2011 10:12 -0700:\n> > I agree with almost all your points. I have answered them each in-line\n> > below.\n> > \n> > On Mon, 2011-09-12 at 03:34 -0400, Luke Diamand wrote:\n> > > On 08/09/11 21:40, L. A. Linden Levy wrote:\n> > > > Hi All,\n> > > >\n> > > > I have been using git-p4 for a while and it has allowed me to completely\n> > > > change the way I develop and still be able to use perforce which my\n> > > > company has for its main VCS. One thing that was driving me nuts was\n> > > > that \"git p4 submit\" cycles through all of my individual commits and\n> > > > asks me if I want to change them. The way I develop I often am checking\n> > > > in 20 to 50 different small commits each with a descriptive git comment.\n> > > > I felt like I was doing double duty by having emacs open on every commit\n> > > > into perforce. So I modified git-p4 to have an option to skip the\n> > > > editor. This option coupled with git-p4.skipSubmitEditCheck will make\n> > > > the submission non-interactive for \"git p4 submit\".\n> > > \n> > > \n> > > Sorry - I've not had a chance to look at this before now. But a couple \n> > > of comments:\n> > > \n> > > - Is there a line wrap problem in the patch? It doesn't seem to want \n> > > to apply for me.\n> > \n> > Probably. Below are the results from following the patch submission\n> > instructions. \n> \n> Sorry I sat on this for a month. It is a good idea. Your\n> patches were good in content, but they didn't apply well due to\n> being line-wrapped and having one duplicate.\n> \n> Reading the code, though, I decided that the whole\n> skipSubmitEdit* checking was a bit convoluted even before you got\n> to it. So I moved it all to a separate function. And added a\n> unit test.\n> \n> Tell me if you think this is okay instead. If I got a bit too\n> wordy in the doc, please help with that too.\n> \n> \t\t-- Pete\n> \n> --- 8< ---\n> \n> Subject: [PATCH] git-p4: introduce skipSubmitEdit\n> \n> Add a configuration variable to skip invoking the editor in the\n> submit path.\n> \n> The existing variable skipSubmitEditCheck continues to make sure\n> that the submit template was indeed modified by the editor; but,\n> it is not considered is skipSubmitEdit is true.\n> \n> Reported-by: Loren A. Linden Levy \n> Signed-off-by: Pete Wyckoff \n> ---\n> contrib/fast-import/git-p4 | 60 +++++++++++++++++++----------\n> contrib/fast-import/git-p4.txt | 19 ++++++++-\n> t/t9804-skip-submit-edit.sh | 82 ++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 137 insertions(+), 24 deletions(-)\n> create mode 100755 t/t9804-skip-submit-edit.sh\n> \n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> index f885d70..abd6778 100755\n> --- a/contrib/fast-import/git-p4\n> +++ b/contrib/fast-import/git-p4\n> @@ -847,6 +847,39 @@ class P4Submit(Command, P4UserMap):\n> \n> return template\n> \n> + def edit_template(self, template_file):\n> + \"\"\"Invoke the editor to let the user change the submission\n> + message. Return true if okay to continue with the submit.\"\"\"\n> +\n> + # if configured to skip the editing part, just submit\n> + if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n> + return True\n> +\n> + # look at the modification time, to check later if the user saved\n> + # the file\n> + mtime = os.stat(template_file).st_mtime\n> +\n> + # invoke the editor\n> + if os.environ.has_key(\"P4EDITOR\"):\n> + editor = os.environ.get(\"P4EDITOR\")\n> + else:\n> + editor = read_pipe(\"git var GIT_EDITOR\").strip()\n> + system(editor + \" \" + template_file)\n> +\n> + # If the file was not saved, prompt to see if this patch should\n> + # be skipped. But skip this verification step if configured so.\n> + if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n> + print \"return true for skipSubmitEditCheck\"\n> + return True\n> +\n> + if os.stat(template_file).st_mtime <= mtime:\n> + while True:\n> + response = raw_input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n> + if response == 'y':\n> + return True\n> + if response == 'n':\n> + return False\n> +\n> def applyCommit(self, id):\n> print \"Applying %s\" % (read_pipe(\"git log --max-count=1 --pretty=oneline %s\" % id))\n> \n> @@ -1001,7 +1034,7 @@ class P4Submit(Command, P4UserMap):\n> \n> separatorLine = \"######## everything below this line is just the diff #######\\n\"\n> \n> - [handle, fileName] = tempfile.mkstemp()\n> + (handle, fileName) = tempfile.mkstemp()\n> tmpFile = os.fdopen(handle, \"w+\")\n> if self.isWindows:\n> submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n> @@ -1009,25 +1042,9 @@ class P4Submit(Command, P4UserMap):\n> newdiff = newdiff.replace(\"\\n\", \"\\r\\n\")\n> tmpFile.write(submitTemplate + separatorLine + diff + newdiff)\n> tmpFile.close()\n> - mtime = os.stat(fileName).st_mtime\n> - if os.environ.has_key(\"P4EDITOR\"):\n> - editor = os.environ.get(\"P4EDITOR\")\n> - else:\n> - editor = read_pipe(\"git var GIT_EDITOR\").strip()\n> - system(editor + \" \" + fileName)\n> \n> - if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n> - checkModTime = False\n> - else:\n> - checkModTime = True\n> -\n> - response = \"y\"\n> - if checkModTime and (os.stat(fileName).st_mtime <= mtime):\n> - response = \"x\"\n> - while response != \"y\" and response != \"n\":\n> - response = raw_input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n> -\n> - if response == \"y\":\n> + if self.edit_template(fileName):\n> + # read the edited message and submit\n> tmpFile = open(fileName, \"rb\")\n> message = tmpFile.read()\n> tmpFile.close()\n> @@ -1039,11 +1056,12 @@ class P4Submit(Command, P4UserMap):\n> if self.preserveUser:\n> if p4User:\n> # Get last changelist number. Cannot easily get it from\n> - # the submit command output as the output is unmarshalled.\n> + # the submit command output as the output is\n> + # unmarshalled.\n> changelist = self.lastP4Changelist()\n> self.modifyChangelistUser(changelist, p4User)\n> -\n> else:\n> + # skip this patch\n> for f in editedFiles:\n> p4_revert(f)\n> for f in filesToAdd:\n> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt\n> index 52003ae..5044a12 100644\n> --- a/contrib/fast-import/git-p4.txt\n> +++ b/contrib/fast-import/git-p4.txt\n> @@ -202,11 +202,24 @@ able to find the relevant client. This client spec will be used to\n> both filter the files cloned by git and set the directory layout as\n> specified in the client (this implies --keep-path style semantics).\n> \n> -git-p4.skipSubmitModTimeCheck\n> +git-p4.skipSubmitEdit\n> \n> - git config [--global] git-p4.skipSubmitModTimeCheck false\n> + git config [--global] git-p4.skipSubmitEdit false\n> \n> -If true, submit will not check if the p4 change template has been modified.\n> +Normally, git-p4 invokes an editor after each commit is applied so\n> +that you can make changes to the submit message. Setting this\n> +variable to true will skip the editing step, submitting the change as is.\n> +\n> +git-p4.skipSubmitEditCheck\n> +\n> + git config [--global] git-p4.skipSubmitEditCheck false\n> +\n> +After the editor is invoked, git-p4 normally makes sure you saved the\n> +change description, as an indication that you did indeed read it over\n> +and edit it. You can quit without saving to abort the submit (or skip\n> +this change and continue). Setting this variable to true will cause\n> +git-p4 not to check if you saved the change description. This variable\n> +only matters if git-p4.skipSubmitEdit has not been set to true.\n> \n> git-p4.preserveUser\n> \n> diff --git a/t/t9804-skip-submit-edit.sh b/t/t9804-skip-submit-edit.sh\n> new file mode 100755\n> index 0000000..734ccf2\n> --- /dev/null\n> +++ b/t/t9804-skip-submit-edit.sh\n> @@ -0,0 +1,82 @@\n> +#!/bin/sh\n> +\n> +test_description='git-p4 skipSubmitEdit config variables'\n> +\n> +. ./lib-git-p4.sh\n> +\n> +test_expect_success 'start p4d' '\n> +\tstart_p4d\n> +'\n> +\n> +test_expect_success 'init depot' '\n> +\t(\n> +\t\tcd \"$cli\" &&\n> +\t\techo file1 >file1 &&\n> +\t\tp4 add file1 &&\n> +\t\tp4 submit -d \"change 1\"\n> +\t)\n> +'\n> +\n> +# this works because EDITOR is set to :\n> +test_expect_success 'no config, unedited, say yes' '\n> +\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\techo line >>file1 &&\n> +\t\tgit commit -a -m \"change 2\" &&\n> +\t\techo y | \"$GITP4\" submit &&\n> +\t\tp4 changes //depot/... >wc &&\n> +\t\ttest_line_count = 2 wc\n> +\t)\n> +'\n> +\n> +test_expect_success 'no config, unedited, say no' '\n> +\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\techo line >>file1 &&\n> +\t\tgit commit -a -m \"change 3 (not really)\" &&\n> +\t\tprintf \"bad response\\nn\\n\" | \"$GITP4\" submit\n> +\t\tp4 changes //depot/... >wc &&\n> +\t\ttest_line_count = 2 wc\n> +\t)\n> +'\n> +\n> +test_expect_success 'skipSubmitEdit' '\n> +\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\tgit config git-p4.skipSubmitEdit true &&\n> +\t\t# will fail if editor is even invoked\n> +\t\tgit config core.editor /bin/false &&\n> +\t\techo line >>file1 &&\n> +\t\tgit commit -a -m \"change 3\" &&\n> +\t\t\"$GITP4\" submit &&\n> +\t\tp4 changes //depot/... >wc &&\n> +\t\ttest_line_count = 3 wc\n> +\t)\n> +'\n> +\n> +test_expect_success 'skipSubmitEditCheck' '\n> +\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n> +\ttest_when_finished cleanup_git &&\n> +\t(\n> +\t\tcd \"$git\" &&\n> +\t\tgit config git-p4.skipSubmitEditCheck true &&\n> +\t\techo line >>file1 &&\n> +\t\tgit commit -a -m \"change 4\" &&\n> +\t\t\"$GITP4\" submit &&\n> +\t\tp4 changes //depot/... >wc &&\n> +\t\ttest_line_count = 4 wc\n> +\t)\n> +'\n> +\n> +\n> +test_expect_success 'kill p4d' '\n> +\tkill_p4d\n> +'\n> +\n> +test_done\n\n-- \nAlex Linden Levy\nSenior Software Engineer\nMobiTV, Inc.\n6425 Christie Avenue, 5th Floor, Emeryville, CA 94608\nphone 510.450.5190 mobile 720.352.8394\nemail alevy@mobitv.com web www.mobitv.com\n\n"},{"id":"177956","messageId":"20111018173518.GA14629@padd.com","threadId":"28339","inReplyTo":"1318956684.2717.7.camel@uncle-pecos","subject":"Re: git-p4.skipSubmitEdit","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-10-18T17:35:18Z","receivedAt":"2011-10-18T17:35:18Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"alevy@mobitv.com wrote on Tue, 18 Oct 2011 09:51 -0700:\n> This is just fine for me. Like I said initially I did not approach the\n> problem from an engineering perspective, I just changed it to prevent\n> the patch questions so I could commit faster. Looking forward to seeing\n> the update in the repo.\n\nOkay, thanks. I'll see if I get some review comments, and will\ngradually shepherd this toward the next release.\n\n\t\t-- Pete\n"},{"id":"177957","messageId":"4E9DBD0B.7020505@diamand.org","threadId":"28339","inReplyTo":"20111018004500.GA31768@arf.padd.com","subject":"Re: git-p4.skipSubmitEdit","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-10-18T17:53:15Z","receivedAt":"2011-10-18T17:53:15Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 18/10/11 01:45, Pete Wyckoff wrote:\n> alevy@mobitv.com wrote on Mon, 12 Sep 2011 10:12 -0700:\n>> I agree with almost all your points. I have answered them each in-line\n>> below.\n>>\n>> On Mon, 2011-09-12 at 03:34 -0400, Luke Diamand wrote:\n>>> On 08/09/11 21:40, L. A. Linden Levy wrote:\n>>>> Hi All,\n>>>>\n>>>> I have been using git-p4 for a while and it has allowed me to completely\n>>>> change the way I develop and still be able to use perforce which my\n>>>> company has for its main VCS. One thing that was driving me nuts was\n>>>> that \"git p4 submit\" cycles through all of my individual commits and\n>>>> asks me if I want to change them. The way I develop I often am checking\n>>>> in 20 to 50 different small commits each with a descriptive git comment.\n>>>> I felt like I was doing double duty by having emacs open on every commit\n>>>> into perforce. So I modified git-p4 to have an option to skip the\n>>>> editor. This option coupled with git-p4.skipSubmitEditCheck will make\n>>>> the submission non-interactive for \"git p4 submit\".\n>>>\n>>>\n>>> Sorry - I've not had a chance to look at this before now. But a couple\n>>> of comments:\n>>>\n>>> - Is there a line wrap problem in the patch? It doesn't seem to want\n>>> to apply for me.\n>>\n>> Probably. Below are the results from following the patch submission\n>> instructions.\n>\n> Sorry I sat on this for a month. It is a good idea. Your\n> patches were good in content, but they didn't apply well due to\n> being line-wrapped and having one duplicate.\n>\n> Reading the code, though, I decided that the whole\n> skipSubmitEdit* checking was a bit convoluted even before you got\n> to it. So I moved it all to a separate function. And added a\n> unit test.\n>\n> Tell me if you think this is okay instead. If I got a bit too\n> wordy in the doc, please help with that too.\n\nLooks good, one minor nit (see below) and a comment.\n\nAck.\n\nLuke\n\n\n>\n> \t\t-- Pete\n>\n> --- 8< ---\n>\n> Subject: [PATCH] git-p4: introduce skipSubmitEdit\n>\n> Add a configuration variable to skip invoking the editor in the\n> submit path.\n>\n> The existing variable skipSubmitEditCheck continues to make sure\n> that the submit template was indeed modified by the editor; but,\n> it is not considered is skipSubmitEdit is true.\n>\n> Reported-by: Loren A. Linden Levy\n> Signed-off-by: Pete Wyckoff\n> ---\n> contrib/fast-import/git-p4 | 60 +++++++++++++++++++----------\n> contrib/fast-import/git-p4.txt | 19 ++++++++-\n> t/t9804-skip-submit-edit.sh | 82 ++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 137 insertions(+), 24 deletions(-)\n> create mode 100755 t/t9804-skip-submit-edit.sh\n>\n> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\n> index f885d70..abd6778 100755\n> --- a/contrib/fast-import/git-p4\n> +++ b/contrib/fast-import/git-p4\n> @@ -847,6 +847,39 @@ class P4Submit(Command, P4UserMap):\n>\n> return template\n>\n> + def edit_template(self, template_file):\n> + \"\"\"Invoke the editor to let the user change the submission\n> + message. Return true if okay to continue with the submit.\"\"\"\n> +\n> + # if configured to skip the editing part, just submit\n> + if gitConfig(\"git-p4.skipSubmitEdit\") == \"true\":\n> + return True\n> +\n> + # look at the modification time, to check later if the user saved\n> + # the file\n> + mtime = os.stat(template_file).st_mtime\n> +\n> + # invoke the editor\n> + if os.environ.has_key(\"P4EDITOR\"):\n> + editor = os.environ.get(\"P4EDITOR\")\n> + else:\n> + editor = read_pipe(\"git var GIT_EDITOR\").strip()\n> + system(editor + \" \" + template_file)\n\nThis is where we should really check the return code. However, doing so \nseems to break lots of the existing tests so it's not as easy as it looks.\n\n> +\n> + # If the file was not saved, prompt to see if this patch should\n> + # be skipped. But skip this verification step if configured so.\n> + if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n> + print \"return true for skipSubmitEditCheck\"\n\nYou print a helpful/annoying(?) message here, but not further up at \nskipSubmitEdit?\n\n\n> + return True\n> +\n> + if os.stat(template_file).st_mtime<= mtime:\n> + while True:\n> + response = raw_input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n> + if response == 'y':\n> + return True\n> + if response == 'n':\n> + return False\n> +\n> def applyCommit(self, id):\n> print \"Applying %s\" % (read_pipe(\"git log --max-count=1 --pretty=oneline %s\" % id))\n>\n> @@ -1001,7 +1034,7 @@ class P4Submit(Command, P4UserMap):\n>\n> separatorLine = \"######## everything below this line is just the diff #######\\n\"\n>\n> - [handle, fileName] = tempfile.mkstemp()\n> + (handle, fileName) = tempfile.mkstemp()\n> tmpFile = os.fdopen(handle, \"w+\")\n> if self.isWindows:\n> submitTemplate = submitTemplate.replace(\"\\n\", \"\\r\\n\")\n> @@ -1009,25 +1042,9 @@ class P4Submit(Command, P4UserMap):\n> newdiff = newdiff.replace(\"\\n\", \"\\r\\n\")\n> tmpFile.write(submitTemplate + separatorLine + diff + newdiff)\n> tmpFile.close()\n> - mtime = os.stat(fileName).st_mtime\n> - if os.environ.has_key(\"P4EDITOR\"):\n> - editor = os.environ.get(\"P4EDITOR\")\n> - else:\n> - editor = read_pipe(\"git var GIT_EDITOR\").strip()\n> - system(editor + \" \" + fileName)\n>\n> - if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n> - checkModTime = False\n> - else:\n> - checkModTime = True\n> -\n> - response = \"y\"\n> - if checkModTime and (os.stat(fileName).st_mtime<= mtime):\n> - response = \"x\"\n> - while response != \"y\" and response != \"n\":\n> - response = raw_input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n> -\n> - if response == \"y\":\n> + if self.edit_template(fileName):\n> + # read the edited message and submit\n> tmpFile = open(fileName, \"rb\")\n> message = tmpFile.read()\n> tmpFile.close()\n> @@ -1039,11 +1056,12 @@ class P4Submit(Command, P4UserMap):\n> if self.preserveUser:\n> if p4User:\n> # Get last changelist number. Cannot easily get it from\n> - # the submit command output as the output is unmarshalled.\n> + # the submit command output as the output is\n> + # unmarshalled.\n> changelist = self.lastP4Changelist()\n> self.modifyChangelistUser(changelist, p4User)\n> -\n> else:\n> + # skip this patch\n> for f in editedFiles:\n> p4_revert(f)\n> for f in filesToAdd:\n> diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt\n> index 52003ae..5044a12 100644\n> --- a/contrib/fast-import/git-p4.txt\n> +++ b/contrib/fast-import/git-p4.txt\n> @@ -202,11 +202,24 @@ able to find the relevant client. This client spec will be used to\n> both filter the files cloned by git and set the directory layout as\n> specified in the client (this implies --keep-path style semantics).\n>\n> -git-p4.skipSubmitModTimeCheck\n> +git-p4.skipSubmitEdit\n>\n> - git config [--global] git-p4.skipSubmitModTimeCheck false\n> + git config [--global] git-p4.skipSubmitEdit false\n>\n> -If true, submit will not check if the p4 change template has been modified.\n> +Normally, git-p4 invokes an editor after each commit is applied so\n> +that you can make changes to the submit message. Setting this\n> +variable to true will skip the editing step, submitting the change as is.\n> +\n> +git-p4.skipSubmitEditCheck\n> +\n> + git config [--global] git-p4.skipSubmitEditCheck false\n> +\n> +After the editor is invoked, git-p4 normally makes sure you saved the\n> +change description, as an indication that you did indeed read it over\n> +and edit it. You can quit without saving to abort the submit (or skip\n> +this change and continue). Setting this variable to true will cause\n> +git-p4 not to check if you saved the change description. This variable\n> +only matters if git-p4.skipSubmitEdit has not been set to true.\n>\n> git-p4.preserveUser\n>\n> diff --git a/t/t9804-skip-submit-edit.sh b/t/t9804-skip-submit-edit.sh\n> new file mode 100755\n> index 0000000..734ccf2\n> --- /dev/null\n> +++ b/t/t9804-skip-submit-edit.sh\n> @@ -0,0 +1,82 @@\n> +#!/bin/sh\n> +\n> +test_description='git-p4 skipSubmitEdit config variables'\n> +\n> +. ./lib-git-p4.sh\n> +\n> +test_expect_success 'start p4d' '\n> +\tstart_p4d\n> +'\n> +\n> +test_expect_success 'init depot' '\n> +\t(\n> +\t\tcd \"$cli\"&&\n> +\t\techo file1>file1&&\n> +\t\tp4 add file1&&\n> +\t\tp4 submit -d \"change 1\"\n> +\t)\n> +'\n> +\n> +# this works because EDITOR is set to :\n> +test_expect_success 'no config, unedited, say yes' '\n> +\t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n> +\t(\n> +\t\tcd \"$git\"&&\n> +\t\techo line>>file1&&\n> +\t\tgit commit -a -m \"change 2\"&&\n> +\t\techo y | \"$GITP4\" submit&&\n> +\t\tp4 changes //depot/...>wc&&\n> +\t\ttest_line_count = 2 wc\n> +\t)\n> +'\n> +\n> +test_expect_success 'no config, unedited, say no' '\n> +\t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n> +\t(\n> +\t\tcd \"$git\"&&\n> +\t\techo line>>file1&&\n> +\t\tgit commit -a -m \"change 3 (not really)\"&&\n> +\t\tprintf \"bad response\\nn\\n\" | \"$GITP4\" submit\n> +\t\tp4 changes //depot/...>wc&&\n> +\t\ttest_line_count = 2 wc\n> +\t)\n> +'\n> +\n> +test_expect_success 'skipSubmitEdit' '\n> +\t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n> +\t(\n> +\t\tcd \"$git\"&&\n> +\t\tgit config git-p4.skipSubmitEdit true&&\n> +\t\t# will fail if editor is even invoked\n> +\t\tgit config core.editor /bin/false&&\n> +\t\techo line>>file1&&\n> +\t\tgit commit -a -m \"change 3\"&&\n> +\t\t\"$GITP4\" submit&&\n> +\t\tp4 changes //depot/...>wc&&\n> +\t\ttest_line_count = 3 wc\n> +\t)\n> +'\n> +\n> +test_expect_success 'skipSubmitEditCheck' '\n> +\t\"$GITP4\" clone --dest=\"$git\" //depot&&\n> +\ttest_when_finished cleanup_git&&\n> +\t(\n> +\t\tcd \"$git\"&&\n> +\t\tgit config git-p4.skipSubmitEditCheck true&&\n> +\t\techo line>>file1&&\n> +\t\tgit commit -a -m \"change 4\"&&\n> +\t\t\"$GITP4\" submit&&\n> +\t\tp4 changes //depot/...>wc&&\n> +\t\ttest_line_count = 4 wc\n> +\t)\n> +'\n> +\n> +\n> +test_expect_success 'kill p4d' '\n> +\tkill_p4d\n> +'\n> +\n> +test_done\n"},{"id":"178061","messageId":"20111020011610.GA7292@arf.padd.com","threadId":"28339","inReplyTo":"4E9DBD0B.7020505@diamand.org","subject":"Re: git-p4.skipSubmitEdit","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-10-20T01:16:10Z","receivedAt":"2011-10-20T01:16:10Z","isPatch":false,"sender":{"key":"pw@padd.com","avatar":null},"body":"luke@diamand.org wrote on Tue, 18 Oct 2011 18:53 +0100:\n> Looks good, one minor nit (see below) and a comment.\n[..]\n> >+ # invoke the editor\n> >+ if os.environ.has_key(\"P4EDITOR\"):\n> >+ editor = os.environ.get(\"P4EDITOR\")\n> >+ else:\n> >+ editor = read_pipe(\"git var GIT_EDITOR\").strip()\n> >+ system(editor + \" \" + template_file)\n> \n> This is where we should really check the return code. However, doing\n> so seems to break lots of the existing tests so it's not as easy as\n> it looks.\n\nIndeed. I'll not fix that now, but agree it should be.\n\n> >+\n> >+ # If the file was not saved, prompt to see if this patch should\n> >+ # be skipped. But skip this verification step if configured so.\n> >+ if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n> >+ print \"return true for skipSubmitEditCheck\"\n> \n> You print a helpful/annoying(?) message here, but not further up at\n> skipSubmitEdit?\n\nAargh. Leaked debug code. Thanks for noticing. I got rid of\nit.\n\n\t\t-- Pete\n"},{"id":"181300","messageId":"CAFLRboo8DBk3zFEBF9OqKmre=d5PM7+3J9V0pHNz53MPtqjOWA@mail.gmail.com","threadId":"28339","inReplyTo":"20111020011610.GA7292@arf.padd.com","subject":"Re: git-p4.skipSubmitEdit","fromName":"Michael Horowitz","fromEmail":"michael.horowitz@ieee.org","sentAt":"2011-12-16T15:38:58Z","receivedAt":"2011-12-16T15:38:58Z","isPatch":false,"sender":{"key":"michael.horowitz@ieee.org","avatar":null},"body":"All,\n\nIt appears that this change has introduced a bug that causes submit to\nfail every time if you do not skip the submit edit.\n\nFrom what I can tell, this is because the new edit_template method\ndoes not return True at the end.\n\nThanks,\n\nMike\n\n\n\nOn Wed, Oct 19, 2011 at 9:16 PM, Pete Wyckoff wrote:\n> luke@diamand.org wrote on Tue, 18 Oct 2011 18:53 +0100:\n>> Looks good, one minor nit (see below) and a comment.\n> [..]\n>> >+        # invoke the editor\n>> >+        if os.environ.has_key(\"P4EDITOR\"):\n>> >+            editor = os.environ.get(\"P4EDITOR\")\n>> >+        else:\n>> >+            editor = read_pipe(\"git var GIT_EDITOR\").strip()\n>> >+        system(editor + \" \" + template_file)\n>>\n>> This is where we should really check the return code. However, doing\n>> so seems to break lots of the existing tests so it's not as easy as\n>> it looks.\n>\n> Indeed.  I'll not fix that now, but agree it should be.\n>\n>> >+\n>> >+        # If the file was not saved, prompt to see if this patch should\n>> >+        # be skipped.  But skip this verification step if configured so.\n>> >+        if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n>> >+            print \"return true for skipSubmitEditCheck\"\n>>\n>> You print a helpful/annoying(?) message here, but not further up at\n>> skipSubmitEdit?\n>\n> Aargh.  Leaked debug code.  Thanks for noticing.  I got rid of\n> it.\n>\n>                -- Pete\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"181328","messageId":"4EEBA106.9010001@diamand.org","threadId":"28339","inReplyTo":"CAFLRboo8DBk3zFEBF9OqKmre=d5PM7+3J9V0pHNz53MPtqjOWA@mail.gmail.com","subject":"Re: git-p4.skipSubmitEdit","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2011-12-16T19:50:30Z","receivedAt":"2011-12-16T19:50:30Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 16/12/11 15:38, Michael Horowitz wrote:\n> All,\n>\n> It appears that this change has introduced a bug that causes submit to\n> fail every time if you do not skip the submit edit.\n>\n> From what I can tell, this is because the new edit_template method\n> does not return True at the end.\n\nCould you say exactly what you're doing?\n\nI've just tried it myself and it seems to work fine:\n\ngit-p4 clone\ngit commit -m 'a change'\ngit-p4 submit\n\n\nAnd I couldn't see any paths in edit_template that returned without a \nvalue of True, except the one where the user decides to bail out.\n\nThis is with Pete's skipSubmitEdit change.\n\nThanks!\nLuke\n"},{"id":"181356","messageId":"CAFLRbor3Gnqhudmg8G_U37Nbo7ENoCEy0iFVGRP4i_AmatJHxw@mail.gmail.com","threadId":"28339","inReplyTo":"4EEBA106.9010001@diamand.org","subject":"Re: git-p4.skipSubmitEdit","fromName":"Michael Horowitz","fromEmail":"michael.horowitz@ieee.org","sentAt":"2011-12-17T00:46:11Z","receivedAt":"2011-12-17T00:46:11Z","isPatch":false,"sender":{"key":"michael.horowitz@ieee.org","avatar":null},"body":"Actually, it is the opposite. Bailout works fine, it is when I \":wq\"\nin Vi for example, that it fails to submit and reverts all my changes.\n\n if os.stat(template_file).st_mtime <= mtime:\n while True:\n response = raw_input(\"Submit template unchanged.\nSubmit anyway? [y]es, [n]o (skip this patch) \")\n if response == 'y':\n return True\n if response == 'n':\n return False\n # I think this else needs to be added here, so when the file\nhas been modified since it was opened in the editor, it will properly\nsubmit the change.\n else:\n return True\n\n\nMike\n\n\n\nOn Fri, Dec 16, 2011 at 2:50 PM, Luke Diamand wrote:\n>\n> On 16/12/11 15:38, Michael Horowitz wrote:\n>>\n>> All,\n>>\n>> It appears that this change has introduced a bug that causes submit to\n>> fail every time if you do not skip the submit edit.\n>>\n>>  From what I can tell, this is because the new edit_template method\n>> does not return True at the end.\n>\n>\n> Could you say exactly what you're doing?\n>\n> I've just tried it myself and it seems to work fine:\n>\n> git-p4 clone\n> git commit -m 'a change'\n> git-p4 submit\n> \n>\n> And I couldn't see any paths in edit_template that returned without a value of True, except the one where the user decides to bail out.\n>\n> This is with Pete's skipSubmitEdit change.\n>\n> Thanks!\n> Luke\n"},{"id":"181358","messageId":"CAFLRboqJAC0h27m=B9Tw5SFcupEgn4fe9YvWksgqxOVs20nFHw@mail.gmail.com","threadId":"28339","inReplyTo":"CAFLRbor3Gnqhudmg8G_U37Nbo7ENoCEy0iFVGRP4i_AmatJHxw@mail.gmail.com","subject":"Re: git-p4.skipSubmitEdit","fromName":"Michael Horowitz","fromEmail":"michael.horowitz@ieee.org","sentAt":"2011-12-17T00:49:17Z","receivedAt":"2011-12-17T00:49:17Z","isPatch":false,"sender":{"key":"michael.horowitz@ieee.org","avatar":null},"body":"Oh, and in the case where you do edit the template, it doesn't give\nyou an error or anything, it looks like it succeeded, but you'll\nnotice the change never got submitted to Perforce. If you look\ncarefully though, you can see it reverting each of your edited files\nin the P4 tree.\n\nMike\n\n\n\nOn Fri, Dec 16, 2011 at 7:46 PM, Michael Horowitz\n wrote:\n> Actually, it is the opposite.  Bailout works fine, it is when I \":wq\"\n> in Vi for example, that it fails to submit and reverts all my changes.\n>\n>        if os.stat(template_file).st_mtime <= mtime:\n>            while True:\n>                response = raw_input(\"Submit template unchanged.\n> Submit anyway? [y]es, [n]o (skip this patch) \")\n>                if response == 'y':\n>                    return True\n>                if response == 'n':\n>                    return False\n>        # I think this else needs to be added here, so when the file\n> has been modified since it was opened in the editor, it will properly\n> submit the change.\n>        else:\n>            return True\n>\n>\n> Mike\n>\n>\n>\n> On Fri, Dec 16, 2011 at 2:50 PM, Luke Diamand wrote:\n>>\n>> On 16/12/11 15:38, Michael Horowitz wrote:\n>>>\n>>> All,\n>>>\n>>> It appears that this change has introduced a bug that causes submit to\n>>> fail every time if you do not skip the submit edit.\n>>>\n>>>  From what I can tell, this is because the new edit_template method\n>>> does not return True at the end.\n>>\n>>\n>> Could you say exactly what you're doing?\n>>\n>> I've just tried it myself and it seems to work fine:\n>>\n>> git-p4 clone\n>> git commit -m 'a change'\n>> git-p4 submit\n>> \n>>\n>> And I couldn't see any paths in edit_template that returned without a value of True, except the one where the user decides to bail out.\n>>\n>> This is with Pete's skipSubmitEdit change.\n>>\n>> Thanks!\n>> Luke\n"},{"id":"181402","messageId":"20111217173903.GA13674@padd.com","threadId":"28339","inReplyTo":"CAFLRboqJAC0h27m=B9Tw5SFcupEgn4fe9YvWksgqxOVs20nFHw@mail.gmail.com","subject":"[PATCH] git-p4: fix skipSubmitEdit regression","fromName":"Pete Wyckoff","fromEmail":"pw@padd.com","sentAt":"2011-12-17T17:39:03Z","receivedAt":"2011-12-17T17:39:03Z","isPatch":true,"sender":{"key":"pw@padd.com","avatar":null},"body":"Commit 7c766e5 (git-p4: introduce skipSubmitEdit, 2011-12-04)\nmade it easier to automate submission to p4, but broke the most\ncommon case.\n\nAdd a test for when the user really does edit and save the change\ntemplate, and fix the bug that causes the test to fail.\n\nAlso add a confirmation message when submission is cancelled.\n\nReported-by: Michael Horowitz \nSigned-off-by: Pete Wyckoff \n---\nmichael.horowitz@ieee.org wrote on Fri, 16 Dec 2011 19:49 -0500:\n> Oh, and in the case where you do edit the template, it doesn't give\n> you an error or anything, it looks like it succeeded, but you'll\n> notice the change never got submitted to Perforce. If you look\n> carefully though, you can see it reverting each of your edited files\n> in the P4 tree.\n[..]\n> >> On 16/12/11 15:38, Michael Horowitz wrote:\n> >>>\n> >>> It appears that this change has introduced a bug that causes submit to\n> >>> fail every time if you do not skip the submit edit.\n> >>>\n> >>> From what I can tell, this is because the new edit_template method\n> >>> does not return True at the end.\n\nOops. In adding this code, I failed to test what should be the\nnormal code path. How sad.\n\nJunio: this bug is in master. Could you apply this fix?\n\n\t\t-- Pete\n\n contrib/fast-import/git-p4 | 18 +++++++++++-------\n t/t9805-skip-submit-edit.sh | 24 +++++++++++++++++++++++-\n 2 files changed, 34 insertions(+), 8 deletions(-)\n\ndiff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4\nindex 3571362..d501eac 100755\n--- a/contrib/fast-import/git-p4\n+++ b/contrib/fast-import/git-p4\n@@ -878,13 +878,16 @@ class P4Submit(Command, P4UserMap):\n if gitConfig(\"git-p4.skipSubmitEditCheck\") == \"true\":\n return True\n \n- if os.stat(template_file).st_mtime <= mtime:\n- while True:\n- response = raw_input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n- if response == 'y':\n- return True\n- if response == 'n':\n- return False\n+ # modification time updated means user saved the file\n+ if os.stat(template_file).st_mtime > mtime:\n+ return True\n+\n+ while True:\n+ response = raw_input(\"Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) \")\n+ if response == 'y':\n+ return True\n+ if response == 'n':\n+ return False\n \n def applyCommit(self, id):\n print \"Applying %s\" % (read_pipe(\"git log --max-count=1 --pretty=oneline %s\" % id))\n@@ -1068,6 +1071,7 @@ class P4Submit(Command, P4UserMap):\n self.modifyChangelistUser(changelist, p4User)\n else:\n # skip this patch\n+ print \"Submission cancelled, undoing p4 changes.\"\n for f in editedFiles:\n p4_revert(f)\n for f in filesToAdd:\ndiff --git a/t/t9805-skip-submit-edit.sh b/t/t9805-skip-submit-edit.sh\nindex 734ccf2..df929e0 100755\n--- a/t/t9805-skip-submit-edit.sh\n+++ b/t/t9805-skip-submit-edit.sh\n@@ -38,7 +38,7 @@ test_expect_success 'no config, unedited, say no' '\n \t\tcd \"$git\" &&\n \t\techo line >>file1 &&\n \t\tgit commit -a -m \"change 3 (not really)\" &&\n-\t\tprintf \"bad response\\nn\\n\" | \"$GITP4\" submit\n+\t\tprintf \"bad response\\nn\\n\" | \"$GITP4\" submit &&\n \t\tp4 changes //depot/... >wc &&\n \t\ttest_line_count = 2 wc\n \t)\n@@ -74,6 +74,28 @@ test_expect_success 'skipSubmitEditCheck' '\n \t)\n '\n \n+# check the normal case, where the template really is edited\n+test_expect_success 'no config, edited' '\n+\t\"$GITP4\" clone --dest=\"$git\" //depot &&\n+\ttest_when_finished cleanup_git &&\n+\ted=\"$TRASH_DIRECTORY/ed.sh\" &&\n+\ttest_when_finished \"rm \\\"$ed\\\"\" &&\n+\tcat >\"$ed\" <<-EOF &&\n+\t\t#!$SHELL_PATH\n+\t\tsleep 1\n+\t\ttouch \"\\$1\"\n+\t\texit 0\n+\tEOF\n+\tchmod 755 \"$ed\" &&\n+\t(\n+\t\tcd \"$git\" &&\n+\t\techo line >>file1 &&\n+\t\tgit commit -a -m \"change 5\" &&\n+\t\tEDITOR=\"\\\"$ed\\\"\" \"$GITP4\" submit &&\n+\t\tp4 changes //depot/... >wc &&\n+\t\ttest_line_count = 5 wc\n+\t)\n+'\n \n test_expect_success 'kill p4d' '\n \tkill_p4d\n-- \n1.7.8.154.g767b7.dirty\n"}]}