git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <junkio@cox.net>
To: Jakub Narebski <jnareb@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/3] gitweb: Whitespace cleanup: realign, reindent
Date: Tue, 22 Aug 2006 18:34:14 -0700	[thread overview]
Message-ID: <7virkkjkvd.fsf@assigned-by-dhcp.cox.net> (raw)
In-Reply-To: 200608221652.50443.jnareb@gmail.com

Jakub Narebski <jnareb@gmail.com> writes:

> This patch tries (but no too hard) to fit gitweb source in 80 columns,
> for 2 columns wide tabs, and indent and align source for better
> readibility.

If you are doing the wrapping, which I think is a good idea, I
do not think you should target for an oddball setting.  Where
did you get the idea of 2-column tabs?

> While at it added comment to 'snapshot' entry defaults for %feature
> hash, corrected "blobl" action in git_blame2 and git_blame to "blob",
> key of argument to $cgi->a from 'class' to '-class'.

All look good otherwise; applied.

> +our %feature = (
> +	# feature => {'sub' => feature-sub, 'override' => allow-override, 'default' => [ default options...]
> +	# if feature is overridable, feature-sub will be called with default options;
> +	# return value indicates if to enable specified feature

I'd prefer if you wrapped these to match the real ones.

> +	'snapshot' => {
> +		'sub' => \&feature_snapshot,
> +		'override' => 0,
> +		#         => [content-encoding, suffix, program]
> +		'default' => ['x-gzip', 'gz', 'gzip']},
>  );

Because making this 3-tuple into a hashref would be an overkill,
it is a very good idea to comment it here.

  reply	other threads:[~2006-08-23  1:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-22 14:51 [PATCH 0/3] gitweb: Yet another cleanup series Jakub Narebski
2006-08-22 14:52 ` [PATCH 1/3] gitweb: Whitespace cleanup: realign, reindent Jakub Narebski
2006-08-23  1:34   ` Junio C Hamano [this message]
2006-08-23  8:55     ` Jakub Narebski
2006-08-23  9:07       ` Andreas Ericsson
2006-08-23  9:55       ` Junio C Hamano
2006-08-22 14:55 ` [PATCH 2/3] gitweb: Use underscore instead of hyphen to separate words in HTTP headers names Jakub Narebski
2006-08-22 14:59 ` [PATCH 3/3] gitweb: Route rest of action subroutines through %actions Jakub Narebski
2006-08-22 15:03 ` [PATCH 0/3] gitweb: Yet another cleanup series Jakub Narebski
2006-08-22 21:42 ` [PATCH 4/3] gitweb: Use here-doc Jakub Narebski
2006-08-22 21:54   ` Jakub Narebski
2006-08-23  1:34   ` Junio C Hamano

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=7virkkjkvd.fsf@assigned-by-dhcp.cox.net \
    --to=junkio@cox.net \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    /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).