[RFC] Introduce prettier-ignore by vjeux · Pull Request #671 · prettier/prettier

vjeux · 2017-02-12T03:24:16Z

This was surprisingly easy to write!

prettier-ignore will leave the expression the comment is attached to printed as it was before.

Fixes #120

aaronyo · 2017-02-12T20:48:42Z

tests/disable/__snapshots__/jsfmt.spec.js.snap

@@ -0,0 +1,90 @@
+exports[`test disable.js 1`] = `
+"function a() {
+  // prettier-ignore-next-line


Is the test producing a false positive somehow? It looks the the code is expecting "prettier-disable-next-line" rather than "...-ignore-..."

I first named it ignore and then looked at eslint where it's disable. I must have forgotten to change this here :)

aaronyo · 2017-02-12T20:59:29Z

My $.02 on naming: maybe // prettier-disable-next-node or even just // prettier-disable-next. If I'm understanding the code correctly, the scope of the escape hatch is the next AST node? I think it might actually confuse me a tad at first if it is "next-line" but it doesn't behave like eslint.

Regardless, I'll be happy to have this feature! Thank you!

rtsao · 2017-02-12T21:06:56Z

Istanbul (which is fairly common) uses /* istanbul ignore next */ for skipping the next expression/block. Agreed that it seems confusing if next-line would actually ignore the next block/expression.

vjeux · 2017-02-12T22:02:04Z

Thanks for the feedback, I think that prettier-ignore-next is the best choice.

vjeux · 2017-02-13T02:53:38Z

Done!

jlongster · 2017-02-13T14:58:21Z

Yeah I was going to suggest a simpler name as well. We could even do // prettier-ignore and it would just read like a leading comment. I always have to look up eslint's ignore names because they are always 3-5 words. The easier we can make it to remember the better.

I don't think we need to worry about a learning curve and stick to eslint's conventions. This is something people will learn once and it's ok.

It there any chance you all would be ok with just // prettier-ignore?

jlongster · 2017-02-13T15:01:39Z

I'm interested to see if this ever interacts badly with our formatting. There might be places where if you use this it will inject code that leaves it in a weird indentation state or something. But I think this is the right idea.

jlongster · 2017-02-13T15:04:19Z

src/printer.js

+      util.locStart(node),
+      util.locEnd(node)
+    );
+  }


Try adding some test cases where the expression is an argument to function, a property of an object, etc. Those interactions are where things might get weird.

The problem is the above code will naively output \n's and will mess up the measurement system. We might need split the content on newline and insert our own hardlines to make sure it never gets messed up. We might should add a breakParent just to make sure it always breaks up the expression too.

vjeux · 2017-02-13T17:09:48Z

// prettier-ignore works for me. I can even remove the leading check so you can use it as a trailing comment if you want to.

aaronyo · 2017-02-13T18:45:28Z

+1 for // prettier-ignore

jlongster · 2017-02-14T01:00:09Z

I can even remove the leading check so you can use it as a trailing comment if you want to.

Huh, that's an interesting idea. I suppose there's no reason why not to do that. Sure!

This was surprisingly easy to write! Technically, it doesn't ignore the next line but the next expression or block but I'm guessing that people are already used to `// eslint-disable-next-line` so it's going to be an easier learning curve. Fixes prettier#120

vjeux · 2017-02-14T02:43:24Z

I just updated it to prettier-ignore and removed the leading check.

  // Incorrectly indented on purpose
      function f</* prettier-ignore */ T    :    B>(
        a : Array  <   number   > // prettier-ignore
      ) {

        call(
          f(         1          )
          // prettier-ignore
        )
      }

turns into

  // Incorrectly indented on purpose
  function f</* prettier-ignore */ T    :    B>(
    a : Array  <   number   > // prettier-ignore
  ) {
    call(
      f(         1          )
      // prettier-ignore
    );
  }

it works really well!

jlongster · 2017-02-14T02:58:47Z

Haha... /me cringes. Well, unleash the hounds I suppose.

sonaye · 2017-03-04T15:11:55Z

@vjeux how does this work exactly, can you summarize? couldn't find any docs on this. was checking on any #582 related updates.

vjeux · 2017-03-04T16:25:59Z

Oops, forgot to add documentation.

If you put a comment named prettier-ignore, it is going to print the raw text of the node it is attached to.

karlhorky · 2017-03-22T00:13:09Z

Would you guys want documentation for this? Where would it go? Just a quick paragraph with example in the readme?

vjeux · 2017-03-22T00:26:19Z

@karlhorky yeah, would be awesome to have it in the README! PR welcome!

karlhorky · 2017-03-22T00:53:47Z

Alright, I'll take a look at this tomorrow!

karlhorky · 2017-03-23T13:07:47Z

Ok, I've opened #1082.

Original PR: #671

aaronyo reviewed Feb 12, 2017

View reviewed changes

vjeux force-pushed the escape_hatch branch from 004d7cd to bcd5f68 Compare February 13, 2017 02:53

erbridge mentioned this pull request Feb 13, 2017

Wrap comments #265

Open

jlongster mentioned this pull request Feb 13, 2017

[RFC] Preserve new lines next to a + where either side is a string #670

Closed

jlongster reviewed Feb 13, 2017

View reviewed changes

vjeux changed the title ~~[RFC] Introduce prettier-disable-next-line~~ [RFC] Introduce prettier-disable Feb 13, 2017

vjeux changed the title ~~[RFC] Introduce prettier-disable~~ [RFC] Introduce prettier-ignore Feb 13, 2017

vjeux force-pushed the escape_hatch branch from bcd5f68 to 53a8ade Compare February 14, 2017 02:42

jlongster merged commit 162ae7b into prettier:master Feb 14, 2017

karlhorky mentioned this pull request Mar 23, 2017

Document prettier-ignore #1082

Merged

vjeux pushed a commit that referenced this pull request Mar 23, 2017

Document prettier-ignore (#1082)

862adaf

Original PR: #671

azz mentioned this pull request Oct 22, 2017

Previous node ignored if prettier-ignore is the last line #3008

Closed

lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 21, 2019

lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019

Uh oh!

Conversation

vjeux commented Feb 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronyo Feb 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vjeux Feb 12, 2017

Choose a reason for hiding this comment

Uh oh!

aaronyo commented Feb 12, 2017

Uh oh!

rtsao commented Feb 12, 2017

Uh oh!

vjeux commented Feb 12, 2017

Uh oh!

vjeux commented Feb 13, 2017

Uh oh!

jlongster commented Feb 13, 2017

Uh oh!

jlongster commented Feb 13, 2017

Uh oh!

jlongster Feb 13, 2017

Choose a reason for hiding this comment

Uh oh!

vjeux commented Feb 13, 2017

Uh oh!

aaronyo commented Feb 13, 2017

Uh oh!

jlongster commented Feb 14, 2017

Uh oh!

vjeux commented Feb 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jlongster commented Feb 14, 2017

Uh oh!

sonaye commented Mar 4, 2017

Uh oh!

vjeux commented Mar 4, 2017

Uh oh!

karlhorky commented Mar 22, 2017

Uh oh!

vjeux commented Mar 22, 2017

Uh oh!

karlhorky commented Mar 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

karlhorky commented Mar 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vjeux commented Feb 12, 2017 •

edited

Loading

aaronyo Feb 12, 2017 •

edited

Loading

vjeux commented Feb 14, 2017 •

edited

Loading

karlhorky commented Mar 22, 2017 •

edited

Loading