prettier / prettier

Prettier is an opinionated code formatter.
https://prettier.io
MIT License
49.45k stars 4.37k forks source link

Formatting of generators changed without sufficient consensus #7512

Closed glen-84 closed 4 years ago

glen-84 commented 4 years ago

I noted in #3903 that the formatting of generators was recently changed based on an issue (#7028) that had a total of just 2 up-votes (excluding the OP's).

I was under the impression that such changes would require far more consensus and motivation. There are issues with many more votes that have not yet even been considered.

I'm opening this issue to suggest that the relevant changes be reverted until a proper process has been followed.

FWIW, the original arguments didn't seem that compelling to me, but ultimately this is a very subjective matter and there isn't really a right or wrong answer. However, the change should only be made if there are either objective reasons to do so, or a really high percentage of votes in favour of the change.

I'd like to avoid discussing the pros and cons in this issue, as that can be done elsewhere.

alexander-akait commented 4 years ago

Is this a major change for you? There were no heated discussions on this issue because it is just space. What are the disadvantages of the current formatting? I don’t think that every change should create a holy war. In addition, all contributors have chosen that formatting. You can find details and explanation why we made this choice.

I was under the impression that such changes would require far more consensus and motivation. There are issues with many more votes that have not yet even been considered.

We don’t have any regulations when we decide what we do or not, the main reason for the change is the improvement in readability, in this problem we decided to choose a consistent output.

If after each merge the developers open up new issues without explanations of problems (and there are already a lot of them) or bugs with the idea of discussion, we will never go far.

glen-84 commented 4 years ago

Is this a major change for you?

Honestly, it's not a big change for me personally. I just feel that the wider community should be given the opportunity to provide their input before such changes are made.

because it is just space

3847 was also just a space, but it was open since 31 January 2018 and received 487 up-votes (!).

What are the disadvantages of the current formatting?

What were the disadvantages of the previous formatting? The items mentioned by Brian were:

  1. Consistency.

    No one likes consistency more than I do, but it seems unlikely that someone would be mixing shorthand syntax with longhand syntax within the same object. If you do, I'd be interested in seeing the code.

  2. Catching bugs.

    I didn't quite understand how this catches bugs. I don't think that I'd miss the * after the function keyword when reading code, but I can't speak for others.

I don’t think that every change should create a holy war.

I'm not asking for a holy war. If the majority of users prefer the new formatting, then we'll just live with it.

In addition, all contributors have chosen that formatting. You can find details and explanation why we made this choice.

Where can I find this? I just saw about 3 contributors giving a thumbs up. Are there only 3 contributors? The reason for accepting the proposal seems to simply be "I guess we just need to make sure there is no good reason for rejecting this proposal", a couple of days before making the change.

the main reason for the change is the improvement in readability

Which is subjective. I personally find 4-space indentation and double quotes to be much more readable, but that's not enough to change the defaults.

If after each merge the developers open up new issues without explanations of problems

I explained the problem, the problem is that the change was made on relatively short notice with no community input. However, if that's just the way that things are done here, then let it be.

FWIW:

alexander-akait commented 4 years ago

Out of the top 9 Google results (1st page) for "js generators", 6 use left-aligned, 2 use centre-aligned, and 1 is mixed. None of the articles use right-aligned.

I think because many developers use them extremely rarely and do not pay attention to the style

In addition to MDN (the most popular resource for web development), the most popular JavaScript style guide (Airbnb) also recommends the left alignment.

I would recommend MDN to use prettier. Also they used single quotes https://developer.mozilla.org/en-US/docs/Learn/JavaScript/First_steps/Arrays and other strange things, sometimes I saw very poorly readable code there

Airbnb - contains a lot of strange things, it's just one of many style guides

"Why? function and are part of the same conceptual keyword - is not a modifier for function, function* is a unique construct, different from function."

"... usually the first syntax is preferred, as the star * denotes that it’s a generator function, it describes the kind, not the name, so it should stick with the function keyword."

Eslint has other opinion https://eslint.org/docs/rules/generator-star-spacing

The default is {"before": true, "after": false}.

We can read other people's guides for a long time, but it's useless

alexander-akait commented 4 years ago

I think we can leave this problem issue for discussion (but I do not think that we will change our decision in the near future, perhaps never), but I will ask other developers not to write “give an option”, “please change the behavior”, “I hate prettier”, etc.

bakkot commented 4 years ago

I am also very surprised by this. It was previously discussed and everyone seemed to be in agreement that it would be function* gen() {} and function* () {}.

brainkim commented 4 years ago

No one likes consistency more than I do, but it seems unlikely that someone would be mixing shorthand syntax with longhand syntax within the same object. If you do, I'd be interested in seeing the code.

I don’t think consistency for a code formatter is based on whether the code appears within the same expression or statement. You don’t think it’s inconsistent that you put no space before the star when using the function keyword and no space after the star when using methods/shorthand? There’s no before for generator stars in the case of methods, and you literally can’t use a yield star without an operand, so it should be formatted like literally every other prefix operator in JavaScript.

I didn't quite understand how this catches bugs. I don't think that I'd miss the * after the function keyword when reading code, but I can't speak for others.

Accidentally exploding a string iterable using yield* is a rite of passage for anyone who uses generator functions. You don’t think you’d miss it, but most syntax highlighters color function and yield the same as the generator star, see github’s highlighting for instance:

yield* this.isABusyCallExpression(a + b);
yield *this.isABusyCallExpression(a + b);

Which is more obvious? I’m sure you read yield stars just fine, but code formatting decisions can often be based on small details like this.

I haven’t seen a single argument in favor of the former styling. The consistent refrain from the top-page of google results is that function* and yield* are keywords, but that’s 100% not true; keywords are special words which the JavaScript parser treats differently, as in actual English words, not words + punctuation between which there can be an arbitrary number of spaces.

"... usually the first syntax is preferred, as the star * denotes that it’s a generator function, it describes the kind, not the name, so it should stick with the function keyword."

I don’t understand how the fact that the star describes the “kind“ of function motivates that it should stick to the function keyword and not the name of the function. It is really important to distinguish between generator functions and regular functions to the reader; in other words, the “kind” of the function is just as important as the name because generator functions execute lazily. I regularly see people treat generator functions like side-effect producing functions, in the hopes that calling them executes side-effects. They are then surprised when the function bodies never run. Again, this formatting decision is about the little things, which is more obviously a generator?

function* myLongFunctionName(withLots, ofArguments = {}) {
}
function *myLongFunctionName(withLots, ofArguments = {}) {
}
thorn0 commented 4 years ago

Does this output really look okay to somebody here?

Prettier 1.19.1 Playground link

--parser typescript

Input:

function *foo<T>() {}
class Bar {
  static *foo<T>() {
    yield *foo<T>();
  }
}
baz = {
  async *foo<T>() {}
};
f = function *<T>() {};

Output:

function* foo<T>() {}
class Bar {
  static *foo<T>() {
    yield* foo<T>();
  }
}
baz = {
  async *foo<T>() {}
};
f = function*<T>() {};
brainkim commented 4 years ago

@thorn0 That could probably be an example used in the changelog entry that’s a great way to show how it’s more consistent.

thorn0 commented 4 years ago

The situation with yield * is more complicated though. Its precedence is tricky:

c = yield *a + b;

actually means

c = yield *(a + b);

and not

c = (yield *a) + b;

but the star placed closer to the expression seems to make the misinterpretation more probable.

Apparently Prettier should always add parens in such cases, when a binary or ternary expression gets trapped in a yield, regardless of whether the yield has a star. This should prevent bugs caused by this precedence, which seems very counter-intuitive now that everyone is used to await, whose precedence is completely different.

brainkim commented 4 years ago

I didn’t think about the way operator precedence would work if we treated yield stars like prefix operators, insofar as yield star precedence is so low, but I also don’t think it’s a big deal, insofar as using yield star implies that the operand is an iterable, and I have never seen a binary expression used as the operand for a yield star.

I am perplexed by the different precedences of await and yield. Feels like a mistake to me. But I also dislike whenever prettier adds or removes parens from my code so there’s that.

thorn0 commented 4 years ago

I have never seen a binary expression used as the operand for a yield star

For this reason, adding the parens seems to be the right thing to do. If Prettier adds the parens and transforms yield *a + byield *(a + b), the mistake won't go unnoticed. Also we might want to skip adding the parens in cases where yield is used as a statement and feels more like return than await.

I am perplexed by the different precedences of await and yield. Feels like a mistake to me.

Totally agree. Also I wish we had yield ... instead of yield *.

glen-84 commented 4 years ago

You don’t think it’s inconsistent that you put no space before the star when using the function keyword and no space after the star when using methods/shorthand?

You can put a space after the asterisk for methods ...

function* example {
    // ...
}

class Example {
    foo() {
        // ...
    }

    * example() {
        // ...
    }

    * [Symbol.iterator]() {
        // ...
    }
}

It would take the place of the function keyword. But it doesn't bother me too much either way.

... it should be formatted like literally every other prefix operator in JavaScript

Except it's not a prefix operator? It works together with function (which might be implied in the case of methods or shorthand syntax), or yield.

*this.isABusyCallExpression(a + b); is not going to do anything by itself.

It's commonly referred to as one thing:

Do we refer to it the same way after this change, or as something like "function *{funcName} declarations"?

Accidentally exploding a string iterable using yield* is a rite of passage for anyone who uses generator functions.

Doesn't seem like a big issue to me.

most syntax highlighters color function and yield the same as the generator star

Reference? VSCode (multiple themes): no, PhpStorm (default theme): no. GitLab: no.

And I think most developers can notice the asterisk even when it's the same colour. If we are truly concerned about the accessibility, then we should also switch to 4-space indentation and double-quotes (I actually use both).

I haven’t seen a single argument in favor of the former styling.

And I haven't seen a strong argument for changing the current formatting.

The consistent refrain from the top-page of google results is that function and yield are keywords

They are keywords followed by an asterisk ...

"The function* declaration (function keyword followed by an asterisk) defines a generator function"

I don’t understand how the fact that the star describes the “kind“ of function motivates that it should stick to the function keyword and not the name of the function.

Because it's a "generator function", and has nothing to do with the name.

Does this output really look okay to somebody here?

It could just as easily be left-aligned ...

function* foo<T>() {}
class Bar {
  static* foo<T>() {
    yield* foo<T>();
  }
}
baz = {
  async* foo<T>() {}
};
f = function*<T> () {};

(although GH and VSC seem to choke on async*)


For what little it's worth, I only noticed one code example in the spec (EnumerateObjectProperties), and it is left-aligned. 🤷‍♂

Anyway, I think that we've dumped enough information here for others to make up their own minds, although I guess that that decision has already been made for them.

bakkot commented 4 years ago

Created https://github.com/prettier/prettier/pull/7516 to revert.

This is just a style preference, but it looks to me like there was pretty broad agreement (per previous discussions here and here) prettier's previous style was correct. When there is such broad agreement I don't think it makes sense to do something different - in fact, as far as I am aware, this would be the only case where there is broad agreement on a style and prettier chose to do something different.

phaux commented 4 years ago

How about making the star left-aligned in yield, but right-aligned in function declarations? It would be still much more consistent than the current behavior.

thorn0 commented 4 years ago

@bakkot IMHO people in that issue didn't care about generators at all and would have +1'd anything just to get that whitespace after function as quickly as possible. Calling those reactions 'broad agreement' is a stretch.

Generators are still a relatively little-used feature of the language, so I don't think we can really speak of any 'broad agreement' or 'widespread formatting' with regard to them. This gives Prettier freedom to consider other factors: consistency, expressiveness, readability, etc. #7028 had been open since November and nobody had any substantial objections against its reasoning.

But you linked twice to the same comment in that issue. Did you want to link to something else?

@glen-84

Do we refer to it the same way after this change, or as something like

What's wrong with "generator declaration"? "The function* keyword" makes as much sense as "the async function keyword". Anyway, people can call things whatever they like. Prettier and formatting in general have nothing to do with that.

It works together with function (which might be implied

Implied? I don't think so. Just because function is an older syntax, that doesn't mean all function-like constructions imply the function keyword. Isn't it more logical to say that * works together with the signature? The signature is always there, no need to imply anything.

Except it's not a prefix operator?

* after yield does a thing very similar to ..., which is not an operator too, strictly speaking, but is often called 'spread operator' and is formatted as such, without whitespace between it and the subsequent expression:

yield *[firstValue, ...getOtherValues(...args)];

Can you see what I mean? Isn't it expressive when similar things are formatted in a similar way? The formatting helps communicate the fact that the same idea of spreading is in play here.

You can put a space after the asterisk for methods

One-character modifiers like * don't look good on their own. What idea would the space between it and the name communicate? "There is an implied function keyword in front of me (but it can't actually be here)"? Also that space would seem especially strange in private class methods:

class Foo {
  * #bar() { ... }
}
brainkim commented 4 years ago

yield ... would have made so much more sense. I’m mourning that realization today because all these language decisions are so irreversible.

thorn0 commented 4 years ago

@brainkim I think it still can be added as an alternative syntax. Grammarwise, nothing seems to block this possibility. Who wants to write a proposal to TC39? :)

bakkot commented 4 years ago

IMHO people in that issue didn't care about generators at all and would have +1'd anything just to get that whitespace after function as quickly as possible. Calling those reactions 'broad agreement' is a stretch.

I really don't think that's true; if people didn't agree it would be weird to vote. (And a dozen people also upvoted the next comment, which gives an explicit reason to prefer function*.)

But if you like we can look elsewhere: function* is the style used by the spec itself, MDN, rauschma's tutorial, Eloquent JS, pretty much every other tutorial I could find (etc, etc, etc, etc), babel, wu.js, gatsby, co, node, deno, v8, spidermonkey, angular, bluebird, typescript, GraphQL, msgpack-javascript, etc.

I did find a couple of exceptions: flow-typed and YDKJS both use the function * style. But not many others do.

But you linked twice to the same comment in that issue. Did you want to link to something else?

Oops. I meant to link to this earlier discussion. Fixed.

thorn0 commented 4 years ago

I really don't think that's true; if people didn't agree it would be weird to vote.

We can't know for sure, but that's how it often goes in those popular issues. People go through comments upvoting everything that appears to be in support of the issue's proposal (space vs no space in this case).

FWIW that comment also says 'Since this is all about consistency with named functions, the spaces should be exactly like for a named generator function'. What if the upvotes were addressed to this statement, with which the behavior in next is in line? What if people simply voted for consistency?

thorn0 commented 4 years ago

BTW, another argument against

function and * are part of the same conceptual keyword - * is not a modifier for function, function* is a unique construct, different from function

If they're different constructs, why make them look similar by not putting a space in front of *? Instead, we should emphasize their difference to avoid confusion. Everything is more noticeable when it's separated, that's what separators are for.

bakkot commented 4 years ago

People go through comments upvoting everything that appears to be in support of the issue's proposal (space vs no space in this case).

But that comment wasn't in support of the issue's proposal. It was in support of a particular answer to an unrelated question.

In any case, again, a dozen people also upvoted the next comment (and zero people downvoted it), which specifically gave a reason to prefer function* over function * (a reason other maintainers have also spontaneously generated in the past), and that is still three times as many people as upvoted https://github.com/prettier/prettier/issues/7028.

Anyway, this sort of exegesis is not really that helpful. I think that 40 people upvoting a comment saying the format should be function*, and a dozen people upvoting the immediately following comment which agrees gives an explicit reason to do so, is pretty clear support of function*. You, apparently, don't. I don't think we're going to convince each other; going back and forth here isn't all that useful.

Which is why I also went through pretty much every major project or tutorial I could find which makes use of generators. That also showed what looks to me like very broad agreement. Does this list not look like broad agreement to you?

thorn0 commented 4 years ago

But that comment wasn't in support of the issue's proposal.

It was a pro-space comment. It included that much desired space in the code snippet.

The point I’m trying to make is that if a feature is relatively little-used, if the absolute number of its users is much smaller compared to other language features, then any agreement about it automatically isn't broad and a change in its formatting isn't a massive change.

thorn0 commented 4 years ago

The utter lack of interest towards #7028, compared to #3847 for example, clearly shows that people don't feel strongly about how to format decorators. As I wrote earlier, this gives Prettier some freedom. What's wrong with using this freedom if it exists?

brainkim commented 4 years ago

@bakkot I mean, thanks for doing a survey of the literature on generators, but my response when I wrote #7028 and now is the same, I use generators all the time, and I noticed two concrete reasons why the former formatting is less optimal. I bet you if you went back and asked each author if they agree with my issue they’d either 1. not care or 2. agree with me.

Most of these articles repeat the dubious claim that function* or yield* are keywords, implying that the authors did not know you could add a space between the keyword and the star in the first place. Again, I feel like I’m repeating myself, they’re not keywords, no keyword permits spaces between its members or has members to begin with. The closest analogue for what generator stars are in JS is spread/rest syntax, which can only appear in specific places like function parameter lists or object/literal arrays, but is treated like a prefix operator for formatting.

@bakkot Besides your comprehensive review of the literature can you respond to #7028 based on the arguments it does make? I just don’t understand how you and @glen-84 can both argue that this is an unimportant formatting decision, and also that the consistency/bug-catching reasons are also unimportant. In programming, when faced with arbitrary decisions, small concerns like the two which I brought up are god-sends because they help us choose. I’m glad that @glen-84 has eagle eyes that can spot a misplaced asterisk anywhere in their code, but if you can’t concede that the new formatting could help us mere mortals catch bugs then I don’t know how to respond. Also, if you somehow end up arguing that true consistency in code would be to write classes like this:

class Foo {
  * bar() {
  }

  async* baz() {
  }

  * #quux() {
  }
}

despite the fact that this would also be a “breaking” change to prettier, I dunno. Again I’m not sure how this conversation moves forward.

@thorn0

The utter lack of interest towards #7028

Wow I thought my prose was compelling but guess not 😂.

thorn0 commented 4 years ago

Wow I thought my prose was compelling but guess not 😂.

I meant the number of reactions. :) But I guess we need more prose. Something like a "Generator formatting mini-FAQ" for the 2.0 blog post.

thorn0 commented 4 years ago

this would also be a “breaking” change to prettier

I'd like to remind that Prettier doesn't consider formatting changes 'breaking'. Some of them are considered 'massive' though. I believe none of the changes we discuss in this issue can be qualified as 'massive'.

bakkot commented 4 years ago

@thorn0

The utter lack of interest towards #7028 clearly shows that people don't feel strongly about how to format decorators.

Very few people hang out on prettier's issue tracker. What happens is that people show up when prettier is doing something they don't like, and look for (or file) an issue asking for it to be changed. #3847 had a lot of activity because a lot of people disliked prettier's then-current behavior on that topic. All we can conclude from the lack of activity on #7028 is that there are not a lot of people who dislike prettier's then-current behavior on this one.

We can't say whether that's because they don't care or because they agree with it. We could, of course, ship the change to master and then wait for people to show up asking for it to be changed, but that seems... bad. Usually we try to figure out if there's consensus on one style or the other by looking at the ecosystem, instead of just shipping it and waiting for people to get mad. And it seems to me that there is rough consensus in the ecosystem.

That said, the comment in #3847 gives us something of an opportunity to distinguish between "people don't care" and "people agreed with what was already happening": people were already in that issue because they had a different thing they disliked, so the comment asking about function* reached a larger than usual subset of users not preselected for disliking prettier's generator formatting. And it turned out that a lot of those users took the time to upvote a comment - sometimes two comments - which were in favor of the function* style. (And zero people downvoted or disagreed!)

As I wrote earlier, this gives Prettier some freedom. What's wrong with using this freedom if it exists?

In cases like this, I don't think prettier ought to use the fact that a feature is relatively rarely used to do something which is different from what most people who are using it are doing.

@brainkim:

I bet you if you went back and asked each author if they agree with my issue they’d either 1. not care or 2. agree with me.

Since they all apparently independently thought that function* was the way to go, I think the burden of this claim ought to be on you.

Also, though, I can't speak for all of those projects, but at least for the JavaScript specification itself: I help maintain the specification - I recently rewrote the section in which the generator in question appears, in fact - and I care, and I don't agree with you.

Most of these articles repeat the dubious claim that function* or yield* are keywords, implying that the authors did not know you could add a space between the keyword and the star in the first place. Again, I feel like I’m repeating myself, they’re not keywords, no keyword permits spaces between its members or has members to begin with.

I am familiar with the rules for parsing JavaScript. I suspect rauschma and marijnh and vjeux are as well. The claim is not that function* is literally a keyword; it's that it is conceptually a keyword. It changes the kind of function you are declaring (or in the case of yield*, it changes behavior from yield-once to yield-each). It's not changing how the function is invoked: function *f() {} is still called as f(), not *f(). So it should be treated as a keyword.

I just don’t understand how you and @glen-84 can both argue that this is an unimportant formatting decision

I didn't say it was an unimportant formatting decision.

Besides your comprehensive review of the literature can you respond to #7028 based on the arguments it does make?

Sure:

* does not behave like a prefix operator. yield* is a different kind of thing that yield; it's not the the value being provided to yield is transformed (if it were, it would only yield once). Also, * is a prefix operator in some other languages, like C++ and Rust, where it has nothing to do with the behavior here. So I don't think it should be treated like a prefix operator.

And I find yield* x to be much more obviously different from yield x than yield *x would be (especially having written a decent amount of C++). So I don't think it will catch more bugs - just the opposite, in fact.

I agree with the consistency argument. I just don't find it compelling in light of all of the above. A foolish consistency, and all that. As far as I can tell, pretty much everyone who uses generators decides to write function* f() {} and also to write ({ *f(){} }). These are inconsistent, but people do it anyway, so whatever.

thorn0 commented 4 years ago

All we can conclude from the lack of activity on #7028 is that there are not a lot of people who dislike prettier's then-current behavior on this one.

I personally don't like Standard JS for its misleading naming and don't know how much popular it is, but it turns out it prescribes to format generators this way:

function * lonelyGenerator () {
  yield * increment()
}

One of the big complaints in #3847 was that Prettier's behavior was incompatible with Standard. How come all this time nobody from the Standard users complained about formatting of generators?

brainkim commented 4 years ago

@bakkot

The claim is not that function* is literally a keyword; it's that it is conceptually a keyword. It changes the kind of function you are declaring...

Your conception of what a “keyword” is, is incoherent. Just because the async keyword makes functions return a promise doesn’t make async function a keyword, the keywords are still async and function, and the function keyword can be used independently. So too with generator stars, except that because it is punctuation, the whitespace rules with respect to parsing are relaxed.

It's not changing how the function is invoked

Isn’t the fact that function and function * both create invokables an argument that the generator star is a modifier, not a keyword?

The whole concept of “keywords” exists because JavaScript treats certain English words as special, so that you can’t use them as identifiers. To muddy that distinction by saying function* or yield* is a keyword is unhelpful, no more helpful than saying for( or import( is a keyword. The pedagogical value of the concept of keywords is to draw attention to the fact that not every word can be a variable identifier.

* does not behave like a prefix operator.

The * in yield * behaves EXACTLY like a prefix operator. Check out the following code:

function *foo() {
  yield; // not a syntax error
  yield *; // a syntax error
}

The fact that you can yield without an operand but cannot yield * without an operand is the clearest evidence that we should treat the * as though it were a prefix operator. In addition, I imagine that if the syntax was yield ... you would find your position that it is not prefix-like untenable.

I find yield* x to be much more obviously different from yield x than yield *x

Your code examples are trivial enough where it does not matter, but when the yielded expression is complicated it’s very easy to miss the delegation star when it is flush against yield. I guess this borders on subjective, but let me point out two facts which point to yield * being more obvious than yield*:

  1. Almost every JavaScript syntax highlighter colors yield and * the same color.
  2. No experienced programmer is spending their time reading keywords when scanning code.

Putting the star against the operand makes the delegation much more obvious. Just look at the examples I posted.

yield* this.isABusyCallExpression(a + b);
yield *this.isABusyCallExpression(a + b);

Are you really going to hold the position that the former is more obviously a delegated yield than the latter?

bakkot commented 4 years ago

Isn’t the fact that both function and function * create invokables an argument that the generator star is a modifier, not a keyword?

... No? Lots of things create functions. => creates functions. function* is just another way of declaring a function.

Again, I'm not claiming that function* is literally a keyword. I'm just saying that it makes sense to treat it as one. This is a position that multiple other contributors - at least josephfrazier and vjeux - have expressed as well. (And at least a couple of other contributors :+1:'d that first comment too.)

The * in yield * behaves EXACTLY like a prefix operator. Check out the following code:

Again, I am intimately familiar with how JavaScript is parsed. But the way I would think of this is that some operations, like await and yield*, require an argument, whereas others, like yield, do not. This makes perfect sense if you think of yield* as being a different operation than yield. Which it is: yield yields exactly once, whereas yield* yields arbitrarily many times.

On the other hand, unlike actual prefix operators like ! or ~, you cannot use the * from yield * anywhere except immediately following yield. This is because yield* is one unit: the * is a part of yield*. It is not a part of the subsequent expression.

Are you really going to hold the position that the former is more obviously a delegated yield than the latter?

Yes, that is my position. Much more obviously, to my eye, especially (but not only) if I've been reading C++ or Rust recently.

brainkim commented 4 years ago

Again, I'm not claiming that function* is literally a keyword. I'm just saying that it makes sense to treat it as one.

I don’t think this line of argumentation is sound. There are plenty of syntax combinations which change the behavior of code: async function makes functions async, for loops vary based on whether its expression contains an in or of. And I hate to be “mansplaining” JavaScript syntax to a spec contributor, but your position that we treat syntax combinations which do different things as atomic “keywords” has so many counter-examples. Just because different syntaxes do different things doesn’t mean the tokens should be treated as keywords, and the fact that you probably think async function should not be “treated like” a keyword but function* should, despite the fact that these syntax combinations are more or less analogous, makes me think your reasoning is inconsistent and circular. You want to call function* a keyword because you want there not to be a space between the two tokens, and you want there to be no spaces between function and * because you think they’re a keyword.

On the other hand, unlike actual prefix operators like ! or ~, you cannot use the from yield anywhere except immediately following yield.

Yeah I conceded this, and mentioned ... syntax as the closest analogue. If the ... token could possibly appear after a word, I would similarly argue that it should be word ...expression.

bakkot commented 4 years ago

your position that we treat syntax combinations which do different things as atomic “keywords” has so many counter-examples

I'm not saying we should treat all such combinations as keywords. I'm saying it makes sense to treat function* and yield*, specifically, as keywords, for the purposes of formatting code. This is not something for which there is an objectively correct answer, but mine is an intuition that many other people share - I linked to two other maintainers above independently expressing the same point of view, one of those comments has a upvotes from multiple other maintainers, the AirBnB style guide linked in the OP says the same thing, etc.

thorn0 commented 4 years ago

But if you like we can look elsewhere: function* is the style used by the spec itself, MDN, rauschma's tutorial, Eloquent JS, pretty much every other tutorial I could find (etc, etc, etc, etc), babel, wu.js, gatsby, co, node, deno, v8, spidermonkey, angular, bluebird, typescript, GraphQL, msgpack-javascript, etc.

FWIW Babel, Deno and GraphQL use Prettier. Co's readme isn't consistent in how generators are formatted. And as I mentioned, a popular style guide Standard JS is against this style.

That said, the comment in #3847 gives us something of an opportunity to distinguish between "people don't care" and "people agreed with what was already happening":

Still think I may very well be right about that comment. Did you read other comments there? Did you see how much people care about that space? Everything that was at least remotely in support of it got upvoted. The closer to the OP the more attention and votes.

so the comment asking about function* reached a larger than usual subset of users not preselected for disliking prettier's generator formatting.

They totally were preselected. Prettier's formatting was function*(). Those users came there because they disliked function(). Obviously, they disliked function*() too.

This makes perfect sense if you think of yield* as being a different operation than yield.

But is it really easier to distinguish visually these two different operations when yield* is written as one word than when there is a space that presumably makes * more visible?

All that said, I don't have much to add to this discussion. If other collaborators are strongly against this change, obviously it has to be reverted.

glen-84 commented 4 years ago

@thorn0,

Can you see what I mean? Isn't it expressive when similar things are formatted in a similar way? The formatting helps communicate the fact that the same idea of spreading is in play here.

So what does the star spread in function *gen()?

I see the star as meaning "generator", so:

function* = generator function yield* = yield from a generator

You don't spread a generator, you yield a value from it.

Also that space would seem especially strange in private class methods

I would argue that two sigils next to each other might be difficult to read, but remember that this issue is not supposed to be about the ideal formatting, but instead about how a change was made with very little consensus. You think that "no one seems to care", but you've not considered "no one cares because they're happy with the way that it is currently" – this is why there should be a couple of hundred votes + contributer consensus before such changes are made (IMO).

And don't get me started on the # sigil prefix. That one still hurts. 😆

And as I mentioned, a popular style guide Standard JS is against this style.

They're also against using *gen, and the issue that you linked to literally provides another example of someone expecting function*.

@brainkim,

Almost every JavaScript syntax highlighter colors yield and * the same color.

You still haven't provided references.

No experienced programmer is spending their time reading keywords when scanning code.

References?

thorn0 commented 4 years ago

So what does the star spread in function *gen()?

I was talking only specifically about * after yield in this case.

yield* = yield from a generator

This false impression (I wonder how widespread it is) is one of the reasons why yield ... would be a better syntax.

yield* = iterate through an iterable yielding values from it one by one. In other words, it's a shortcut for for (const value of iterable) yield value;.

You don't spread a generator, you yield a value from it.

yield* accepts any iterables: arrays, generators, strings... Just like spread syntax in array literals and function calls.

there should be a couple of hundred votes + contributer consensus before such changes are made

What do you mean, "such"? Prettier makes lots of formatting changes in every release. Only some of them lead to massive code reformatting and those should be considered really carefully. But this specific change isn't a change of that scale.

[Standard JS is] also against using *gen, and the issue that you linked to literally provides another example of someone expecting function*.

Still it's a precedent in the ecosystem. A precedent against function*. And their justification is similar to brainkim's: make the star stand out. Apparently, they thought it needs to stand out even more.

brainkim commented 4 years ago

Almost every JavaScript syntax highlighter colors yield and * the same color.

I was wrong about yield and * coloring the same on most editors. It seems like the big exception is VSCode/Atom which color them differently. I use vim exclusively and the syntax highlighting is not great.

RE: “experienced programmers don’t look at keywords.” I don’t have an eye tracking study, but are you gonna argue that you’re scrutinizing every await, yield, or return when you look at JS?

I still think it’s pretty objective that adding a space between the keywords and the star is more obvious, and I can’t for the life of me figure out how anyone can think differently.

RE: yield * is a single unit because it does different things:

return !expression does a different thing from return expression and yet it would be wild to suggest return! expression right? Why is yield * any different? It can often make sense to either yield or yield * the same expression, and from the perspective of the generator the result is the same, you suspend execution until something outside the generator’s control resumes it.

We’ve established that function * and yield * are neither keywords nor the * a prefix operator, but if we count up all the analogies and disanalogies, the count is overwhelmingly in the favor of it being closer to a prefix operator. It can be added and removed independently of the function or yield, adding a star without an expression is a syntax error in yield, and no one here is arguing that if the syntax was yield ..., which makes infinitely more sense, we’d format it like yield... expression.

RE: “you didn’t work off consensus.” If TC39 dies it’s gonna be because they perpetually tried to do the impossible of building consensus with people who never concede when they’re clearly wrong, and moreover seem to be bad-take-factories about literally everything in JavaScript (I’m not referring to anyone in present company). It’s impossible to build consensus with people who don’t seem to be arguing in good faith.

bakkot commented 4 years ago

return !expression does a different thing from return expression

No, it doesn't. The first evaluates !expression and returns it, and the second evaluates expression and returns it. In both cases you are evaluating the thing after return, and then returning it.

It can often make sense to either yield or yield * the same expression, and from the perspective of the generator the result is the same, you suspend execution until something outside the generator’s control resumes it.

First, that's not actually true: yield* might not suspend at all (if the iterable is empty). And second, they are very different from the perspective of the whole program, which is the thing you care about; "from the perspective of the generator" is not really that interesting.

if we count up all the analogies and disanalogies, the count is overwhelmingly in the favor of it being closer to a prefix operator

I disagree with this very strongly. But I also don't think it's the only relevant question; "what is the usual formatting in the rest of the ecosystem" is at least as important.

brainkim commented 4 years ago

The first evaluates !expression and returns it, and the second evaluates expression and returns it. In both cases you are evaluating the thing after return, and then returning it.

I don’t fully understand the distinction between return ! and yield * doing completely different things. The end result is the return value of the function is changed. In the first case, it’s coerced to a boolean and negated, in the second case, the yielded expression is flattened into the returned iterator. What is the principled distinction that you’re making which makes yield* special while return! not special such that you format the former “like a keyword”? This distinction can’t be that yield* somehow changes the execution of the program outside of the function, because return! could also theoretically change how the caller executes. I don’t think there is a principled distinction you could make which distinguishes return ! and yield *, and even if you care about the total effects of your programs, saying you need to care about all of it when writing local functions breaks the idea that functions are about abstracting and encapsulating logic.

brainkim commented 4 years ago

And even if we say yield * is special, thorn0’s point still stands. If you think it should be treated differently shouldn’t we format it in the most obvious way? I think reasonable people would say that yield * is more obvious than yield*, and I don’t really know what from C++/Rust makes you think otherwise.

bakkot commented 4 years ago

What is the principled distinction that you’re making which makes yield* special

yield* does something which yield x cannot do for any expression x. That is the distinction.

return expression returns a value and return !expression returns a (different) value. Those are the same operation. You could just as well do expression = !expression; return expression and you would get the same effect. That's how expressions work.

But yield* is not like this. It is performing a different operation than yield is. There is no value for expression2 which will make yield expression2 behave like yield* expression1; you have to actually use yield* (or a loop).

Honestly this argument is getting a little surreal, so I'm going to step away.

If you think it should be treated differently shouldn’t we format it in the most obvious way?

I do think we should format it the most obvious way! The easiest way to figure out what the most obvious way is is to look at what everyone defaults to. Everyone defaults to function* and yield*. We should do that.

I think reasonable people would say that yield * is more obvious than yield*

I don't. A lot of people, evidently, don't. Reasonable people can disagree.

ljharb commented 4 years ago

The airbnb config has always required the * be next to function, not the name, because it's part of what kind of thing it is - it's not part of the (optional) name.

function *() {} is weird, so function *foo() {} is weird too.

brainkim commented 4 years ago

I want to stop but I can’t 😂.

yield* x does something which yield x cannot do for any expression x.

Okay I understand that you can’t do with yield what you would do with a yield* on the same line. That’s mildly compelling. But isn’t the following statement also true?

return! x does something which return x cannot do for any expression x.

There is nothing the expression x could resolve to that would make return! and return perform “the same operation”: that’s just the law of the excluded middle. Maybe we’re delving into some philosophical notions of what an “expression” is, but there is no value which also cancels out its own negation, right?

thorn0 commented 4 years ago

@ljharb So what about methods then?

ljharb commented 4 years ago

@thorn0 altho i'd say that could be decided differently, the airbnb config mandates * foo() {} just like you do async foo() {}. The syntax that denotes the kind of method is separate from the name.

thorn0 commented 4 years ago

@ljharb What about async and static methods?

glen-84 commented 4 years ago

@thorn0,

I was talking only specifically about * after yield in this case.

So this "prefix operator" behaves differently in different positions? Is that common in JS?

What do you mean, "such"?

Any changes that aren't considered bug fixes.

Prettier makes lots of formatting changes in every release.

Where can I find a list of non-bug-fix formatting changes in each release?

@brainkim,

but are you gonna argue that you’re scrutinizing every await, yield, or return when you look at JS?

Not scrutinizing, but I'd like to think that I'd notice an asterisk after a function/yield keyword. How many developers have complained about not being able to notice the asterisk with the current formatting?

It can be added and removed independently of the function or yield, adding a star without an expression is a syntax error in yield

No it can't. If you remove the yield asterisk, you're no longer yielding to a generator/iterable, and if you remove the function asterisk, you no longer have a generator. They work together.

and no one here is arguing that if the syntax was yield ..., which makes infinitely more sense, we’d format it like yield... expression.

But spreads happen within brackets, (...x), [...y], {...z}. If you just used yield ...x, that would expand to yield a, b, c? They don't seem like similar concepts to me.

It’s impossible to build consensus with people who don’t seem to be arguing in good faith

I am arguing in good faith, and not just for myself. However, I'm done now. If Prettier's process is "if it suits 2 or 3 contributers, while potentially affecting 1.3m other projects*, that's good enough for us", then I'm not sure what else I can say.

[*] We can't know how many of these projects use generators, but that's for you to prove before making changes.

PS. There's an asterisk after projects, in case you didn't notice it. (I'm just kidding :heart:)

thorn0 commented 4 years ago

So this "prefix operator" behaves differently in different positions? Is that common in JS?

Many tokens are used for multiple purposes. As for operators, + can be binary and unary. As for "quasi-operators", spreads in arrays and objects have different mechanics. Etc, etc.

Anyway, the analogy with operators and spreads makes sense only for * after yield because there is an expression after it, and operators are applied to expressions. We're discussing the formatting of two different language constructs. Quite expectedly, not all the points made in this discussion are applicable to both.

Where can I find a list of non-bug-fix formatting changes in each release?

In the blog. Please also have a look at this explanation.

If you just used yield ...x, that would expand to yield a, b, c?

To yield a; yield b; yield c;. Expanding to a comma-operator expression would be useless. (Although it would make sense if yield actually supported this syntax and yield *[a, b, c], yield ...[a, b, c] and yield a, b, c all did the same thing, but it's too late to add this one.)

They don't seem like similar concepts to me.

Honestly?

There's an asterisk after projects, in case you didn't notice it. (I'm just kidding

BTW, what would really be useful is to ask visually impaired developers what position of the star would be better for them.

ljharb commented 4 years ago

@thorn0 the "function" keyword is omitted in concise methods, but all spacing remains otherwise unchanged - it's all quite consistent. The same logic is applied to the function's name - either it's there or not, but the spacing is unaffected.

function* () {}
function* foo() {}

async function () {}
async function foo() {}

async function* () {}
async function* foo() {}

class C {
  * foo() {}
  async foo() {}
  async * foo() {}

  static * foo() {}
  static async foo() {}
  static async * foo() {}
}
thorn0 commented 4 years ago

So, this change turned out a bit more controversial than it initially seemed to be. I'm not comfortable with including it in the release. At least in the next release.

We can keep this issue open or better open a new one with a brief summary of the discussion above (key arguments for and against).

Meanwhile, I'd like to proceed with reverting.

alexander-akait commented 4 years ago

We can keep this issue open or better open a new one with a brief summary of the discussion above (key arguments for and against).

Yes, a new issue will be great, we can close it after reverting