Re: [PATCH 1/2] git-p4: Improve rename detection support.

Hi Pete,

On Sun, Feb 6, 2011 at 12:21 AM, Pete Wyckoff [off-list ref] wrote:

The comparisons confuse me.  detectRenames != "false" > 0  ?
How about just detectRenames == "true"?

The "> 0" was for the length check. I somehow (*feeling embarrassed*)
misplaced that code...

You could rename the existing self.detectRename to add an "s" so
it all makes a bit more sense.

   if not self.detectRenames:
       # not explicitly set, check the config variable
       b = gitConfig("git-p4.detectRenames")
       if b == "true":
           self.detectRenames = "-M"

   if self.detectRenames:
       diffOpts = "-M"
   else:
       diffOpts = ""

Seems like a better idea. I kind of like the original code to set
diffOpts, so I would prefer to keep it. What do you think of (didn't
test it):

        if not self.detectRenames:
            # If not explicitly set check the config variable
            self.detectRenames =
gitConfig("git-p4.detectRenames").lower() == "true"

        diffOpts = ("", "-M")[self.detectRenames]

quoted

         diff = read_pipe_lines("git diff-tree -r %s \"%s^\" \"%s\"" % (diffOpts, id, id))
         filesToAdd = set()
         filesToDelete = set()

@@ -640,7 +646,8 @@ class P4Submit(Command):

             elif modifier == "R":
                 src, dest = diff['src'], diff['dst']
                 p4_system("integrate -Dt \"%s\" \"%s\"" % (src, dest))
-                p4_system("edit \"%s\"" % (dest))
+                if diff['src_sha1'] != diff['dst_sha1']:
+                    p4_system("edit \"%s\"" % (dest))
                 if isModeExecChanged(diff['src_mode'], diff['dst_mode']):
                     filesToChangeExecBit[dest] = diff['dst_mode']
                 os.unlink(dest)

If you rename the file and also cause its perms to change (say
add +x), does it still work if dest is not open?

This is a very good point. I will also open the file for edit when
there is a mode change.

Thanks!

-- 
Vitor Antunes