Re: [PATCH 1/3] git p4: remove unused P4Submit interactive setting

[Luke Diamand <luke@diamand.org>]

On 04/07/12 14:34, Pete Wyckoff wrote:
> The code is unused.  Delete.

I've used that non-interactive code path in the past, in the very early 
days of using it (setting interactive to false manually).

The nice thing about it is that if you're using git-p4 for the very 
first time it lets you do the final submission to p4 by hand, without 
having to trust the script to do the right thing. Once I convinced 
myself that git-p4 was doing the right thing, I then stopped using it.

Is it worth retaining, perhaps fixed so that it can be set on the 
command line and documented? Or just discard?

Thanks
Luke

>
> Signed-off-by: Pete Wyckoff<pw@padd.com>
> ---
>   git-p4.py | 144 ++++++++++++++++++++++++++++----------------------------------
>   1 file changed, 66 insertions(+), 78 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index f895a24..542c20a 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -844,7 +844,6 @@ class P4Submit(Command, P4UserMap):
>           ]
>           self.description = "Submit changes from git to the perforce depot."
>           self.usage += " [name of git branch to submit into perforce depot]"
> -        self.interactive = True
>           self.origin = ""
>           self.detectRenames = False
>           self.preserveUser = gitConfig("git-p4.preserveUser").lower() == "true"
> @@ -1209,86 +1208,77 @@ class P4Submit(Command, P4UserMap):
>
>           template = self.prepareSubmitTemplate()
>
> -        if self.interactive:
> -            submitTemplate = self.prepareLogMessage(template, logMessage)
> +        submitTemplate = self.prepareLogMessage(template, logMessage)
>
> -            if self.preserveUser:
> -               submitTemplate = submitTemplate + ("\n######## Actual user %s, modified after commit\n" % p4User)
> -
> -            if os.environ.has_key("P4DIFF"):
> -                del(os.environ["P4DIFF"])
> -            diff = ""
> -            for editedFile in editedFiles:
> -                diff += p4_read_pipe(['diff', '-du',
> -                                      wildcard_encode(editedFile)])
> -
> -            newdiff = ""
> -            for newFile in filesToAdd:
> -                newdiff += "==== new file ====\n"
> -                newdiff += "--- /dev/null\n"
> -                newdiff += "+++ %s\n" % newFile
> -                f = open(newFile, "r")
> -                for line in f.readlines():
> -                    newdiff += "+" + line
> -                f.close()
> -
> -            if self.checkAuthorship and not self.p4UserIsMe(p4User):
> -                submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail
> -                submitTemplate += "######## Use option --preserve-user to modify authorship.\n"
> -                submitTemplate += "######## Variable git-p4.skipUserNameCheck hides this message.\n"
> -
> -            separatorLine = "######## everything below this line is just the diff #######\n"
> -
> -            (handle, fileName) = tempfile.mkstemp()
> -            tmpFile = os.fdopen(handle, "w+")
> -            if self.isWindows:
> -                submitTemplate = submitTemplate.replace("\n", "\r\n")
> -                separatorLine = separatorLine.replace("\n", "\r\n")
> -                newdiff = newdiff.replace("\n", "\r\n")
> -            tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
> +        if self.preserveUser:
> +           submitTemplate = submitTemplate + ("\n######## Actual user %s, modified after commit\n" % p4User)
> +
> +        if os.environ.has_key("P4DIFF"):
> +            del(os.environ["P4DIFF"])
> +        diff = ""
> +        for editedFile in editedFiles:
> +            diff += p4_read_pipe(['diff', '-du',
> +                                  wildcard_encode(editedFile)])
> +
> +        newdiff = ""
> +        for newFile in filesToAdd:
> +            newdiff += "==== new file ====\n"
> +            newdiff += "--- /dev/null\n"
> +            newdiff += "+++ %s\n" % newFile
> +            f = open(newFile, "r")
> +            for line in f.readlines():
> +                newdiff += "+" + line
> +            f.close()
> +
> +        if self.checkAuthorship and not self.p4UserIsMe(p4User):
> +            submitTemplate += "######## git author %s does not match your p4 account.\n" % gitEmail
> +            submitTemplate += "######## Use option --preserve-user to modify authorship.\n"
> +            submitTemplate += "######## Variable git-p4.skipUserNameCheck hides this message.\n"
> +
> +        separatorLine = "######## everything below this line is just the diff #######\n"
> +
> +        (handle, fileName) = tempfile.mkstemp()
> +        tmpFile = os.fdopen(handle, "w+")
> +        if self.isWindows:
> +            submitTemplate = submitTemplate.replace("\n", "\r\n")
> +            separatorLine = separatorLine.replace("\n", "\r\n")
> +            newdiff = newdiff.replace("\n", "\r\n")
> +        tmpFile.write(submitTemplate + separatorLine + diff + newdiff)
> +        tmpFile.close()
> +
> +        if self.edit_template(fileName):
> +            # read the edited message and submit
> +            tmpFile = open(fileName, "rb")
> +            message = tmpFile.read()
>               tmpFile.close()
> +            submitTemplate = message[:message.index(separatorLine)]
> +            if self.isWindows:
> +                submitTemplate = submitTemplate.replace("\r\n", "\n")
> +            p4_write_pipe(['submit', '-i'], submitTemplate)
>
> -            if self.edit_template(fileName):
> -                # read the edited message and submit
> -                tmpFile = open(fileName, "rb")
> -                message = tmpFile.read()
> -                tmpFile.close()
> -                submitTemplate = message[:message.index(separatorLine)]
> -                if self.isWindows:
> -                    submitTemplate = submitTemplate.replace("\r\n", "\n")
> -                p4_write_pipe(['submit', '-i'], submitTemplate)
> -
> -                if self.preserveUser:
> -                    if p4User:
> -                        # Get last changelist number. Cannot easily get it from
> -                        # the submit command output as the output is
> -                        # unmarshalled.
> -                        changelist = self.lastP4Changelist()
> -                        self.modifyChangelistUser(changelist, p4User)
> -
> -                # The rename/copy happened by applying a patch that created a
> -                # new file.  This leaves it writable, which confuses p4.
> -                for f in pureRenameCopy:
> -                    p4_sync(f, "-f")
> -
> -            else:
> -                # skip this patch
> -                print "Submission cancelled, undoing p4 changes."
> -                for f in editedFiles:
> -                    p4_revert(f)
> -                for f in filesToAdd:
> -                    p4_revert(f)
> -                    os.remove(f)
> +            if self.preserveUser:
> +                if p4User:
> +                    # Get last changelist number. Cannot easily get it from
> +                    # the submit command output as the output is
> +                    # unmarshalled.
> +                    changelist = self.lastP4Changelist()
> +                    self.modifyChangelistUser(changelist, p4User)
> +
> +            # The rename/copy happened by applying a patch that created a
> +            # new file.  This leaves it writable, which confuses p4.
> +            for f in pureRenameCopy:
> +                p4_sync(f, "-f")
>
> -            os.remove(fileName)
>           else:
> -            fileName = "submit.txt"
> -            file = open(fileName, "w+")
> -            file.write(self.prepareLogMessage(template, logMessage))
> -            file.close()
> -            print ("Perforce submit template written as %s. "
> -                   + "Please review/edit and then use p4 submit -i<  %s to submit directly!"
> -                   % (fileName, fileName))
> +            # skip this patch
> +            print "Submission cancelled, undoing p4 changes."
> +            for f in editedFiles:
> +                p4_revert(f)
> +            for f in filesToAdd:
> +                p4_revert(f)
> +                os.remove(f)
> +
> +        os.remove(fileName)
>
>       # Export git tags as p4 labels. Create a p4 label and then tag
>       # with that.
> @@ -1437,8 +1427,6 @@ class P4Submit(Command, P4UserMap):
>               commit = commits[0]
>               commits = commits[1:]
>               self.applyCommit(commit)
> -            if not self.interactive:
> -                break
>
>           if len(commits) == 0:
>               print "All changes applied!"