On Wed, 20 Apr 2011 19:07:01 -0500, Jonathan Nieder wrote:
>> However - and this is the key point - if you are going to be mixing
>> tabs and spaces ANYWAY, then you might as well do it in a way that
>> maintains alignment within a tab level regardless of the current
>> setting for the tabwidth:
>
> In principle, I generally agree. But as mentioned in the thread you
> reference, most text editors don't make that very easy.
You just need an editor that can insert into a newly created line the
same leading whitespace characters that were on the previous line (of
course, it's nice if lines with only whitespace are automatically
converted to empty lines); if your text editor can't do that, then any
style of indentation/alignment is going to be painful.
> I personally use a tabwidth of 6 when I really want to concentrate on
> reading. When coding in a rush for other people, that leads to using
> tabstop of 8 and only aligning text that is much shorter than one tab:
>
> if (foo && bar && baz &&
> qux && quux) {
> ...
> } else if (quuux(quuuux, quuuuux,
> long_expresion_comes_here(quuuuuux))) {
> ...
> }
>
> As you can see, this is following the "put continuation lines near the
> right margin" convention advocated in linux-2.6's
> Documentation/CodingStyle.
It's not hard to see why "near the right margin" is an important remark;
with a tabstop of 8, your code is rendered like this:
if (foo && bar && baz &&
qux && quux) {
...
} else if (quuux(quuuux, quuuuux,
long_expresion_comes_here(quuuuuux))) {
...
}
However, with a tabstop of 2, it looks like this:
if (foo && bar && baz &&
qux && quux) {
...
} else if (quuux(quuuux, quuuuux,
long_expresion_comes_here(quuuuuux))) {
...
}
Notice that your `qux && quux' line (which follows the method we both like)
remained nicely aligned, but your `long_expression_comes_here' line has
moved back so much that it doesn't even satisfy the `right margin' rule
anymore.
> Two advantages:
>
> - looks sensible with any tabstop
As we have seen, the sensibility of using tabs for *alignment* is precarious;
on the contrary, using spaces for alignment [across lines of the same level of
tabular indentation] remains *exactly* as sensible as intended by the original
author, regardless of the tabstop setting.
The problem is that there are really two issues that are being conflated"
* Indentation : This can be implemented with tabs
* Alignment : This should be implemented with only spaces
> - no need to cascade changes on following lines when the width of a
> function name changes
That is indeed the best reason to avoid alignment (after all, by its
very nature, alignment implies a coupling between lines), and I find
myself in constant intellectual conflict over that very issue. However,
if the number of lines to be aligned is rather small or alignment
obviously helps readability, then it's a rather minor risk.
Now, your example of a changing function name is a good one; for instance,
consider one of the functions that my patch series introduces:
int parse_date_mode_config_internal(const char *var_date,
const char *var_time_zone,
const char *var,
const char *value,
struct date_mode *d,
int *ret);
I just noticed that the parameters are misaligned by one space; it should
be the following:
int parse_date_mode_config_internal(const char *var_date,
const char *var_time_zone,
const char *var,
const char *value,
struct date_mode *d,
int *ret);
I probably changed the function name by 1 character at some point and then
forgot to update the alignment of the parameters. :-(
Now, I actually hate that style of continuation, and I only used it because
that's basically what everybody else on the planet (and in the git project)
tries to do (albeit with some apalling combination of tabs and spaces).
In such a case, I would much rather treat parameters as `subelements' of
the function construct (prototype, call, etc.) by explicitly giving them
their own indentation level; this transforms the problem from one of
indentation and alignment to a problem of just indentation, thereby allowing
for nothing but tabs:
int parse_date_mode_config_internal
(
const char *var_date,
const char *var_time_zone,
const char *var,
const char *value,
struct date_mode *d,
int *ret
);
A function call might look like this:
parse_date_mode_config_internal
(
"Some value of some sort",
"Another value of some sort",
"dingleberry",
d,
ret
);
Of course, this opens up the debate about where the opening parenthesis should
go; if the `{' of a block is routinely put on the same line as, say, an `if',
then perhaps this would look more natural:
parse_date_mode_config_internal (
"Some value of some sort",
"Another value of some sort",
"dingleberry",
date_mode,
return_value
);
An `if' statement is kind of gross to mold in this way due to the fact that
there are two parts, each with its own subelements; at some point, you might
as well just make some temporary variables with reasonably short names in
order to hold the condition value, thereby obviating this nightmare in the
first place.
However, if you must write every expression explicitly in place, then your
example could be written thusly:
if (
foo && bar &&
baz && qux &&
quux
){
...
} else if (
quuux (
quuuux,
quuuuux,
long_expresion_comes_here (
quuuuuux
)
)
){
...
}
or more drawn out:
if
(
foo &&
bar &&
baz &&
qux &&
quux
)
{
...
}
else if
(
quuux
(
quuuux,
quuuuux,
long_expresion_comes_here
(
quuuuuux
)
)
)
{
...
}
Again, it should be recognized that there are 2 issues:
* Indentation : This can be implemented with tabs
* Alignment : This should be implemented with only spaces
and it should also be recognized that treating tabs as a primitive
means of space-saving compression by having it always represent
some constant number of spaces (8) is just as flaky as expecting
people to properly use spaces for alignment; thus you might as
well go for the latter in order to get the best of both worlds,
because somebody is going to mess it up either way.
Sincerely,
Michael
Witten
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html