On Mon, Mar 1, 2010 at 1:41 AM, Peter Tyser <[email protected]> wrote: > On Mon, Mar 1, 2010 at 12:38 AM, Kai Willadsen <[email protected]> > wrote: >> Patches! that look good! and have commit messages! You're my hero for the >> day. > > I aim to please!:) > >> On 1 March 2010 06:42, Peter Tyser <[email protected]> wrote: >> <snip> >>> @@ -39,12 +39,12 @@ class Vc(_vc.CachedVc): >>> PATCH_STRIP_NUM = 1 >>> PATCH_INDEX_RE = "^diff --git a/(.*) b/.*$" >>> state_map = { >>> - "unknown": _vc.STATE_NONE, >>> - "new file": _vc.STATE_NEW, >>> - "deleted": _vc.STATE_REMOVED, >>> - "modified": _vc.STATE_MODIFIED, >>> - "typechange": _vc.STATE_MODIFIED, >>> - "unmerged": _vc.STATE_CONFLICT, >>> + "X": _vc.STATE_NONE, # Unknown >>> + "A": _vc.STATE_NEW, # New >>> + "D": _vc.STATE_REMOVED, # Deleted >>> + "M": _vc.STATE_MODIFIED, # Modified >>> + "T": _vc.STATE_MODIFIED, # Type-changed >>> + "U": _vc.STATE_CONFLICT, # Unmerged >>> } >> >> So this won't handle renames or copies. The old code treated rename as >> new/deleted, and a copy as a straight new. I guess adding --no-renames >> to the command line would give us the same result for the rename case? >> I couldn't find an equivalent for no copies, but I'm sure it's around >> somewhere. > > Ahh, good point. The patch handles renames and copies, but just as > the addition/deletion of files. It'd be much nicer to detect the > rename/copy and create a relevant diff as you suggest. I'm pretty > sure "git diff-index -M" should do the trick. This would have the > same functionality as the previous code where renames, but not copies > are detected. > > It could be argued that detecting copies would be nice too (eg add -C > to "git diff-index"). Git can detect copied files with changes, so > the diff might actually be usefully when copying and slightly > modifying a file from the original. Let me know if you have any > issues with this operation as it is slightly different from the > original code. > > I'll rework the patch and resubmit. Thanks for the comments,
I just looked into it again, and I think this patch should not change the functionality of the old code. My patch also treats a rename as a new/deleted, and a copy as a straight new. For example: pty...@ptyser-laptop u-boot $ git mv Makefile Makefile.rename pty...@ptyser-laptop u-boot $ cp MAINTAINERS MAINTAINERS.copy pty...@ptyser-laptop u-boot $ git add MAINTAINERS.copy pty...@ptyser-laptop u-boot $ echo "asdf" >> README pty...@ptyser-laptop u-boot $ git st # On branch master # Your branch is ahead of 'origin/master' by 32 commits. # # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # new file: MAINTAINERS.copy # renamed: Makefile -> Makefile.rename # # Changed but not updated: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # modified: README pty...@ptyser-laptop u-boot $ git diff-index --name-status HEAD A MAINTAINERS.copy D Makefile A Makefile.rename M README Last night I was under the impression that it would be easy to add the ability to detect a rename/copy, and then provide a diff of any changes to the renamed/copied file. This looks like it would be more difficult than I thought since now there is a 1-to-1 correspondence between a file and state. In order to detect and display modified renames/copies you'd need to have 1 state correspond to 2 files - the original plus the modified renamed/copied file. I can look into how hard this would be, but I'd prefer to do it in a follow-up patch if at all. Also, using git diff-index resolves another bug with the current code. Take the following example: pty...@ptyser-laptop u-boot $ git mv Makefile Makefile.move pty...@ptyser-laptop u-boot $ echo "asdf" >> Makefile.move pty...@ptyser-laptop u-boot $ git st # On branch master # Your branch is ahead of 'origin/master' by 32 commits. # # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # renamed: Makefile -> Makefile.move # # Changed but not updated: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # modified: Makefile.move The old code would parse this output from top to bottom. Initially it would correctly think Makefile.move was a new file. But later it would see "modified: Makefile.move" and relabel it as a modified file. The meld output would be incorrect in this instance. "git diff-index" doesn't have this issue: pty...@ptyser-laptop u-boot $ git diff-index --name-status HEAD D Makefile A Makefile.move Sorry for the long-winded email. Long story short, I think the current patch is an improvement as is. I won't resubmit unless someone sees a flaw in the logic above. Best, Peter _______________________________________________ meld-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/meld-list
