Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
imranolas opened this issue Oct 11, 2017 · 5 comments
Closed

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

imranolas opened this issue Oct 11, 2017 · 5 comments
Labels
area:comments Issues with how Prettier prints comments lang:javascript Issues affecting JS locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program

Comments

@imranolas
Copy link

Prettier 1.7.4
Playground link

Input:

Promise.resolve().then().then();

// prettier-ignore

Output:

Promise.resolve().then().then();

// prettier-ignore

Prettier 1.7.4
Playground link

Input:

Promise.resolve().then().then();

// prettier-ignore

Promise.resolve().then().then();

Output:

Promise.resolve()
  .then()
  .then();

// prettier-ignore

Promise.resolve().then().then();

When the // prettier-ignore comment is the last line, it ignores the previous node.

Expected behavior:

The first line should be formatted in both examples.

@azz
Copy link
Member

azz commented Oct 11, 2017

This is likely due to the way comments with in prettier. They are attached to AST nodes, and in this case there's nothing after the comment so it is added as a dangling comment of the previous statement.

We could probably fix it by checking that the start index of the comment is less than the start index of the AST node.

I'm curious what caused you to find this issue? Was it just by observation, or is it causing a real issue?

@azz azz added area:comments Issues with how Prettier prints comments lang:javascript Issues affecting JS labels Oct 11, 2017
@imranolas
Copy link
Author

Granted it's very much an edge case. I actually found it whilst attempting to recreate another issue with prettier-ignore.

@ikatyang ikatyang added the type:bug Issues identifying ugly output, or a defect in the program label Oct 15, 2017
@duailibe
Copy link
Member

duailibe commented Oct 22, 2017

@azz There's a case where we expect a prettier-ignore in a line after the node:

f( 1 )
// prettier-ignore

So I guess that's intentional?

@azz
Copy link
Member

azz commented Oct 22, 2017

Found it - looks like it is:

#671 (comment)

@azz
Copy link
Member

azz commented Oct 22, 2017

Going to close this as there could be cases where people are relying on the lack of a leading check, and fixing this would break them for no good reason.

@azz azz closed this as completed Oct 22, 2017
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 6, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:comments Issues with how Prettier prints comments lang:javascript Issues affecting JS locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

No branches or pull requests

4 participants