[phobos] lots of spurious code that doesn't work

[Jonathan M Davis]

On Saturday 22 January 2011 20:28:32 Andrei Alexandrescu wrote:
> FWIW that style guide is useless.
> 
> It's simplest to do what the existing code does. It's okay to adapt one
> to a specific style inside of a library, but mixing styles inside the
> same file is not very pleasant. For example, does it kill whoever likes
> this:
> 
> if(expression)
> 
> to insert one space between the if and the paren? Now we have 1% of all
> ifs different from all others. What good does that do?
> 
> Then we have of course
> 
> if( expression )
> 
> which accounts for about 10% of all ifs (looks like Sean and Don are
> using that one) and finally the oddest of all:
> 
> if( expression)
> 
> I'd say at least if making small edits for one module it's common
> courtesy to keep whatever style the module already had instead of
> debating (and inconsistently inserting) the relative merits of different
> styles.

Well, I would point out that the style guide doesn't actually say anything about 
where braces or parens go or anything like that. It's pretty short, and a good 
chunk of it is just naming conventions. I pointed out the blank line thing 
because you were advocating something which was in contradiction to the style 
guide, and the style guide claims that that's what submissions to Phobos are 
supposed to use. So, if we're really trying to follow that style guide, we 
either need to update the style guide or change what we're doing to match it. Or 
we can change it to not say that that's what Phobos requires. There _are_ people 
who look at the style guide and then wonder why Phobos doesn't match it. It _is_ 
primarily a naming convention issue though - particularly because the guide does 
_not_ say anythingc about where you put braces or parens.

So, I'm not really advocating that we push a single style throughout Phobos 
beyond naming conventions (since that _does_ affect the API). I'm just pointing 
out that it looks bad when the style guide says one thing, and we do another - 
particularly when the style guide claims that it's what Phobos does. It's fine 
with me if someone puts one blank lines betwen functions (though I'd prefer to 
be able to use two). If someone wants to put extra spaces around their parens, I 
don't like it, but I'm not going to complain about it and insist that we try and 
be stylistically consistent.

All I was trying to do was point out that what you were suggesting that we check 
with a style checker was contradicted the style guide that the official site 
claims that we follow. And if there's something that we really want to do which 
doesn't match the style guide, then we should either change the style guide to 
match (unfortunately, there _are_ people who read it and expect us to follow it 
and complain about it when we don't) or change it so that it doesn't claim that 
Phobos uses it.

Personally, I think what really matters stylistically is how we name things in 
the public API. It would be nice if certain things are common in code in the D 
community in general (such as the declaration style putting the * next to the 
type, not the variable name), but what really matters is the public API. And we 
have been doing work to make it so that Phobos' public API is consistent - in 
particular some of the changes that you've been making to std.string of late.

- Jonathan M Davis