Re: [PATCH] gitweb: tree view: eliminate redundant "blob"

[Jakub Narebski <jnareb@gmail.com>]

Junio C Hamano wrote:
> Junio C Hamano <junkio@cox.net> writes:
> 
>> Jakub Narebski <jnareb@gmail.com> writes:
>>
>>> ... That means that we cannot distinguish really well (at 
>>> least color) between tree and blob entries.
>>
>> Do we even say links are blue and underlined by forcing that in
>> our css?
>>
>> Doesn't leading drwxr-xr-x mean anything?
>>
>> Why is making the distinction important in the first place?
> 
> Anyhow, I was too tired to sleep after an unscheduled day-job on
> Sunday X-<, and whipped this up for fun.
> 
> -- >8 --
> [PATCH] gitweb: remove UNIXy mode bits from tree display
> 
> and replace it with an image icon for cuteness ;-).

[...]
> +td.executable {
> +  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAACVJREFUCNdjYGBgYGdgMEhgUFBgYGJgcBKAIiAbKAIUB8oyMAAANBcCqbivEbgAAAAASUVORK5CYII=);
> +  background-repeat: no-repeat;
> +}
> +
> +td.folder {
> +  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAAB1JREFUCNdjYGBgkGFgUGJgcPzA4CCABdV/ACoBAFTTBQ822ZerAAAAAElFTkSuQmCC);
> +  background-repeat: no-repeat;
> +}
> +
> +td.regular {
> +  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAABtJREFUCNdjqD/A4JDA4BDA4FDB4MCBHdX/AACO5wbfUNnbqwAAAABJRU5ErkJggg==);
> +  background-repeat: no-repeat;
> +}
> +
> +td.symlink {
> +  background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAA0AAAANAQMAAABIJXY/AAAABlBMVEX///+UAN7OszyBAAAAAXRSTlMAQObYZgAAACVJREFUCNdjYGBg4G9gMEhgUFBgcBAAIQYYYlFg4ElgkG8AKgEARSsDX750+Y0AAAAASUVORK5CYII=);
> +  background-repeat: no-repeat;
> +}
> +

Wouldn't it be better to use out-of-line images? I'm not sure if all browsers
support CSS embedded images, and if all browsers can cache such images.

Perhaps we could reuse Apache's MultiView/index.html.var... err, gitweb
tries to be web server agnostic...

And what about text browsers? It would be better to use <img> element, with
alt attribute set either to old UNIX-y mode bits, and title set to the file
type... or perhaps both alt and title attributes set to file type.

>  # convert file mode in octal to symbolic file mode string
> +sub kind_class {
> +	my ($type, $mode) = @_;
> +	$mode = oct $mode;
> +	if (S_ISDIR($mode & S_IFMT)) {
> +		return 'folder';
> +	} elsif (S_ISLNK($mode)) {
> +		return 'symlink';
> +	} elsif (S_ISREG($mode)) {
> +		# git cares only about the executable bit
> +		if ($mode & S_IXUSR) {
> +			return 'executable';
> +		} else {
> +			return 'regular';
> +		};
> +	}
> +}
> +

We have file_type subroutine, which does almost the same work. It doesn't
mark file as "executable", and it uses "directory" instead of non-standard
"folder".

By the way, $type argument (parameter) is not used at all.

> @@ -1651,7 +1668,9 @@ sub git_print_tree_entry {
>  	# the mode of the entry, list is the name of the entry, an href,
>  	# and link is the action links of the entry.
>  
> -	print "<td class=\"mode\">" . mode_str($t->{'mode'}) . "</td>\n";
> +	my $kind = kind_class($t->{'type'}, $t->{'mode'});
> +	print "<td class=\"$kind\">&nbsp;</td>\n";
> +
>  	if ($t->{'type'} eq "blob") {
>  		print "<td class=\"list\">" .
>  			$cgi->a({-href => href(action=>"blob", hash=>$t->{'hash'},

-- 
Jakub Narebski
Poland