git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: Junio C Hamano <junkio@cox.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/4] gitweb: Whitespace cleanup - tabs are for indent, spaces are for align (2)
Date: Sun, 22 Oct 2006 22:36:45 +0200	[thread overview]
Message-ID: <200610222236.45414.jnareb@gmail.com> (raw)
In-Reply-To: <7v3b9g5cde.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Code should be aligned the same way, regardless of tab size.
>> Use tabs for indent, but spaces for align.
> 
> I do not necessarily agree with that policy; the result of
> applying this patch is still inconsistent in some places, and I
> think that is primarily because the policy itself is flawed.

To be true I do those "whitespace cleanup" patches when I notice
that something is mis-aligned for _my_ tab width (2 spaces).
I use Emacs with show-whitespace-mode to not introduce errors
which would be aligned for 2 columsn wide tabs but be misaligned
for more common 5 or 8 characters wide tabs.
 
> For example, a part of sub format_paging_nav looks like this:
> 
>         sub format_paging_nav {
>         >>>>>>>>my ($action, $hash, $head, $page, $nrevs) = @_;
>         ...
>         >>>>>>>>if ($page > 0) {
>         >>>>>>>>>>>>>>>>$paging_nav .= " &sdot; " .
>         >>>>>>>>>>>>>>>>>>>>>>>>$cgi->a({-href => href(action=>$a
>         >>>>>>>>>>>>>>>>>>>>>>>>         -accesskey => "p", -titl
>         >>>>>>>>} else {
>         ...
> 
> If your policy is to indent continuation lines (which is why you
> have a TAB before "$cgi->a"), not having a TAB before the
> continued parameter list for the $cgi->a() call look inconsistent.
> 
> If on the other hand your policy is to align parameters to an
> operator that are spread over multiple lines, " &sdot; " and
> "$cgi-a(..." are left and right parameters to the string
> concatenation operator "." in between them, so "$cgi->a" should
> be pushed back with a run of SP starting at the column that
> begins $paging_nav and aligned with the DQ at the beginning of
> the " &sdot; " string.

That is preferred, and usually used policy, although I sometimes
use former to avoid too long lines.

I never claimed to be perfect...

Nevertheless this patch for example correct situation where the
line which should be aligned is aligned using mixture of tabs and
spaces differing from line to line in the "same alignment" block.

-- 
Jakub Narebski
Poland

  reply	other threads:[~2006-10-22 20:36 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-10-21 15:50 [PATCH 0/4] gitweb: Improving tree view (plus some cleanups) Jakub Narebski
2006-10-21 15:52 ` [PATCH 1/4] gitweb: Whitespace cleanup - tabs are for indent, spaces are for align (2) Jakub Narebski
2006-10-22 20:22   ` Junio C Hamano
2006-10-22 20:36     ` Jakub Narebski [this message]
2006-10-22 21:03       ` Linus Torvalds
2006-10-22 21:27         ` Jakub Narebski
2006-10-22 22:55         ` Junio C Hamano
2006-10-22 23:36         ` Luben Tuikov
2006-10-21 15:53 ` [PATCH 2/4] gitweb: Do not esc_html $basedir argument to git_print_tree_entry Jakub Narebski
2006-10-21 15:53 ` [PATCH 3/4] gitweb: Improve git_print_page_path Jakub Narebski
2006-10-21 15:54 ` [PATCH 4/4] gitweb: Add '..' (up directory) to tree view if applicable Jakub Narebski

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