Support "// prettier-ignore" in comment blocks #1125

Merged
merged 2 commits into from Apr 5, 2017

Conversation

Projects
None yet
3 participants
@ericclemmons
Contributor

ericclemmons commented Apr 5, 2017

Pretty much took the example from #1109.

The important caveat is that // prettier-ignore has to be the only or last comment attached to a node.

I could update this to use a .filter(...).length check so it can exist anywhere in the comments, but felt that doesn't really make sense cosmetically to warrant it.

@vjeux vjeux merged commit def91ef into prettier:master Apr 5, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Apr 5, 2017

Collaborator

Thanks, I think it makes sense.

Note that it can be attached as a trailing comment as well.

function x() {
  a(  a  ); // prettier-ignore
  // something else
}

is no longer going to work with your code. So we may want to do comments.some(comment => comment.text === 'prettier-ignore') to be on the safe side, but I'm not really sure if it's worth caring about this edge case. Happy to take another PR for it.

Collaborator

vjeux commented Apr 5, 2017

Thanks, I think it makes sense.

Note that it can be attached as a trailing comment as well.

function x() {
  a(  a  ); // prettier-ignore
  // something else
}

is no longer going to work with your code. So we may want to do comments.some(comment => comment.text === 'prettier-ignore') to be on the safe side, but I'm not really sure if it's worth caring about this edge case. Happy to take another PR for it.

@ericclemmons

This comment has been minimized.

Show comment
Hide comment
@ericclemmons

ericclemmons Apr 5, 2017

Contributor

Support for .some looks good enough to use:

http://node.green/#ES2015-built-ins-typed-arrays--TypedArray--prototype-some

I'll send another PR now.

Contributor

ericclemmons commented Apr 5, 2017

Support for .some looks good enough to use:

http://node.green/#ES2015-built-ins-typed-arrays--TypedArray--prototype-some

I'll send another PR now.

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Apr 5, 2017

Collaborator

We already have 7 usages of .some( in the printer :) Also, we run travis on node 4 to make sure it doesn't break for the lowest version of node we support.

Collaborator

vjeux commented Apr 5, 2017

We already have 7 usages of .some( in the printer :) Also, we run travis on node 4 to make sure it doesn't break for the lowest version of node we support.

@lydell

This comment has been minimized.

Show comment
Hide comment
@lydell

lydell Apr 5, 2017

Collaborator

(For the record, Array.prototype.some has been around since ES5.)

Collaborator

lydell commented Apr 5, 2017

(For the record, Array.prototype.some has been around since ES5.)

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