threads / discuss / 4893
Subject: Strange output of 'git diff <revision1>:<file> <revision2>:<file>'
Jakub Narebskigit diff output for files specified by revision is somewhat unexpected.
$ git diff <revision_1>:<file> <revision_2>:<file>
outputs the following diff metainfo
diff --git a/<revision_2>:<file> b/<revision_2>:<file> index 5eabe06..2e87de4 100644 --- a/<revision_2>:<file> +++ b/<revision_2>:<file>
Is it intended, or is it a bug? Looks like a bug to me...
git 1.4.0 (git-core-1.4.0-1.fc4 by Fedora Project).
-- Jakub Narebski Warsaw, Poland ShadeHawk on #git
[PATCH] use the path as name from the revision:path syntax in setup_revisions
--- Jakub Narebski <jnareb@gmail.com> wrote:
> git diff output for files specified by revision is somewhat unexpected. > > $ git diff <revision_1>:<file> <revision_2>:<file> > > outputs the following diff metainfo > > diff --git a/<revision_2>:<file> b/<revision_2>:<file> > index 5eabe06..2e87de4 100644 > --- a/<revision_2>:<file> > +++ b/<revision_2>:<file> > > Is it intended, or is it a bug? Looks like a bug to me...
I guess the revision should not be there. Here is a patch to remove
it.
<matled> is there a name for 'revision:path' to specify a file at a
certain revision?
<jdl> "That weird syntax, you know."
<matled> get_path_out_of_the_weird_syntax(const char *s)
<matled> you know :)
<jdl> That should do it!
Could someone please come up with a nice name for the function?
Without knowing a name for the '<revision>:<path>' syntax I can only
think of get_path or similar names and this is much too generic imo.
---
cache.h | 1 +
revision.c | 2 ++
sha1_name.c | 12 ++++++++++++
3 files changed, 15 insertions(+), 0 deletions(-)diff --git a/cache.h b/cache.h index d433d46..7da1c5c 100644 --- a/cache.h +++ b/cache.h @@ -249,6 +249,7 @@ #define DEFAULT_ABBREV 7 extern int get_sha1(const char *str, unsigned char *sha1); extern int get_sha1_hex(const char *hex, unsigned char *sha1); +extern char *get_path_out_of_the_weird_syntax(const char *str); extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */ extern int read_ref(const char *filename, unsigned char *sha1); extern const char *resolve_ref(const char *path, unsigned char *sha1, int); diff --git a/revision.c b/revision.c index 874e349..fbd1458 100644 --- a/revision.c +++ b/revision.c @@ -908,6 +908,8 @@ int setup_revisions(int argc, const char if (!seen_dashdash) verify_non_filename(revs->prefix, arg); object = get_reference(revs, arg, sha1, flags ^ local_flags); + if (object->type == OBJ_BLOB || object->type == OBJ_TREE) + arg = get_path_out_of_the_weird_syntax(arg); add_pending_object(revs, object, arg); } if (show_merge) diff --git a/sha1_name.c b/sha1_name.c index 5fe8e5d..718ce1b 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -543,3 +543,15 @@ int get_sha1(const char *name, unsigned } return ret; } + +/* get the path from sha1:path, :path and :[0-3]:path */ +char *get_path_out_of_the_weird_syntax(const char *str) +{ + if (strlen(str) >= 3 && + str[0] == ':' && str[2] == ':' && + str[1] >= '0' && str[2] <= '3') + return((char*)(str+3)); + if (index(str, ':')) + return index(str, ':')+1; + return str; +}
-- 1.4.1.ga3e6
[PATCH] array index mixup
--- Jakub Narebski <jnareb@gmail.com> wrote:
> git diff output for files specified by revision is somewhat unexpected. > > $ git diff <revision_1>:<file> <revision_2>:<file> > > outputs the following diff metainfo > > diff --git a/<revision_2>:<file> b/<revision_2>:<file> > index 5eabe06..2e87de4 100644 > --- a/<revision_2>:<file> > +++ b/<revision_2>:<file> > > Is it intended, or is it a bug? Looks like a bug to me...
I dunno if this is really an index mixup or was intended. If this is intended please add a comment what it's for. (Without this you get rename information, perhaps this is the reason.) --- builtin-diff.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin-diff.c b/builtin-diff.c index cb38f44..4d43a5c 100644 --- a/builtin-diff.c +++ b/builtin-diff.c @@ -136,7 +136,7 @@ static int builtin_diff_blobs(struct rev stuff_change(&revs->diffopt, mode, mode, blob[1].sha1, blob[0].sha1, - blob[0].name, blob[0].name); + blob[1].name, blob[0].name); diffcore_std(&revs->diffopt); diff_flush(&revs->diffopt); return 0;
-- 1.4.1.ga3e6
Junio C HamanoRe: [PATCH] array index mixup
Matthias Lederhofer <matled@gmx.net> writes:
> I dunno if this is really an index mixup or was intended. If this is > intended please add a comment what it's for. (Without this you get > rename information, perhaps this is the reason.)
That is exactly the reason -- it was a temporary workaround which nobody noticed so far.
The right fix would involve updating diff_resolve_rename_copy so that it does not rely on the comparison of path names (that means DIFF_PAIR_RENAME() macro needs to change), and instead mark the pairs synthesized in diffcore-rename as such, and use that to tell if a pair is a result of rename/copy [*1*].
Your other patch (not the one to change the index of the array used for labels, but the one that extracts the pathname out of the syntax to name a blob by path in an arbitrary tree object) could be safely applied when that happens.
[Footnote]
*1* If somebody wants to do this, one thing to watch out for is matching up of broken pairs. If a pair originally broken by diffcore-break (because they were dissimilar enough according to the option given to -B flag) are merged into one by diffcore-rename (because they were similar enough according to the option given to -M flag), we should _not_ say the resulting pair is renamed. In general, the threashold for breaking should be lower than diffcore-rename to merge them for a sane use, so this might be a non-issue in practice, though.