git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jakub Narębski" <jnareb@gmail.com>
To: "Jürgen Kreileder" <jk@blackdown.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 4/4] gitweb: Fix broken blob action parameters on blob/commitdiff pages
Date: Tue, 09 Apr 2013 17:25:59 +0200	[thread overview]
Message-ID: <51643307.9090603@gmail.com> (raw)
In-Reply-To: <m2vc7wbx76.fsf@blackdown.de>

W dniu 08.04.2013 22:10, Jürgen Kreileder pisze:

> Fix broken blob action parameters on blobdiff and commitdiff pages by
> explicitly passing variables instead of relying on global ones.

Do I understand it correctly that those variables (e.g. $hash variable
in git_patchset_body in second chunk below, after this change passed
as parameter to format_diff_cc_simplified()) can be filled in then,
or at least adjusted correctly?

> (The broken parameters on blob links lead to blob pages which show the
> blob but with a hash instead of a commit message and have broken
> blob_plain (404 - Cannot find file) and tree links (404 - Reading tree
> failed))
> 
> Signed-off-by: Jürgen Kreileder <jk@blackdown.de>

I wonder how we missed this.  Does this happen always, or in some
specific conditions?

Anyway, this change is a good change.  Internal subroutines should not
use global variables.

I hope that in the future we would get rid of most global variables...

Acked-by: Jakub Narebski <jnareb@gmail.com>

[not tested]

>  # create note for patch simplified by combined diff
>  sub format_diff_cc_simplified {
> -	my ($diffinfo, @parents) = @_;
> +	my ($diffinfo, $hash, @parents) = @_;
>  	my $result = '';
>  
>  	$result .= "<div class=\"diff header\">" .
[...]
> @@ -5404,7 +5405,7 @@ sub git_patchset_body {
>  
>  		# generate anchor for "patch" links in difftree / whatchanged part
>  		print "<div class=\"patch\" id=\"patch". ($patch_idx+1) ."\">\n" .
> -		      format_diff_cc_simplified($diffinfo, @hash_parents) .
> +		      format_diff_cc_simplified($diffinfo, $hash, @hash_parents) .
>  		      "</div>\n";  # class="patch"
>  
>  		$patch_number++;
> 


-- 
Jakub Narębski

  reply	other threads:[~2013-04-09 15:26 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-08 20:10 [PATCH 4/4] gitweb: Fix broken blob action parameters on blob/commitdiff pages Jürgen Kreileder
2013-04-09 15:25 ` Jakub Narębski [this message]
2013-04-09 20:17   ` Jürgen Kreileder

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=51643307.9090603@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jk@blackdown.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).