tig show <commit> not working anymore?

tig show <commit> not working anymore?

[Kumar Appaiah]

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

Re: tig show <commit> not working anymore?

[Stefan Naewe]

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.

Re: tig show <commit> not working anymore?

[Jonas Fonseca]

On Thu, Sep 16, 2010 at 08:53, Kumar Appaiah <a.kumar@alumni.iitm.ac.in> 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

Re: tig show <commit> not working anymore?

[Michael J Gruber]

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

Re: tig show <commit> not working anymore?

[Stefan Naewe]

On 9/16/2010 3:52 PM, Michael J Gruber wrote:
> 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>.


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.

Re: tig show <commit> not working anymore?

[Kumar Appaiah]

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 <a.kumar@alumni.iitm.ac.in> wrote:
> > 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

Re: tig show <commit> not working anymore?

[Kumar Appaiah]

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:
> > 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

Re: tig show <commit> not working anymore?

[Kumar Appaiah]

On Thu, Sep 16, 2010 at 03:59:05PM +0200, Stefan Naewe wrote:
> On 9/16/2010 3:52 PM, Michael J Gruber wrote:
> > 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

Re: tig show <commit> not working anymore?

[Kumar Appaiah]

On Thu, Sep 16, 2010 at 03:52:18PM +0200, Michael J Gruber wrote:
> > 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

Re: tig show <commit> not working anymore?

[Jonas Fonseca]

On Sat, Sep 18, 2010 at 01:01, Kumar Appaiah <a.kumar@alumni.iitm.ac.in> 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

Re: tig show <commit> not working anymore?

[Jonas Fonseca]

On Sat, Sep 18, 2010 at 01:01, Kumar Appaiah <a.kumar@alumni.iitm.ac.in> 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

Re: tig show <commit> not working anymore?

[Kumar Appaiah]

Dear Jonas,

On Sun, Sep 19, 2010 at 02:46:22PM -0400, Jonas Fonseca wrote:
> > 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 <a.kumar@alumni.iitm.ac.in>
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

Re: tig show <commit> not working anymore?

[Jonas Fonseca]

On Mon, Sep 20, 2010 at 00:25, Kumar Appaiah <a.kumar@alumni.iitm.ac.in> wrote:
> On Sun, Sep 19, 2010 at 02:46:22PM -0400, Jonas Fonseca wrote:
>> 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