Prepare for long line checks in travis

[JackStouffer]

This should be one of the easier ones to enable and then fix, so let's see how this breaks.

[JackStouffer]

There's no reason to not get rid of lines longer than 120 characters, even if they are inside of unittest blocks.

And I'm of the opinion that automated style checking is a very good idea.

[JackStouffer]

I just say that Dscaning now is premature and not well implemented

This is just a natural part of dogfooding. Eventually, we want dscanner and dfmt to become official tools, but if even the D contributors aren't using/testing it, then why should anyone else.

[JackStouffer]

Your PR fails because what are the constraints of today were not applied yesterday.

Yes, and I expected it to break, and in many places. That's the whole point of this: to fix these style deviations and make it so they cannot happen again.

[wilzbach]

And I'm of the opinion that automated style checking is a very good idea.

Yep I totally agree with @JackStouffer. Thanks a lot for pushing this ;-)

Eventually, we want dscanner and dfmt to become official tools, but if even the D contributors aren't using/testing it, then why should anyone else.

Well you say "that Dscaning now is premature and not well implemented.", but how is it going to be mature if no one uses or tests it?
Yes we had a couple of issues with Dscanner once we integrated it to Phobos, but those should be fixed now and thus it is easier for other people to use dscanner in their projects.

That's the whole point of this: to fix these style deviations and make it so they cannot happen again.

That's exactly why Andrei liked the idea btw!

[wilzbach]

The Travis-CI based dscanner check will never help anyone to make a better pull request. never ever.
It's a complete extravaganza.

Basically it's an automated enforcement of the D Style:

We are not necessarily recommending that all code follow these rules. They're likely to be controversial in any discussion on coding standards. However, they are required in submissions to Phobos and other official D source code.

[JackStouffer]

Wilzback, to use dscanner here is not a good idea. I've banned you because of this. I've also banned Alexandrescu because he trusted you. To use Dscanner right now to check pull requests is totally useless and is a waste of resource. it's completly stupid. Dumb.

Please back up assertions with evidence.

As we have already mentioned, dscanner has bugs, but that's why we're dogfooding it. That's not a reason to throw out the idea wholesale.

[wilzbach]

To use Dscanner right now to check pull requests is totally useless and is a waste of resource. it's completly stupid. Dumb.
Please back up assertions with evidence.

I think it's a waste of resources to have this discussion.
Thanks a lot @JackStouffer for investing the time & fixing this simple constraint!

[CyberShadow]

👍

Don't mind @bbasile, I think he's just a bit too liquored up at the moment :)

Making the error messages nicer would be a point of improvement going forward.

BTW, @JackStouffer, it seems your git email address is not associated with your GitHub account.

[JackStouffer]

@wilzbach @CyberShadow That's everything.

[CyberShadow]

Hmm, why is the doc build failing? Is it this?

string expected, not '"It will be removed from Phobos in October 2016. " ~ "If you still need it, go to https://github.com/DigitalMars/undeaD"'

[CyberShadow]

The internal Unicode tables look like they might be machine-generated, thus they may break again if regenerated. @DmitryOlshansky ?

[JackStouffer]

@CyberShadow hmm, deprecation messages are now supposed to work with enum messages, so if I change it to this:

enum deprecated_message = "It will be removed from Phobos in October 2016. "
    ~"If you still need it, go to https://github.com/DigitalMars/undeaD";
deprecated(deprecated_message) module std.stream;

I get

std/stream.d(30): Error: declaration expected, not 'module'

[JackStouffer]

@CyberShadow for the unicode tables, I just ran them though dfmt. So if they need to be regenerated later, then dfmt can just be rerun.

[CyberShadow]

@CyberShadow hmm, deprecation messages are now supposed to work with enum messages, so if I change it to this:

Might be a use case for the implicit string concatenation thing (unless that's enabled in Travis already?)

[Hackerpilot]

Putting the enum above a deprecated module won't work because the grammar doesn't allow anything before the module declaration.

I'm pretty sure that concatenating strings in deprecated messages is allowed as of dmd 2.071.

[JackStouffer]

I'm pretty sure that concatenating strings in deprecated messages is allowed as of dmd 2.071.

I thought so too, but it seems to not work.

[Hackerpilot]

It's right there in the changelog: http://dlang.org/changelog/2.071.0.html#extended-deprecated

[JackStouffer]

It's right there in the changelog: http://dlang.org/changelog/2.071.0.html#extended-deprecated

I don't know what to tell you man. It doesn't work.

[JackStouffer]

https://issues.dlang.org/show_bug.cgi?id=16014

[CyberShadow]

Omit the ~ in the mean time?

[JackStouffer]

Can't, the line is too long.

[CyberShadow]

I mean this stupid trick (same as in C++):

writeln("abc" "def");

It should be deprecated and removed, but IIRC it should still work for the moment.

[JackStouffer]

@CyberShadow fixed.

[JackStouffer]

but IIRC it should still work for the moment

Welp, I guess travis answered that question.

[CyberShadow]

D'oh.

(unless that's enabled in Travis already?)

[CyberShadow]

Auto-merge toggled on

[wilzbach]

@JackStouffer what stopped you from enabling the long line check?
Since May a couple of new violations were checked in.

std/format.d(5191:12)[warn]: Line is longer than 120 characters
std/stream.d(26:12)[warn]: Line is longer than 120 characters
std/socketstream.d(37:12)[warn]: Line is longer than 120 characters
std/socket.d(1070:116)[warn]: Line is longer than 120 characters
std/experimental/ndslice/selection.d(1436:118)[warn]: Line is longer than 120 characters
std/experimental/ndslice/selection.d(1529:106)[warn]: Line is longer than 120 characters
std/math.d(7787:119)[warn]: Line is longer than 120 characters

I don't know what to tell you man. It doesn't work.

Be creative. It's just one occurrence in socketstream that will be removed anyhows, e.g. just removing "from Phobos" in the message already gets you below 120 ;-)

[JackStouffer]

what stopped you from enabling the long line check?

The unwillingness of the compiler maintainers to admit that 16014 is a bug.

[wilzbach]

The unwillingness of the compiler maintainers to admit that 16014 is a bug.

Yeah I know, but as said it's easy to workaround and there's only one occurrence - the following just uses 115 chars ;-)

deprecated("It will be removed in October 2016. If you still need it, go to https://github.com/DigitalMars/undeaD")
module std.socketstream;

[wilzbach]

Btw in the bug another workaround was suggested:

pragma(msg, __FILE__,"(",__LINE__,"): this module is deprecated");

Imho just pick one workaround and enable the check before more violations are merged in, socketstream will be removed anyways in two months :)