Skip to content

posix.mak: Enforce whitespace before opening version parenthesis#6160

Closed
ibuclaw wants to merge 1 commit intodlang:masterfrom
ibuclaw:lintversionparens
Closed

posix.mak: Enforce whitespace before opening version parenthesis#6160
ibuclaw wants to merge 1 commit intodlang:masterfrom
ibuclaw:lintversionparens

Conversation

@ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Feb 11, 2018

Get everyone using the same style.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@n8sh
Copy link
Member

n8sh commented Feb 11, 2018

If this is being added to posix.max there should also be a line about it in https://github.com/dlang/dlang.org/blob/master/dstyle.dd. I recently checked it for guidance on version but couldn't find any.

alias SQLWCHAR = ushort;

version( UNICODE )
version ( UNICODE )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use this opportunity to normalize these too?

@wilzbach
Copy link
Contributor

I'm all in favor of being consistent in the style, but may I ask why you picked this one?

> ~/dlang/phobos on master ± ag "version\(" | wc -l
714

> ~/dlang/phobos on master ± ag "version \(" | wc -l
545

@ibuclaw
Copy link
Member Author

ibuclaw commented Feb 11, 2018

I'm all in favor of being consistent in the style, but may I ask why you picked this one?

You mean version style? Because it is a heavily used feature that has at least three different styles, frequently intermixed in the same condition statement!

@wilzbach
Copy link
Contributor

You mean version style?

No I meant why version( and not version ) - see the output of the grep command.

@ibuclaw
Copy link
Member Author

ibuclaw commented Feb 11, 2018

Ah, that would mean having its own regexp to catch that particular style.

I could use the same argument against the following also:

if ( cond )
switch ( cond )
while ( cond )
foreach ( var; array )

But to be really precise about this, we may need something smarter in order to lint, as you'd also want to catch e.g:

if ( cond1 ||
     cond2 )

@wilzbach
Copy link
Contributor

Yeah, you want this to be a D-Scanner check, e.g.

dlang-community/D-Scanner#450
dlang-community/D-Scanner#348

However, I never managed to push these into D-Scanner (there were some controversies on how D-Scanner can access the tokens), but we now have a special branch phobos there into which we can push anything (even things Brian objects to).

@n8sh n8sh closed this Jun 24, 2019
@n8sh
Copy link
Member

n8sh commented Jun 24, 2019

(superseded by #6717)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants