Re: [PATCH] gitweb: Add charset info to "raw" blob output

From: Junio C Hamano <gitster@pobox.com>
Date: May 31, 2008, 18:22 UTC
Message-ID: <7vprr2fi5z.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080531112513.30913.44393.stgit@localhost.localdomain>

Jakub Narebski <jnareb@gmail.com> writes:

Show 6 quoted lines

> Always add charset info from $default_text_plain_charset (if it is
> defined) to "raw" (a=blob_plain) output for 'text/plain' blobs.
> Adding charset info in a special case was removed from blob_mimetype().
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---

Looks Ok but it took a bit of digging on the list for me to figure out that something like this was missing from the beginning of your commit log message:

	Earlier "blob_plain" view sent "charset=utf-8" only when gitweb
	guessed the content type to be text by reading from it, and not
	when the MIME type was obtained from /etc/mime.types.

	This fixes the bug by always adding....

But I wonder if moving of this to the calling site is the right thing to do. Wouldn't it become much more contained and robust if you did it this way?

 gitweb/gitweb.perl |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 57a1905..f5338e1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2471,29 +2471,33 @@ sub mimetype_guess {
 sub blob_mimetype {
 	my $fd = shift;
 	my $filename = shift;
+	my $mime;
 
 	if ($filename) {
-		my $mime = mimetype_guess($filename);
-		$mime and return $mime;
-	}
-
-	# just in case
-	return $default_blob_plain_mimetype unless $fd;
-
-	if (-T $fd) {
-		return 'text/plain' .
-		       ($default_text_plain_charset ? '; charset='.$default_text_plain_charset : '');
+		$mime = mimetype_guess($filename);
+	} else if (!defined $fd) {
+		$mime = $default_blob_plain_mimetype;
+	} else if (-T $fd) {
+		$mime = 'text/plain';
 	} elsif (! $filename) {
-		return 'application/octet-stream';
+		$mime = 'application/octet-stream';
 	} elsif ($filename =~ m/\.png$/i) {
-		return 'image/png';
+		$mime = 'image/png';
 	} elsif ($filename =~ m/\.gif$/i) {
-		return 'image/gif';
+		$mime = 'image/gif';
 	} elsif ($filename =~ m/\.jpe?g$/i) {
-		return 'image/jpeg';
+		$mime = 'image/jpeg';
 	} else {
-		return 'application/octet-stream';
+		$mime = 'application/octet-stream';
 	}
+
+	# Type specific postprocessing can be added as needed...
+	if ($mime =~ /^text\//i &&
+	    $mime !~ /charset=/i && $default_text_plain_charset) {
+		$mime .=  '; charset='.$default_text_plain_charset;
+	}
+
+	return $mime;
 }
 
 ## ======================================================================