# [PATCH 2/4] gitweb: Change the name of diff to parent link in "commit" view to "diff"

14 messages from 2006-09-04 to 2006-09-05. Participants: Jakub Narebski, Linus Torvalds, Junio C Hamano.
Thread: https://gitlist.dev/t/5474

## Jakub Narebski, 2006-09-04 16:10

Subject: [PATCH 0/4] gitweb: Some improvements
Message-ID: <200609041810.09838.jnareb@gmail.com>
URL: https://gitlist.dev/e/200609041810.09838.jnareb%40gmail.com

```
This is small series of patches, applicable both to 'master' and 'next',
which adds simple improvements, or in the case of last patch
fix bug/typo.

First two patches were sent earlier, I don't know if they were dropped
or forgotten. 

---
Diffstat:
 Makefile               |    2 ++
 gitweb/git-favicon.png |  Bin
 gitweb/gitweb.perl     |   44 +++++++++++++++++++++++++++++++-------------
 3 files changed, 33 insertions(+), 13 deletions(-)

Shortlog:
 gitweb: Add GIT favicon, assuming image/png type.
 gitweb: Change the name of diff to parent link in "commit" view to "diff"
 gitweb: Divide page path into directories -- path's "breadcrumbs"
 gitweb: Correct typo: '==' instead of 'eq' in git_difftree_body

-- 
Jakub Narebski
ShadeHawk on #git
Poland

```

## Jakub Narebski, 2006-09-04 16:13

Subject: PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type
Message-ID: <200609041813.49103.jnareb@gmail.com>
URL: https://gitlist.dev/e/200609041813.49103.jnareb%40gmail.com
In-Reply-To: <200609041810.09838.jnareb@gmail.com>

```
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is my first post with binary patch. I'd rather not send file
git-favicon.png as an attachement due to filtering.

You can see (and download) the favicon istelf at
  http://git.or.cz/gitwiki/GitRelatedLogos
(the first favicon).

 Makefile               |    2 ++
 gitweb/git-favicon.png |  Bin
 gitweb/gitweb.perl     |    5 +++++
 3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 199fbe8..f3b762d 100644
--- a/Makefile
+++ b/Makefile
@@ -137,6 +137,7 @@ GITWEB_LIST =
 GITWEB_HOMETEXT = indextext.html
 GITWEB_CSS = gitweb.css
 GITWEB_LOGO = git-logo.png
+GITWEB_FAVICON = git-favicon.png
 
 export prefix bindir gitexecdir template_dir GIT_PYTHON_DIR
 
@@ -674,6 +675,7 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
 	    -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \
 	    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
 	    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
+	    -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
 	    $< >$@+
 	chmod +x $@+
 	mv $@+ $@
diff --git a/gitweb/git-favicon.png b/gitweb/git-favicon.png
new file mode 100644
index 0000000..de637c0
Binary files /dev/null and b/gitweb/git-favicon.png differ
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7961945..bebaa0f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -48,6 +48,8 @@ # URI of default stylesheet
 our $stylesheet = "++GITWEB_CSS++";
 # URI of GIT logo
 our $logo = "++GITWEB_LOGO++";
+# URI of GIT favicon, assumed to be image/png type
+our $favicon = "++GITWEB_FAVICON++";
 
 # source of projects list
 our $projects_list = "++GITWEB_LIST++";
@@ -1192,6 +1194,9 @@ EOF
 		       'href="%s" type="application/rss+xml"/>'."\n",
 		       esc_param($project), href(action=>"rss"));
 	}
+	if (defined $favicon) {
+		print qq(<link rel="shortcut icon" href="$favicon" type="image/png"/>\n);
+	}
 
 	print "</head>\n" .
 	      "<body>\n" .
-- 
1.4.1.1

```

## Jakub Narebski, 2006-09-04 16:15

Subject: [PATCH 2/4] gitweb: Change the name of diff to parent link in "commit" view to "diff"
Message-ID: <200609041815.29203.jnareb@gmail.com>
URL: https://gitlist.dev/e/200609041815.29203.jnareb%40gmail.com
In-Reply-To: <200609041810.09838.jnareb@gmail.com>

```
Change the name of diff to parent (current commit to one of parents)
link in "commit" view (git_commit subroutine) from "commitdiff" to
"diff".  Let's leave "commitdiff" for equivalent of git-show, or
git-diff-tree with one revision, i.e. diff for a given commit to its
parent (parents).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The commit message is barely smaller than the diff itself, but it
is expected as commit message tell the why behind the change.

 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index bebaa0f..3b5c0e2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2758,7 +2758,7 @@ sub git_commit {
 		      "<td class=\"link\">" .
 		      $cgi->a({-href => href(action=>"commit", hash=>$par)}, "commit") .
 		      " | " .
-		      $cgi->a({-href => href(action=>"commitdiff", hash=>$hash, hash_parent=>$par)}, "commitdiff") .
+		      $cgi->a({-href => href(action=>"commitdiff", hash=>$hash, hash_parent=>$par)}, "diff") .
 		      "</td>" .
 		      "</tr>\n";
 	}
-- 
1.4.1.1

```

## Jakub Narebski, 2006-09-04 16:17

Subject: [PATCH 3/4] gitweb: Divide page path into directories -- path's "breadcrumbs"
Message-ID: <200609041817.59054.jnareb@gmail.com>
URL: https://gitlist.dev/e/200609041817.59054.jnareb%40gmail.com
In-Reply-To: <200609041810.09838.jnareb@gmail.com>

```
Divide page path into directories, so that each part of path links to
the "tree" view of the $hash_base (or HEAD, if $hash_base is not set)
version of the directory.

If the entity is blob, final part (basename) links to $hash_base or
HEAD revision of the "raw" blob ("blob_plain" view).  If the entity is
tree, link to the "tree" view.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   29 +++++++++++++++++++++--------
 1 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3b5c0e2..4240a30 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1375,19 +1375,32 @@ sub git_print_page_path {
 
 	if (!defined $name) {
 		print "<div class=\"page_path\">/</div>\n";
-	} elsif (defined $type && $type eq 'blob') {
+	} else {
+		my @dirname = split '/', $name;
+		my $basename = pop @dirname;
+		my $fullname = '';
+
 		print "<div class=\"page_path\">";
-		if (defined $hb) {
+		foreach my $dir (@dirname) {
+			$fullname .= $dir . '/';
+			print $cgi->a({-href => href(action=>"tree", file_name=>$fullname,
+			                             hash_base=>$hb),
+			              -title => $fullname}, esc_html($dir));
+			print "/";
+		}
+		if (defined $type && $type eq 'blob') {
 			print $cgi->a({-href => href(action=>"blob_plain", file_name=>$file_name,
-			                             hash_base=>$hb)},
-			              esc_html($name));
+			                             hash_base=>$hb),
+			              -title => $name}, esc_html($basename));
+		} elsif (defined $type && $type eq 'tree') {
+			print $cgi->a({-href => href(action=>"tree", file_name=>$file_name,
+			                             hash_base=>$hb),
+			              -title => $name}, esc_html($basename));
+			print "/";
 		} else {
-			print $cgi->a({-href => href(action=>"blob_plain", file_name=>$file_name)},
-			              esc_html($name));
+			print esc_html($basename);
 		}
 		print "<br/></div>\n";
-	} else {
-		print "<div class=\"page_path\">" . esc_html($name) . "<br/></div>\n";
 	}
 }
 
-- 
1.4.1.1

```

## Jakub Narebski, 2006-09-04 16:19

Subject: [PATCH 4/4] gitweb: Correct typo: '==' instead of 'eq' in git_difftree_body
Message-ID: <200609041819.58737.jnareb@gmail.com>
URL: https://gitlist.dev/e/200609041819.58737.jnareb%40gmail.com
In-Reply-To: <200609041810.09838.jnareb@gmail.com>

```
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This caused that the "commitdiff" version, with "patch" link to anchor
instead of "diff" link to "blobdiff" view was used even in "commit" view.

 gitweb/gitweb.perl |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4240a30..e7d7bd7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1519,7 +1519,7 @@ sub git_difftree_body {
 			      $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
 			                             hash_base=>$hash, file_name=>$diff{'file'})},
 			              "blob");
-			if ($action == "commitdiff") {
+			if ($action eq 'commitdiff') {
 				# link to patch
 				$patchno++;
 				print " | " .
@@ -1540,7 +1540,7 @@ sub git_difftree_body {
 			                             hash_base=>$parent, file_name=>$diff{'file'})},
 			              "blob") .
 			      " | ";
-			if ($action == "commitdiff") {
+			if ($action eq 'commitdiff') {
 				# link to patch
 				$patchno++;
 				print " | " .
@@ -1586,7 +1586,7 @@ sub git_difftree_body {
 			                             hash_base=>$hash, file_name=>$diff{'file'})},
 			              "blob");
 			if ($diff{'to_id'} ne $diff{'from_id'}) { # modified
-				if ($action == "commitdiff") {
+				if ($action eq 'commitdiff') {
 					# link to patch
 					$patchno++;
 					print " | " .
@@ -1628,7 +1628,7 @@ sub git_difftree_body {
 			                             hash=>$diff{'to_id'}, file_name=>$diff{'to_file'})},
 			              "blob");
 			if ($diff{'to_id'} ne $diff{'from_id'}) {
-				if ($action == "commitdiff") {
+				if ($action eq 'commitdiff') {
 					# link to patch
 					$patchno++;
 					print " | " .
-- 
1.4.1.1

```

## Linus Torvalds, 2006-09-04 17:15

Subject: Re: PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type
Message-ID: <Pine.LNX.4.64.0609041010540.27779@g5.osdl.org>
URL: https://gitlist.dev/e/Pine.LNX.4.64.0609041010540.27779%40g5.osdl.org
In-Reply-To: <200609041813.49103.jnareb@gmail.com>

```


On Mon, 4 Sep 2006, Jakub Narebski wrote:
>
> This is my first post with binary patch. I'd rather not send file
> git-favicon.png as an attachement due to filtering.

You forgot the "--binary" flag, so it didn't actually create a binary 
patch, it just did the "Binary files .. differ" thing:

> diff --git a/gitweb/git-favicon.png b/gitweb/git-favicon.png
> new file mode 100644
> index 0000000..de637c0
> Binary files /dev/null and b/gitweb/git-favicon.png differ

See diff.c: builtin_diff() for the logic:

	...
                if (o->binary)
                        emit_binary_diff(&mf1, &mf2);
                else
                        printf("Binary files %s and %s differ\n",
                               lbl[0], lbl[1]);
	...

ie the default is to act like a regular "diff", and we do the extended git 
binary diffs only if you ask for them explicitly.

(Of course, since we do the _other_ extended git diff headers regardless, 
maybe that doesn't make much sense. On the other hand, the normal extended 
git diff headers are compatible with normal patches, so there is a real 
difference, and so arguably the "--binary" flag makes tons of sense).

		Linus

```

## Jakub Narebski, 2006-09-04 18:32

Subject: [PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type
Message-ID: <200609042032.13742.jnareb@gmail.com>
URL: https://gitlist.dev/e/200609042032.13742.jnareb%40gmail.com
In-Reply-To: <200609041813.49103.jnareb@gmail.com>

```
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Corrected version, with the actual binary diff
(that was the version sent earlier, by the way).

 Makefile               |    2 ++
 gitweb/git-favicon.png |  Bin
 gitweb/gitweb.perl     |    5 +++++
 3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 199fbe87384cd3f4686916277dd124cefc751e8a..f3b762de92e579300eba3f53ee26b2a80b4e76c7 100644
--- a/Makefile
+++ b/Makefile
@@ -137,6 +137,7 @@ GITWEB_LIST =
 GITWEB_HOMETEXT = indextext.html
 GITWEB_CSS = gitweb.css
 GITWEB_LOGO = git-logo.png
+GITWEB_FAVICON = git-favicon.png
 
 export prefix bindir gitexecdir template_dir GIT_PYTHON_DIR
 
@@ -674,6 +675,7 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
 	    -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \
 	    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
 	    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
+	    -e 's|++GITWEB_FAVICON++|$(GITWEB_FAVICON)|g' \
 	    $< >$@+
 	chmod +x $@+
 	mv $@+ $@
diff --git a/gitweb/git-favicon.png b/gitweb/git-favicon.png
new file mode 100644
index 0000000000000000000000000000000000000000..de637c0608090162a6ce6b51d5f9bfe512cf8bcf
GIT binary patch
literal 164
zc-rd>@N?(olHy`uVBq!ia0vp^0wB!93?!50ihlx9JOMr-t_OgO28RD&a4c8tKak5=
z;1OBOz`!jG!i)^F=12eq?L1u^Ln;_q4{j86a1dcV@b%g0mmUiOK9(+Io+#BK-XURJ
z*52lzAh4o%_q+oa1XgVS7Wa3@eurhH>!fs<8s*Qab3eLq`JX({BnD4cKbLh*2~7aN
C3N}^%

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7961945630e44e94649f3ccd9f96a962c94af0d9..bebaa0fb1e93995da8ea19af8b70a453b9572488 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -48,6 +48,8 @@ # URI of default stylesheet
 our $stylesheet = "++GITWEB_CSS++";
 # URI of GIT logo
 our $logo = "++GITWEB_LOGO++";
+# URI of GIT favicon, assumed to be image/png type
+our $favicon = "++GITWEB_FAVICON++";
 
 # source of projects list
 our $projects_list = "++GITWEB_LIST++";
@@ -1192,6 +1194,9 @@ EOF
 		       'href="%s" type="application/rss+xml"/>'."\n",
 		       esc_param($project), href(action=>"rss"));
 	}
+	if (defined $favicon) {
+		print qq(<link rel="shortcut icon" href="$favicon" type="image/png"/>\n);
+	}
 
 	print "</head>\n" .
 	      "<body>\n" .
-- 
1.4.1.1

```

## Jakub Narebski, 2006-09-04 18:34

Subject: Re: [PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type
Message-ID: <edhrij$l9j$1@sea.gmane.org>
URL: https://gitlist.dev/e/edhrij%24l9j%241%40sea.gmane.org
In-Reply-To: <200609042032.13742.jnareb@gmail.com>

```
Jakub Narebski wrote:

> index 199fbe87384cd3f4686916277dd124cefc751e8a..f3b762de92e579300eba3f53ee26b2a80b4e76c7 100644

Hmmm... I do wonder why --binary implies --full-index option.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

```

## Junio C Hamano, 2006-09-04 22:46

Subject: Re: [PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type
Message-ID: <7virk3l07a.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7virk3l07a.fsf%40assigned-by-dhcp.cox.net
In-Reply-To: <edhrij$l9j$1@sea.gmane.org>

```
Jakub Narebski <jnareb@gmail.com> writes:

> Jakub Narebski wrote:
>
>> index 199fbe87384cd3f4686916277dd124cefc751e8a..f3b762de92e579300eba3f53ee26b2a80b4e76c7 100644
>
> Hmmm... I do wonder why --binary implies --full-index option.

Patch safety.  Yours happened to be a creation patch so it might
not matter in this particular case, but we would want to be
reasonably sure there is no ambiguity in the preimage when you
are doing a modification patch.

```

## Jakub Narebski, 2006-09-04 23:00

Subject: Re: [PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type
Message-ID: <edib58$61k$1@sea.gmane.org>
URL: https://gitlist.dev/e/edib58%2461k%241%40sea.gmane.org
In-Reply-To: <7virk3l07a.fsf@assigned-by-dhcp.cox.net>

```
Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Jakub Narebski wrote:
>>
>>> index 199fbe87384cd3f4686916277dd124cefc751e8a..f3b762de92e579300eba3f53ee26b2a80b4e76c7 100644
>>
>> Hmmm... I do wonder why --binary implies --full-index option.
> 
> Patch safety.  Yours happened to be a creation patch so it might
> not matter in this particular case, but we would want to be
> reasonably sure there is no ambiguity in the preimage when you
> are doing a modification patch.
 
Couldn't we do this (show full sha1) only for the binary part of patch?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

```

## Junio C Hamano, 2006-09-04 23:17

Subject: Re: [PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type
Message-ID: <7vejurkyro.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7vejurkyro.fsf%40assigned-by-dhcp.cox.net
In-Reply-To: <edib58$61k$1@sea.gmane.org>

```
Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano wrote:
>
>> Jakub Narebski <jnareb@gmail.com> writes:
>> 
>>> Jakub Narebski wrote:
>>>
>>>> index 199fbe87384cd3f4686916277dd124cefc751e8a..f3b762de92e579300eba3f53ee26b2a80b4e76c7 100644
>>>
>>> Hmmm... I do wonder why --binary implies --full-index option.
>> 
>> Patch safety.  Yours happened to be a creation patch so it might
>> not matter in this particular case, but we would want to be
>> reasonably sure there is no ambiguity in the preimage when you
>> are doing a modification patch.
>  
> Couldn't we do this (show full sha1) only for the binary part of patch?

Personally I would say why bother, but patches welcome if
cleanly done.

```

## Junio C Hamano, 2006-09-04 23:22

Subject: Re: [PATCH 0/4] gitweb: Some improvements
Message-ID: <7v8xkzkyie.fsf@assigned-by-dhcp.cox.net>
URL: https://gitlist.dev/e/7v8xkzkyie.fsf%40assigned-by-dhcp.cox.net
In-Reply-To: <200609041810.09838.jnareb@gmail.com>

```
Jakub Narebski <jnareb@gmail.com> writes:

> This is small series of patches, applicable both to 'master' and 'next',
> which adds simple improvements, or in the case of last patch
> fix bug/typo.
>
> First two patches were sent earlier, I don't know if they were dropped
> or forgotten. 

I think you were just expecting too quick turnaround ;-).

If I recall correctly it used to be that we could not use png
for favicon but it seems that things have improved.

It is a while since I tried gitweb on my machine the last time
but was it always this slow I wonder...  We probably would need
a good benchmark and automated test before going too much
further.

```

## Jakub Narebski, 2006-09-04 23:53

Subject: Re: [PATCH 0/4] gitweb: Some improvements
Message-ID: <ediea9$d3d$2@sea.gmane.org>
URL: https://gitlist.dev/e/ediea9%24d3d%242%40sea.gmane.org
In-Reply-To: <7v8xkzkyie.fsf@assigned-by-dhcp.cox.net>

```
Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> This is small series of patches, applicable both to 'master' and 'next',
>> which adds simple improvements, or in the case of last patch
>> fix bug/typo.
>>
>> First two patches were sent earlier, I don't know if they were dropped
>> or forgotten. 
> 
> I think you were just expecting too quick turnaround ;-).
> 
> If I recall correctly it used to be that we could not use png
> for favicon but it seems that things have improved.
> 
> It is a while since I tried gitweb on my machine the last time
> but was it always this slow I wonder...  We probably would need
> a good benchmark and automated test before going too much
> further.

The problem is that before commit 5d043a3d856bd40d8b34b8836a561e438d23573b
  gitweb: fill in gitweb configuration by Makefile
by Martin Waitz one had to modify gitweb script to change the configuration
from default.

But benchmarking is good. Simple time to run script from command line, with
environment variables GATEWAY_INTERFACE="CGI/1.1", HTTP_ACCEPT="*/*",
REQUEST_METHOD="GET" and of course QUERY_STRING set, and perhaps using
ApacheBench.


>From my experience with gitweb, the longest it take to show summary and tags
views (for a large number of tags, which grows linearly with history), and
to show history (and of course blame).
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

```

## Jakub Narebski, 2006-09-05 08:59

Subject: Re: gitweb testing and benchmarking (was: [PATCH 0/4] gitweb: Some improvements)
Message-ID: <edje8a$sdo$1@sea.gmane.org>
URL: https://gitlist.dev/e/edje8a%24sdo%241%40sea.gmane.org
In-Reply-To: <ediea9$d3d$2@sea.gmane.org>

```
Jakub Narebski wrote:

> Junio C Hamano wrote:
>
>> It is a while since I tried gitweb on my machine the last time
>> but was it always this slow I wonder...  We probably would need
>> a good benchmark and automated test before going too much
>> further.
> 
> The problem is that before commit 5d043a3d856bd40d8b34b8836a561e438d23573b
>   gitweb: fill in gitweb configuration by Makefile
> by Martin Waitz one had to modify gitweb script to change
> the configuration from default.
> 
> But benchmarking is good. Simple time to run script from command line,
> with environment variables GATEWAY_INTERFACE="CGI/1.1", HTTP_ACCEPT="*/*",
> REQUEST_METHOD="GET" and of course QUERY_STRING set, and perhaps using
> ApacheBench.

I think that for testing gitweb we will be better with creating some twisted
test repository (c.f. t/t4114-apply-typechange.sh), but for benchmarking it
would be better to use some larger repository, for example git repository
cut-off from above at some version (including tags!). Is there a way to do
that?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

```