threads / patch / 5474

patch, 4 partsgitweb: Change the name of diff to parent link in "commit" view to "diff"

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

## tl;dr

14 messages between Sep 4, 2006 and Sep 5, 2006. Diffs are folded; open one to read it.

replies: 13people: 3as markdown or json

Jakub Narebski· Sep 4, 2006, 16:10 UTC · lore

[PATCH 0/4] gitweb: Some improvements

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· Sep 4, 2006, 16:13 UTC · re: Jakub Narebski · lore

PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type

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(-)
Show changes to 3 files +7 −0

Makefile, gitweb/git-favicon.png, gitweb/gitweb.perl

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
Linus Torvalds· Sep 4, 2006, 17:15 UTC · re: Jakub Narebski · lore

Re: PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type

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· Sep 4, 2006, 18:32 UTC · re: Jakub Narebski · lore

[PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type

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(-)
Show changes to 3 files +7 −0

Makefile, gitweb/git-favicon.png, gitweb/gitweb.perl

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· Sep 4, 2006, 18:34 UTC · re: Jakub Narebski · lore

Re: [PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type

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· Sep 4, 2006, 22:46 UTC · re: Jakub Narebski · lore

Re: [PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type

Jakub Narebski <jnareb@gmail.com> writes:
Show 5 quoted lines
> 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· Sep 4, 2006, 23:00 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type

Junio C Hamano wrote:
Show 12 quoted lines
> 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· Sep 4, 2006, 23:17 UTC · re: Jakub Narebski · lore

Re: [PATCH 1/4] gitweb: Add GIT favicon, assuming image/png type

Jakub Narebski <jnareb@gmail.com> writes:
Show 16 quoted lines
> 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.

Jakub Narebski· Sep 4, 2006, 16:15 UTC · re: Jakub Narebski · lore

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(-)
Show changes to gitweb/gitweb.perl +1 −1
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· Sep 4, 2006, 16:17 UTC · re: Jakub Narebski · lore

[PATCH 3/4] gitweb: Divide page path into directories -- path's "breadcrumbs"

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(-)
Show changes to gitweb/gitweb.perl +21 −8
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· Sep 4, 2006, 16:19 UTC · re: Jakub Narebski · lore

[PATCH 4/4] gitweb: Correct typo: '==' instead of 'eq' in git_difftree_body

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(-)
Show changes to gitweb/gitweb.perl +4 −4
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
Junio C Hamano· Sep 4, 2006, 23:22 UTC · re: Jakub Narebski · lore

Re: [PATCH 0/4] gitweb: Some improvements

Jakub Narebski <jnareb@gmail.com> writes:
Show 6 quoted lines
> 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· Sep 4, 2006, 23:53 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/4] gitweb: Some improvements

Junio C Hamano wrote:
Show 18 quoted lines
> 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· Sep 5, 2006, 08:59 UTC · re: Jakub Narebski · lore

Re: gitweb testing and benchmarking (was: [PATCH 0/4] gitweb: Some improvements)

Jakub Narebski wrote:
Show 16 quoted lines
> 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

← back to recent threads