tig show <commit> not working anymore?

Hi!

I observed that, since commit
53c089443cd9885fa677becee4bf7ffd56c3c357, tig show <commit> doesn't
seem to work the same way as I had expected it to. The documentation
update doesn't seem to indicate that this was to be expected (or I
didn't understand it properly). Could someone please confirm that
tig show <commit> should actually do something similar to git show <commit> | tig ?

Thanks!

Kumar

On 9/16/2010 2:53 PM, Kumar Appaiah wrote:

Hi!

I observed that, since commit
53c089443cd9885fa677becee4bf7ffd56c3c357, tig show <commit> doesn't
seem to work the same way as I had expected it to. The documentation
update doesn't seem to indicate that this was to be expected (or I
didn't understand it properly). Could someone please confirm that
tig show <commit> should actually do something similar to git show <commit> | tig ?

Thanks!

Works as expected for me on Debian Linux 5.0.6 i686:

$ git version
git version 1.7.2.3

$ git describe
tig-0.16-11-gf1fe753

and on Cygwin with the same tig version and "git version 1.7.2.3.msysgit.1"

Your tig version, OS, etc. ?

Regards,
  Stefan
-- 
----------------------------------------------------------------
/dev/random says: Useless Invention: Leather cutlery.

On Thu, Sep 16, 2010 at 08:53, Kumar Appaiah [off-list ref] wrote:

Hi!

Hello,

I observed that, since commit
53c089443cd9885fa677becee4bf7ffd56c3c357, tig show <commit> doesn't
seem to work the same way as I had expected it to. The documentation
update doesn't seem to indicate that this was to be expected (or I
didn't understand it properly). Could someone please confirm that
tig show <commit> should actually do something similar to git show <commit> | tig ?

What is the expected behavoir that is not working anymore?

-- 
Jonas Fonseca

Kumar Appaiah venit, vidit, dixit 16.09.2010 14:53:

Hi!

I observed that, since commit
53c089443cd9885fa677becee4bf7ffd56c3c357, tig show <commit> doesn't
seem to work the same way as I had expected it to. The documentation
update doesn't seem to indicate that this was to be expected (or I
didn't understand it properly). Could someone please confirm that
tig show <commit> should actually do something similar to git show <commit> | tig ?

It seems that since that tig commit, "tig show <commit>" shows HEAD
rather than <commit>. OTOH, "tig <commit>" does navigate to <commit>.

The reason is that tig passes %(commit), not %(revargs), to git show for
the "show" subcommand, but the obvious simple patch breaks diff view. I
guess somewhere in the codepath one has to differentiate between those.
cc'ing Mr. Tig...

Michael

On 9/16/2010 3:52 PM, Michael J Gruber wrote:

Kumar Appaiah venit, vidit, dixit 16.09.2010 14:53:

quoted

Hi!

I observed that, since commit
53c089443cd9885fa677becee4bf7ffd56c3c357, tig show <commit> doesn't
seem to work the same way as I had expected it to. The documentation
update doesn't seem to indicate that this was to be expected (or I
didn't understand it properly). Could someone please confirm that
tig show <commit> should actually do something similar to git show <commit> | tig ?

It seems that since that tig commit, "tig show <commit>" shows HEAD
rather than <commit>. OTOH, "tig <commit>" does navigate to <commit>.

Oops...how true.

I tested 'tig show <commit>' and noticed it showed something but
didn't look close enough 8-|

Stefan
-- 
----------------------------------------------------------------
/dev/random says: Never argue with a woman when she's tired, or rested.

Dear Jonas,

On Thu, Sep 16, 2010 at 09:47:58AM -0400, Jonas Fonseca wrote:

On Thu, Sep 16, 2010 at 08:53, Kumar Appaiah [off-list ref] wrote:

quoted

I observed that, since commit
53c089443cd9885fa677becee4bf7ffd56c3c357, tig show <commit> doesn't
seem to work the same way as I had expected it to. The documentation
update doesn't seem to indicate that this was to be expected (or I
didn't understand it properly). Could someone please confirm that
tig show <commit> should actually do something similar to git show <commit> | tig ?

What is the expected behavoir that is not working anymore?

I expect the formatted commit diff of the commit I specify after
show. For example, tig show a74fa1900a65384bc4140fe45a55877bc981b185
used to show me the commit titled "Misc cleanups" (in tig), while now,
it shows me HEAD.

Please let me know if I have missed something.

Thanks!

Kumar

Dear Stefan,

On Thu, Sep 16, 2010 at 03:46:20PM +0200, Stefan Naewe wrote:

On 9/16/2010 2:53 PM, Kumar Appaiah wrote:

quoted

I observed that, since commit
53c089443cd9885fa677becee4bf7ffd56c3c357, tig show <commit> doesn't
seem to work the same way as I had expected it to. The documentation
update doesn't seem to indicate that this was to be expected (or I
didn't understand it properly). Could someone please confirm that
tig show <commit> should actually do something similar to git show <commit> | tig ?

Thanks!

Works as expected for me on Debian Linux 5.0.6 i686:

$ git version
git version 1.7.2.3

$ git describe
tig-0.16-11-gf1fe753

and on Cygwin with the same tig version and "git version 1.7.2.3.msysgit.1"

Your tig version, OS, etc. ?

Here are the details:

TIG_VERSION="0.16-11-gf1fe753"

[kumar@redsun ~/Software/tig/tig] git --version
git version 1.7.1

./tig show HEAD
./tig show HEAD^ # Both show me f1fe753be71ef3ce5abcbb16c2a359f97e5c7e7d

Now, if I check out 53c089443cd9885fa677becee4bf7ffd56c3c357^, then I
get back the expected behaviour.

Please let me know if I've made some mistake.

Thanks!

Kumar

On Thu, Sep 16, 2010 at 03:59:05PM +0200, Stefan Naewe wrote:

On 9/16/2010 3:52 PM, Michael J Gruber wrote:

quoted

It seems that since that tig commit, "tig show <commit>" shows HEAD
rather than <commit>. OTOH, "tig <commit>" does navigate to <commit>.

Oops...how true.

I tested 'tig show <commit>' and noticed it showed something but
didn't look close enough 8-|

Ah, maybe I should have been more explicit and specified that it shows
me the wrong commit. :-)

I did attempt to fix the code myself, but I require a more thorough
understanding of the code to be able to fix it myself.

Thanks.

Kumar

On Thu, Sep 16, 2010 at 03:52:18PM +0200, Michael J Gruber wrote:

quoted

I observed that, since commit
53c089443cd9885fa677becee4bf7ffd56c3c357, tig show <commit> doesn't
seem to work the same way as I had expected it to. The documentation
update doesn't seem to indicate that this was to be expected (or I
didn't understand it properly). Could someone please confirm that
tig show <commit> should actually do something similar to git show <commit> | tig ?

It seems that since that tig commit, "tig show <commit>" shows HEAD
rather than <commit>. OTOH, "tig <commit>" does navigate to <commit>.

The reason is that tig passes %(commit), not %(revargs), to git show for
the "show" subcommand, but the obvious simple patch breaks diff view. I
guess somewhere in the codepath one has to differentiate between those.
cc'ing Mr. Tig...

Please bear in mind that I am totally new to the code, and not a power
user of git either. So please don't club me! ;-)

Well, I was exploring the code a little, and I could not understand
some aspects fully. First, I discovered that the following hack fixes
the problem to an extent:

diff --git a/tig.c b/tig.c
index e0e812c..aa38e71 100644
--- a/tig.c
+++ b/tig.c

@@ -7864,7 +7864,7 @@ parse_options(int argc, const char *argv[])

        if (filter_argv)
                filter_options(filter_argv);
-
+       if (!strcmp(subcommand, "show") && opt_rev_args && opt_rev_args[0]) { string_copy_rev(ref_commit, opt_rev_args[0]); }
        return request;
 }

However, I am pretty certain that that is not the intended use of
opt_rev_args and ref_commit, and that they should be used in a
different way. For example, tig show HEAD^^^..HEAD should show me
three commits combined, but it shows me only one, since I haven't run
it through rev-list or the like¸ probably.

I'll continue to try getting at a fix, unless someone comes to it
before me.

Thanks!

Kumar

On Sat, Sep 18, 2010 at 01:01, Kumar Appaiah [off-list ref] wrote:

I'll continue to try getting at a fix, unless someone comes to it
before me.

I will take a look sometime today and hopefully release a new version.

-- 
Jonas Fonseca

On Sat, Sep 18, 2010 at 01:01, Kumar Appaiah [off-list ref] wrote:

Please bear in mind that I am totally new to the code, and not a power
user of git either. So please don't club me! ;-)

OK, I will give you a hug. :-)

Well, I was exploring the code a little, and I could not understand
some aspects fully. First, I discovered that the following hack fixes
the problem to an extent:

[snip]

However, I am pretty certain that that is not the intended use of
opt_rev_args and ref_commit, and that they should be used in a
different way. For example, tig show HEAD^^^..HEAD should show me
three commits combined, but it shows me only one, since I haven't run
it through rev-list or the like¸ probably.

You fix is not that far off but to fix the second issue I had to do
change the function responsible for expanding %(commit). Anyway,
should be fixed in 0.16.1.

-- 
Jonas Fonseca

Dear Jonas,

On Sun, Sep 19, 2010 at 02:46:22PM -0400, Jonas Fonseca wrote:

quoted

Well, I was exploring the code a little, and I could not understand
some aspects fully. First, I discovered that the following hack fixes
the problem to an extent:

[snip]

However, I am pretty certain that that is not the intended use of
opt_rev_args and ref_commit, and that they should be used in a
different way. For example, tig show HEAD^^^..HEAD should show me
three commits combined, but it shows me only one, since I haven't run
it through rev-list or the like¸ probably.

You fix is not that far off but to fix the second issue I had to do
change the function responsible for expanding %(commit). Anyway,
should be fixed in 0.16.1.

Unfortunately, this has introduced another (albeit minor) bug: it
breaks the tree view in some cases. For example:

git clone git://gitorious.org/taggrepper/taggrepper.git

and run tig there, and press `t'. It always gives me a blank tree.

The cause for this is that view->parent is null when the tree view
is invoked, but you merely pass !view->parent in the call to
format_argv in prepare_io.

A simple patch to fix this is attached below, but you might have other
ideas or I might have missed something, or not added sufficient checks
for the argv vector.

As always, if I have missed something above, please do let me know.

Thanks!

Kumar

From 1295e58dbb3035bba3ebaf8686dc328b2b457ae2 Mon Sep 17 00:00:00 2001
From: Kumar Appaiah <redacted>
Date: Sun, 19 Sep 2010 23:19:16 -0500
Subject: [PATCH] Add check to fix tree view

---
 tig.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tig.c b/tig.c
index 7bc5daa..08042ec 100644
--- a/tig.c
+++ b/tig.c

@@ -3332,7 +3332,7 @@ static bool
 prepare_io(struct view *view, const char *dir, const char *argv[], bool replace)
 {
 	view->dir = dir;
-	return format_argv(&view->argv, argv, replace, !view->parent);
+	return format_argv(&view->argv, argv, replace, !view->parent && strcmp(argv[1], "ls-tree"));
 }
 
 static bool

-- 
1.7.1

On Mon, Sep 20, 2010 at 00:25, Kumar Appaiah [off-list ref] wrote:

On Sun, Sep 19, 2010 at 02:46:22PM -0400, Jonas Fonseca wrote:

quoted

You fix is not that far off but to fix the second issue I had to do
change the function responsible for expanding %(commit). Anyway,
should be fixed in 0.16.1.

Unfortunately, this has introduced another (albeit minor) bug: it
breaks the tree view in some cases. For example:

git clone git://gitorious.org/taggrepper/taggrepper.git

and run tig there, and press `t'. It always gives me a blank tree.

The cause for this is that view->parent is null when the tree view
is invoked, but you merely pass !view->parent in the call to
format_argv in prepare_io.

Oops, I should really finish the tig.c split up branch and introduce a
test suite to catch stuff like this.

A simple patch to fix this is attached below, but you might have other
ideas or I might have missed something, or not added sufficient checks
for the argv vector.

As always, if I have missed something above, please do let me know.

Thanks for the patch and for reporting this. I've fixed this by using
view->prev instead of view->parent. The view->prev tracks the list of
views whereas view->parent tracks parents/child relationships in split
view mode. It was one of the things I refactored not long ago.

-- 
Jonas Fonseca