From: Luben Tuikov <ltuikov@yahoo•com>
To: Rafael Garcia-Suarez <rgarciasuarez@gmail•com>,
Jakub Narebski <jnareb@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame
Date: Tue, 3 Jun 2008 13:18:49 -0700 (PDT) [thread overview]
Message-ID: <839911.60903.qm@web31810.mail.mud.yahoo.com> (raw)
In-Reply-To: <m34p8a2173.fsf@localhost.localdomain>
--- On Tue, 6/3/08, Jakub Narebski <jnareb@gmail•com> wrote:
> From: Jakub Narebski <jnareb@gmail•com>
> Subject: Re: [PATCH] Avoid errors from git-rev-parse in gitweb blame
> To: "Rafael Garcia-Suarez" <rgarciasuarez@gmail•com>
> Cc: git@vger•kernel.org, "Luben Tuikov" <ltuikov@yahoo•com>
> Date: Tuesday, June 3, 2008, 4:43 AM
> Cc-ed Luben Tuikov, author of this part.
Thanks guys! :-)
> Rafael Garcia-Suarez <rgarciasuarez@gmail•com>
> writes:
>
> > git-rev-parse will abort with an error when passed a
> non-existent
> > revision spec, such as "deadbeef^" where
> deadbeef has no parent.
Yes, I've known about this ever since I coded this.
The reasoning was that the value of parsing up the "tree" of
parent changes was a lot more (valuable) than the value of detecting that
"deadbeef^" had no parent -- which would've been logically apparent
to the coder/reviewer/user of "blame2".
> > Using the --revs-only parameter makes this error go
> away, while
> > retaining functionality, keeping the web server error
> log nice
> > and clean.
Ok, that's fine, as long as indeed the functionality is preserved.
I leave it up to Jakub and Junio to make sure that indeed the
functionality is preserved. (I.e. saving us yet another patch
from anyone of us.)
> Thanks. This error wasn't detected earlier probably
> because
> 'blame' view is rarely enabled; and repo.or.cz
> gitweb which has
> 'blame' enabled IIRC use gitweb which is modified
> there, allowing
> incremental blame using AJAX.
I'm a heavy user of "blame2", often exploring the course of
"evolution" of the code and/or code lines and segments. I value this
in gitweb (being easier/more visual to use as opposed to command
line).
>
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index 55fb100..f3b4b24 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -4226,9 +4226,9 @@ git_blame2
> > esc_html($rev));
> > print "</td>\n";
> > }
> > - open (my $dd, "-|", git_cmd(),
> "rev-parse", "$full_rev^")
> > + open (my $dd, "-|", git_cmd(),
> "rev-parse", '--revs-only',
> "$full_rev^")
> > or die_error(undef, "Open git-rev-parse
> failed");
> > - my $parent_commit = <$dd>;
> > + my $parent_commit = <$dd> || '';
> > close $dd;
> > chomp($parent_commit);
> > my $blamed = href(action => 'blame',
>
> I'd rather remove this, correct it, or make it optional
> (this is very
> fork-heavy).
>
> But this patch is good as it is now...
Yes, I agree it is good. Just you guys make sure that it doesn't change
the value of the functionality of the code.
Thanks everyone!
Luben
next prev parent reply other threads:[~2008-06-03 20:26 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-03 10:46 [PATCH] Avoid errors from git-rev-parse in gitweb blame Rafael Garcia-Suarez
2008-06-03 11:42 ` Lea Wiemann
2008-06-03 11:43 ` Jakub Narebski
2008-06-03 12:03 ` Rafael Garcia-Suarez
2008-06-03 12:45 ` Jakub Narebski
2008-06-03 13:00 ` Rafael Garcia-Suarez
2008-06-03 13:12 ` Jakub Narebski
2008-06-03 13:36 ` Rafael Garcia-Suarez
2008-06-03 14:14 ` Jakub Narebski
2008-06-03 14:40 ` Rafael Garcia-Suarez
2008-06-03 14:56 ` Jakub Narebski
2008-06-03 15:07 ` Rafael Garcia-Suarez
2008-06-03 17:50 ` Jakub Narebski
2008-06-03 21:09 ` Luben Tuikov
2008-06-03 21:03 ` Luben Tuikov
2008-06-03 20:35 ` Luben Tuikov
2008-06-03 21:31 ` Jakub Narebski
2008-06-04 5:58 ` Junio C Hamano
2008-06-04 14:03 ` Jakub Narebski
2008-06-05 6:07 ` Junio C Hamano
2008-06-05 6:09 ` [PATCH 1/2] git-blame: refactor code to emit "porcelain format" output Junio C Hamano
2008-06-06 9:22 ` Jakub Narebski
2008-06-05 6:09 ` [PATCH 2/2] blame: show "previous" information in --porcelain/--incremental format Junio C Hamano
2008-06-06 9:27 ` Jakub Narebski
2008-06-06 15:17 ` Junio C Hamano
2008-06-06 15:44 ` Jakub Narebski
2008-06-06 0:26 ` [PATCH] Avoid errors from git-rev-parse in gitweb blame Jakub Narebski
2008-06-04 22:24 ` Luben Tuikov
2008-06-03 14:24 ` Lea Wiemann
2008-06-03 20:24 ` Jakub Narebski
2008-06-03 23:11 ` Lea Wiemann
2008-06-04 0:11 ` Jakub Narebski
2008-06-04 0:39 ` Lea Wiemann
2008-06-04 12:31 ` Jakub Narebski
2008-06-08 18:19 ` Lea Wiemann
2008-06-08 20:28 ` Jakub Narebski
2008-06-03 20:18 ` Luben Tuikov [this message]
2008-06-03 20:29 ` Jakub Narebski
2008-06-03 21:27 ` Luben Tuikov
2008-06-03 21:34 ` Jakub Narebski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=839911.60903.qm@web31810.mail.mud.yahoo.com \
--to=ltuikov@yahoo$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jnareb@gmail$(echo .)com \
--cc=rgarciasuarez@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox