Add note about use of whitespace in official code

[JakobOvrum]

No description provided.

[tsbockman]

Thanks. I don't particularly care what the formatting rules are - just don't make me guess.

[JakobOvrum]

Yes, I feel your pain. Thanks for pointing out the omission (I'm sure there are more).

[Hackerpilot]

Looks good to me. This is consistent with the default behavior of dfmt.

[CyberShadow]

IIRC a similar earlier proposal was rejected, but I don't think I'll be able to find the thread. @jmdavis ?

[andralex]

Please also add "Chains containing else if (...) or else static if (...) should set the keywords on the same line".

[andralex]

(to the phobos rules, not the specific rules)

[yebblies]

There shouldn't be a space after cast(). The ddmd source style was picked based on the most common usages in phobos.

[jmdavis]

Well, prior to this, we've specifically avoided making requirements about whitespace or other formatting rules (beyond the character limit and bracing). Beyond that, the only real requirement is that the code within a module be reasonably consistent (though what that means is easily up for debate - sometimes someone has complained about something not being consistent when someone else thought that it was). And while some of the major modules do follow the rules laid out here, many don't (so, if we merge this, a good chunk of Phobos won't follow the style guide).

Personally, I really don't like having whitespace after if, while, etc. (it's pointless noise in the code IMHO), and none of the modules that I have authored have been formatted that way, but Andrei does like it and has pushed for it when he's reviewed stuff. So, while when we've discussed this previously, we've either decided not to add formatting rules like this or not come to any particular decision about it, it still keeps coming up from time to time.

So, if it's up to me, we won't merge this and will leave the style guide as-is, but I expect that Andrei will make an executive decision on this and decide that we're going to do it.

[tsbockman]

So, if it's up to me, we won't merge this and will leave the style guide as-is, but I expect that Andrei will make an executive decision on this and decide that we're going to do it.

@jmdavis This came up because I complained about Andrei telling me to conform my code to rules that aren't in the current style guide.

I don't care whether these new rules get merged or not, but there needs to be agreement that PR authors aren't going to be bugged about formatting issues unless they're actually part of the official standard. Anything else is just frustrating, particularly for new contributors.

[jmdavis]

@tsbockman Like I said, Andrei likes to bring it up, though plenty of druntime and Phobos do not follow that style, and it's never been agreed that we adopt a particular whitespace style for all official code. But given how Andrei feels about it, I expect that he'll keep bringing it up when reviewing code regardless of whether it's in the official style guide or not. And while I don't agree with that, he's one of the two heads of the project, so he'll likely just take this opportunity to set his way in stone rather than letting folks format their code within the general guidelines that we've had.

But I do agree that we need to either not be harping on stuff that's not in the official style guide or to put what we're going to require in the official style guide. I'd just prefer that we go the route of not harping on it than to add something specific about whitespace formatting to the official style guide.

[tsbockman]

But I do agree that we need to either not be harping on stuff that's not in the official style guide or to put what we're going to require in the official style guide. I'd just prefer that we go the route of not harping on it than to add something specific about whitespace formatting to the official style guide.

That would be my preference also (mostly because my personal style is pretty different from the Phobos quasi-standard) - unless we could just run everything (including the existing code) through dfmt. Then I'd consider the improved readability for the existing code, and the knowledge that no one would bug me about formatting if I just ran my code through the tool before submission, a good trade for the loss of flexibility.

[JakobOvrum]

I sure wish my other pull requests to dlang.org would get this kind of attention :)

[CyberShadow]

@JakobOvrum Now that I'm home LMK if there's something I could quicky go over.

[jmdavis]

I will say that as much as I'd prefer that we leave the style guide as-is and not get pickier about whitespace and formatting, if we do decide that we want to be pickier about it, rather than add more Phobos-specific rules to the style guide, it would make more sense to create a config file for dfmt (or whatever is used to configure it) that follows what Andrei wants and just require that it be used on official code. Then at least, we wouldn't have to specify the little details about whitespace, and folks working on Phobos wouldn't have to figure out how they're doing it wrong.

When all we really require is that the bracing be a particular style and that the lines not be beyond a certain length, it's really not that hard for someone to just code stuff up that looks reasonably good and follows the rules, but the more rules we add, the harder it gets for someone to make sure that they follow them, whereas while formatters tend to make things a bit ugly (at least in corner cases), at least then we wouldn't be having to tell anyone to fix whitespace (especially if dfmt were run on the code via a commit hook).

That being said, I'd still prefer that we just didn't get picky about whitespace or formatting beyond the little on it that's currently in the style guide, but if we are going to be picky about it, let's at least do it in a way that doesn't cause problems for contributors by just using a formatter to define the rules (in which case, we wouldn't need to update the style guide beyond saying that you had to use dfmt with the dfmt config file in the repo - and we could do stuff like add make target for that to make it easy).

[ntrel]

LGTM

[yebblies]

Still wrong on cast.

[ntrel]

Still wrong on cast.

Cast guideline now removed.

[yebblies]

LGTM

[PetarKirov]

LGTM too.

BTW, should the same rule be applied assert? E.g.:

assert (...);

[CyberShadow]

BTW, should the same rule be applied assert? E.g.:

Is your example how to or how not to write assert?

Current Phobos use is overwhelmingly without a space. (Nor do I see why adding a space makes sense unless you do so for all function calls, assert generally works just like a function with a bit of magic).

[PetarKirov]

Note: I don't want to start a big debate on this, I just wanted to know what the opinions are on this topic.

I prefer the assert (...) syntax, because it makes clear to the reader that it is special. E.g. I use assert (0) in functions with more compilcated control flow to mark unreachable code. And it also looks more consistent with the rest of the langauge.

Never mind then, I think we should merge this PR, and leave this discussion for some other time.

[yebblies]

BTW, should the same rule be applied assert?

No, we're just adding rules that reflect the current style and the current style has no space after assert.

makes clear to the reader that it is special

I find that basic syntax highlighting does that job fairly well.

[yebblies]

Can someone merge this? I don't have merge rights to this repo or I would.

[CyberShadow]

Was just going to actually :)