alevy@xxxxxxxxxx wrote on Mon, 12 Sep 2011 10:12 -0700:
> I agree with almost all your points. I have answered them each in-line
> below.
>
> On Mon, 2011-09-12 at 03:34 -0400, Luke Diamand wrote:
> > On 08/09/11 21:40, L. A. Linden Levy wrote:
> > > Hi All,
> > >
> > > I have been using git-p4 for a while and it has allowed me to completely
> > > change the way I develop and still be able to use perforce which my
> > > company has for its main VCS. One thing that was driving me nuts was
> > > that "git p4 submit" cycles through all of my individual commits and
> > > asks me if I want to change them. The way I develop I often am checking
> > > in 20 to 50 different small commits each with a descriptive git comment.
> > > I felt like I was doing double duty by having emacs open on every commit
> > > into perforce. So I modified git-p4 to have an option to skip the
> > > editor. This option coupled with git-p4.skipSubmitEditCheck will make
> > > the submission non-interactive for "git p4 submit".
> >
> >
> > Sorry - I've not had a chance to look at this before now. But a couple
> > of comments:
> >
> > - Is there a line wrap problem in the patch? It doesn't seem to want
> > to apply for me.
>
> Probably. Below are the results from following the patch submission
> instructions.
Sorry I sat on this for a month. It is a good idea. Your
patches were good in content, but they didn't apply well due to
being line-wrapped and having one duplicate.
Reading the code, though, I decided that the whole
skipSubmitEdit* checking was a bit convoluted even before you got
to it. So I moved it all to a separate function. And added a
unit test.
Tell me if you think this is okay instead. If I got a bit too
wordy in the doc, please help with that too.
-- Pete
--- 8< ---
Subject: [PATCH] git-p4: introduce skipSubmitEdit
Add a configuration variable to skip invoking the editor in the
submit path.
The existing variable skipSubmitEditCheck continues to make sure
that the submit template was indeed modified by the editor; but,
it is not considered is skipSubmitEdit is true.
Reported-by: Loren A. Linden Levy <lindenle@xxxxxxxxx>
Signed-off-by: Pete Wyckoff <pw@xxxxxxxx>
---
contrib/fast-import/git-p4 | 60 +++++++++++++++++++----------
contrib/fast-import/git-p4.txt | 19 ++++++++-
t/t9804-skip-submit-edit.sh | 82 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 137 insertions(+), 24 deletions(-)
create mode 100755 t/t9804-skip-submit-edit.sh
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index f885d70..abd6778 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -847,6 +847,39 @@ class P4Submit(Command, P4UserMap):
return template
+ def edit_template(self, template_file):
+ """Invoke the editor to let the user change the submission
+ message. Return true if okay to continue with the submit."""
+
+ # if configured to skip the editing part, just submit
+ if gitConfig("git-p4.skipSubmitEdit") == "true":
+ return True
+
+ # look at the modification time, to check later if the user saved
+ # the file
+ mtime = os.stat(template_file).st_mtime
+
+ # invoke the editor
+ if os.environ.has_key("P4EDITOR"):
+ editor = os.environ.get("P4EDITOR")
+ else:
+ editor = read_pipe("git var GIT_EDITOR").strip()
+ system(editor + " " + template_file)
+
+ # If the file was not saved, prompt to see if this patch should
+ # be skipped. But skip this verification step if configured so.
+ if gitConfig("git-p4.skipSubmitEditCheck") == "true":
+ print "return true for skipSubmitEditCheck"
+ return True
+
+ if os.stat(template_file).st_mtime <= mtime:
+ while True:
+ response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
+ if response == 'y':
+ return True
+ if response == 'n':
+ return False
+
def applyCommit(self, id):
print "Applying %s" % (read_pipe("git log --max-count=1 --pretty=oneline %s" % id))
@@ -1001,7 +1034,7 @@ class P4Submit(Command, P4UserMap):
separatorLine = "######## everything below this line is just the diff #######\n"
- [handle, fileName] = tempfile.mkstemp()
+ (handle, fileName) = tempfile.mkstemp()
tmpFile = os.fdopen(handle, "w+")
if self.isWindows:
submitTemplate = submitTemplate.replace("\n", "\r\n")
@@ -1009,25 +1042,9 @@ class P4Submit(Command, P4UserMap):
newdiff = newdiff.replace("\n", "\r\n")
tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
tmpFile.close()
- mtime = os.stat(fileName).st_mtime
- if os.environ.has_key("P4EDITOR"):
- editor = os.environ.get("P4EDITOR")
- else:
- editor = read_pipe("git var GIT_EDITOR").strip()
- system(editor + " " + fileName)
- if gitConfig("git-p4.skipSubmitEditCheck") == "true":
- checkModTime = False
- else:
- checkModTime = True
-
- response = "y"
- if checkModTime and (os.stat(fileName).st_mtime <= mtime):
- response = "x"
- while response != "y" and response != "n":
- response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
-
- if response == "y":
+ if self.edit_template(fileName):
+ # read the edited message and submit
tmpFile = open(fileName, "rb")
message = tmpFile.read()
tmpFile.close()
@@ -1039,11 +1056,12 @@ class P4Submit(Command, P4UserMap):
if self.preserveUser:
if p4User:
# Get last changelist number. Cannot easily get it from
- # the submit command output as the output is unmarshalled.
+ # the submit command output as the output is
+ # unmarshalled.
changelist = self.lastP4Changelist()
self.modifyChangelistUser(changelist, p4User)
-
else:
+ # skip this patch
for f in editedFiles:
p4_revert(f)
for f in filesToAdd:
diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index 52003ae..5044a12 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -202,11 +202,24 @@ able to find the relevant client. This client spec will be used to
both filter the files cloned by git and set the directory layout as
specified in the client (this implies --keep-path style semantics).
-git-p4.skipSubmitModTimeCheck
+git-p4.skipSubmitEdit
- git config [--global] git-p4.skipSubmitModTimeCheck false
+ git config [--global] git-p4.skipSubmitEdit false
-If true, submit will not check if the p4 change template has been modified.
+Normally, git-p4 invokes an editor after each commit is applied so
+that you can make changes to the submit message. Setting this
+variable to true will skip the editing step, submitting the change as is.
+
+git-p4.skipSubmitEditCheck
+
+ git config [--global] git-p4.skipSubmitEditCheck false
+
+After the editor is invoked, git-p4 normally makes sure you saved the
+change description, as an indication that you did indeed read it over
+and edit it. You can quit without saving to abort the submit (or skip
+this change and continue). Setting this variable to true will cause
+git-p4 not to check if you saved the change description. This variable
+only matters if git-p4.skipSubmitEdit has not been set to true.
git-p4.preserveUser
diff --git a/t/t9804-skip-submit-edit.sh b/t/t9804-skip-submit-edit.sh
new file mode 100755
index 0000000..734ccf2
--- /dev/null
+++ b/t/t9804-skip-submit-edit.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+test_description='git-p4 skipSubmitEdit config variables'
+
+. ./lib-git-p4.sh
+
+test_expect_success 'start p4d' '
+ start_p4d
+'
+
+test_expect_success 'init depot' '
+ (
+ cd "$cli" &&
+ echo file1 >file1 &&
+ p4 add file1 &&
+ p4 submit -d "change 1"
+ )
+'
+
+# this works because EDITOR is set to :
+test_expect_success 'no config, unedited, say yes' '
+ "$GITP4" clone --dest="$git" //depot &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ echo line >>file1 &&
+ git commit -a -m "change 2" &&
+ echo y | "$GITP4" submit &&
+ p4 changes //depot/... >wc &&
+ test_line_count = 2 wc
+ )
+'
+
+test_expect_success 'no config, unedited, say no' '
+ "$GITP4" clone --dest="$git" //depot &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ echo line >>file1 &&
+ git commit -a -m "change 3 (not really)" &&
+ printf "bad response\nn\n" | "$GITP4" submit
+ p4 changes //depot/... >wc &&
+ test_line_count = 2 wc
+ )
+'
+
+test_expect_success 'skipSubmitEdit' '
+ "$GITP4" clone --dest="$git" //depot &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEdit true &&
+ # will fail if editor is even invoked
+ git config core.editor /bin/false &&
+ echo line >>file1 &&
+ git commit -a -m "change 3" &&
+ "$GITP4" submit &&
+ p4 changes //depot/... >wc &&
+ test_line_count = 3 wc
+ )
+'
+
+test_expect_success 'skipSubmitEditCheck' '
+ "$GITP4" clone --dest="$git" //depot &&
+ test_when_finished cleanup_git &&
+ (
+ cd "$git" &&
+ git config git-p4.skipSubmitEditCheck true &&
+ echo line >>file1 &&
+ git commit -a -m "change 4" &&
+ "$GITP4" submit &&
+ p4 changes //depot/... >wc &&
+ test_line_count = 4 wc
+ )
+'
+
+
+test_expect_success 'kill p4d' '
+ kill_p4d
+'
+
+test_done
--
1.7.7
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html