sdfmt: Template constraint formatting issue

[maxhaton]

bool shouldApproxEqual(L, R)(auto ref L lhs, auto ref R rhs)
		if (__traits(isFloating, lhs) && __traits(isFloating, rhs)) {
	return true;
}

is currently a fixed-point for sdfmt. This is ugly.

Note that the DStyle dictates a new line per brace.

[deadalnix]

I never understood why the DStyle state one line per brace. this is awful in general, but this is especially awful in D. Consider contract for instance, mostly because of that guideline, a new syntax was introduced, but even more damning, there was a proposal by Andrei no less to change the recommendation for in condition to be written within the function's body with assert because the existing style was too awful.

When a guideline is the source of semantic breaking change followed by a language syntax change, it's a strong tell that the guideline is not good and should be abandoned.

As for uglyness, I don't find this sample especially ugly. What do you think make that formatting bad, objectively? All the alternative I've seen used in D code in the wild have objective downsides:

• Putting the braces on new lines lead to a crazy saga with in preconditions.
• Putting the constraint's if at the top level causes extremely different code to look alike:

void foo(T)(T t)
if (condition) {
   // do stuff...
}

vs

void foo(T)(T t);
if (condition) {
   // do stuff...
}

Putting the if at one indentation level makes it look like it is part of the function's body, so I went for 2 levels, kind of as a default. I was weirded out by it at first, but objectively, it is better than all the alternatives I've seen. Do you have a better idea?

[maxhaton]

Personally, I like having braces on separate lines for function decls at least. I couldn't care less about statements though.

Having the constraint to be intended so that it looks distinct from the function is a good thing (although I don't really care as long as a codebase has a convention) but I think the current formatting just looks really weird.

bool shouldApproxEqual(L, R)(auto ref L lhs, auto ref R rhs)
		if (__traits(isFloating, lhs) && __traits(isFloating, rhs)) //This is double-indented, looks different from body.
{ //This now doesn't make me go wtf
	return true;
}

Having the brace on the same line as a the constraint is a big https://en.wikipedia.org/wiki/Principle_of_least_astonishment violation, and I'm saying that as someone who has made a point of never quoting such laws and principles before.

[deadalnix]

Having the brace on the same line is an exceedingly common pattern, it is called Egyptian braces and is present in numerous coding conventions.

[JohanEngelen]

I don't think the discussion should be about ugliness, which is subjective after all.

How about making it optional? An option like "put brace on newline when function signature and constraints span multiple lines" ? If true, you'd get:

int foo() {
    return 1;
}

int foo()
        if ( .... )
{
    return 1;
}

int foo(int asdkahdkjashdjkahsdkjads,
            int aksjdasdajksda, int asdasd)
{
    return 1;
}

[deadalnix]

This discussion is not about ugliness. Several points were raised as to why this is an inferior style for D code. These points were substantiated by actual problems in the wild causes by the "official" D style, not even abstract supposed ones.

There are a couple of problems with the road you suggest this goes down to:

1. It will make things untestable, as each option introduced to the formatter creates a combinatoric explosion of special cases. The main formatter that went down this road certainly is clang-format, but if you try clang-format with combinations of options that fall out of the suggested style guides, you'll very quickly realize that it start generating garbage. This is unavoidable.
2. It increase the burden to support the software. It's somewhat of a byproduct of 1., but nevertheless worth pointing out. I would advice that devs generally try to leverage their work as much as possible, in order to have as much impact as possible, and this is the opposite of that. That seems like a bad value proposition overall.
3. The goal of code formatter is generally not to make the code pretty, but
   1. Stop debate over style. They are generally a huge time sink for everybody involved, plus, the people who actually get shit done, who's opinion would be the most valuable, are usually the people too busy to participate, leading to inferior style. More options generally works against that goal. See (By far the biggest reason for adopting Prettier is to stop all the on-going debates over styles.)[https://prettier.io/docs/en/why-prettier.html].
   2. Make code uniform. People can expect code that is the similar to look similar. Even better if that happens across an ecosystem. More options work against that goal. "Gofmt's style is no one's favorite, yet gofmt is everyone's favorite."
   3. Try to guide users toward good programming patterns. For instance, the linux kernel enforce 8 character indentation. If you were to debate this with Linus, he'd tell you that if you have more than 3 indentation levels, you are screwed anyways. Instead of fighting this at every turn during code review, it is much easier to guide people toward this realization by making code that do not confirm atrocious looking. It works. Guess what more option does on that front? Correct, they work against that goal.

There are a couple of exception to these principles, (Prettier has a good run down here)[https://prettier.io/docs/en/why-prettier.html].

But I digress, because this failed to address the first thing required for this to take off: demonstrating that it solves an actual problem. The closest thing we have to a problem statement is that it doesn't follow the official D style guide. But following said style is not a goal of the project to begin with, and in addition, deviating from said goal solves actual problems that can be pointed at. So it's a slam dunk, and will remain a slam dunk until new evidence that it solves an actual problem is brought to the table.

[JohanEngelen]

Hi @deadalnix , it's a pity you feel this way.

1. The testability is a real concern, however in practice I have never run into any issues with clang-format.
2. Maybe. But if people start to use sdfmt, do expect to have these kind of discussions on a regular basis, also eating at your time.
3. Ofcourse the formatter is to stop debate over style, within a project. I disagree that it should strive to design one style for the whole world. Fortunately for sdfmt, Linus is not the standard, because max 3 indentation levels is already violated in the very first file I looked at :-) With the line of reasoning you wrote, you are saying that sdfmt's current output can never be changed. Any change (let's say a "bug fix") that changes the output, will trigger point 3.i (debate, is the bug fix an improvement or not) and point 3.ii (code of one project using older sdfmt will look different from newer sdfmt).

The actual problem is: I cannot read the current formatting well, neither can Max. It's unfortunate and means I cannot support use of sdfmt at Weka.

[JohanEngelen]

It's unfortunate and means I cannot support use of sdfmt at Weka.

Apologies for the hyperbole. (I think with a few fixes it'll be fine :-))

[deadalnix]

If this is a blocker for Weka, then it's worth thinking about, but so far, there doesn't seems to be a good solution. If you try to implement things such as "put brace on newline when function signature and constraints span multiple lines" as you suggest, you'll quickly realize that it's not as simple as you think it is and it's very easy to run into situations where the formatter start making very strange decisions because of rules interacting ins strange ways.

It'd be interesting to have @maxhaton 's feedback after using that style for a while at symmetry. The general pattern seems to be an initial "WTF is this" (which I definitively went through myself, this was FAR from my initial choice) but then people get used to it and find it to work for them. It's worth thinking about.