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.

Sign up for GitHub

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

Jump to bottom

Comments not emitted in chaining calls #200

Closed
tnhu opened this issue Jan 14, 2017 · 8 comments
Closed

Comments not emitted in chaining calls #200

tnhu opened this issue Jan 14, 2017 · 8 comments
Labels
area:comments Issues with how Prettier prints comments 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

@tnhu
Copy link

tnhu commented Jan 14, 2017

before:

request
  .post('/version')  // comment 1
  .set('Prefer', 'plurality=singular') /* comment 2 */
  .send()
  .end((error, response) => {});

after:

request
  .post('/version')
  .set('Prefer', 'plurality=singular')
  .send()
  .end((error, response) => {});

Related issues:

#31
#87
Copy link
Contributor

This most likely has to do with printMemberChain not using the generic way of printing things. Comments are preserved if there are no function calls.

request
  .post('/version')  // comment 1
  .set('Prefer', 'plurality=singular') /* comment 2 */
  .send()
  
request
  .post.version  // comment 1
  .set.Prefer.plurality.singular /* comment 2 */
  .send()
request.post("/version").set("Prefer", "plurality=singular").send();

// comment 1
request.post.version.set.Prefer.plurality.singular /* comment 2 */.send();

https://jlongster.github.io/prettier/#%7B%22content%22%3A%22request%5Cn%20%20.post('%2Fversion')%20%20%2F%2F%20comment%201%5Cn%20%20.set('Prefer'%2C%20'plurality%3Dsingular')%20%2F*%20comment%202%20*%2F%5Cn%20%20.send()%5Cn%20%20%5Cnrequest%5Cn%20%20.post.version%20%20%2F%2F%20comment%201%5Cn%20%20.set.Prefer.plurality.singular%20%2F*%20comment%202%20*%2F%5Cn%20%20.send()%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Afalse%7D%7D

@arnarthor arnarthor mentioned this issue Jan 15, 2017
Closed
type:bug Issues identifying ugly output, or a defect in the program label Jan 15, 2017
@jlongster jlongster mentioned this issue Jan 17, 2017
Closed
4 tasks
@jlongster
Copy link
Member

Moved to #275 to consolidate all the comment bugs

@jlongster
Copy link
Member

Re-opening as I didn't fix this one in my comment overhaul.

@jlongster jlongster reopened this Jan 26, 2017
@jlongster jlongster added the area:comments Issues with how Prettier prints comments label Jan 26, 2017
@dmnd
Copy link

dmnd commented Jan 29, 2017

Related. Comments on their own lines really mess with the output.

request
  // handle escaped backticks
  .post('/version')
  /* comment 2 */
  .set('Prefer', 'plurality=singular')
  .send()
request.// handle escaped backticks
post("/version")./* comment 2 */
set("Prefer", "plurality=singular").send();

I'm not sure if this belongs in here or in its own issue.

Copy link
Contributor

Thanks @dmnd, this is a known issue and there's a fix in #469

Also, awesome job on dedent package, it makes template literals usable!

@amasad
Copy link
Contributor

amasad commented Jan 31, 2017 •
edited
Loading

Is this the same issue?

str.replace(/\//g, '_')
  // Remove ending '=' 15 bits doesn't have it but just to be safe
  .replace(/=+$/, '');

prints:

str
  .replace(/\//g, "_")
  .// Remove ending '=' 15 bits doesn't have it but just to be safe
  replace(/=+$/, "");

@amasad
Copy link
Contributor

amasad commented Jan 31, 2017

Actually this reproduces on the prettier codebase:

$ npm run format:all
$ git diff src/printer.js

 function printNumber(rawNumber) {
-  return rawNumber.toLowerCase()
-    // Remove unnecessary plus and zeroes from scientific notation.
-    .replace(/^([\d.]+e)(?:\+|(-))?0*/, "$1$2")
-    // Make sure numbers always start with a digit.
-    .replace(/^\./, "0.")
-    // Remove trailing dot.
-    .replace(/\.(?=e|$)/, "");
+  return rawNumber
+    .toLowerCase()
+    .// Remove unnecessary plus and zeroes from scientific notation.
+    replace(/^([\d.]+e)(?:\+|(-))?0*/, "$1$2")
+    .// Make sure numbers always start with a digit.
+    replace(/^\./, "0.")
+    .// Remove trailing dot.
+    replace(/\.(?=e|$)/, "");
 }

Copy link
Contributor

@amasad for this, I have a fix that I need to properly extract out before we can release it. I want it in before I do the next release

diff --git a/src/comments.js b/src/comments.js
index e05c7f5..d31122e 100644
--- a/src/comments.js
+++ b/src/comments.js
@@ -152,7 +152,10 @@ function attach(comments, ast, text) {
       // If a comment exists on its own line, prefer a leading comment.
       // We also need to check if it's the first line of the file.

-      if (followingNode) {
+      if (enclosingNode.type === "MemberExpression" &&
+          followingNode.type === "Identifier") {
+        addLeadingComment(enclosingNode, comment);
+      } else if (followingNode) {
         // Always a leading comment.
         addLeadingComment(followingNode, comment);
       } else if (precedingNode) {

Feb 1, 2017
Make comments inside of MemberExpression look good
dface9d 
This is using the same technique as prettier#544 and will conflict with it when we try to merge both :)

Fixes prettier#200
Merged
Feb 2, 2017
Make comments inside of MemberExpression look good
2924aae 
This is using the same technique as prettier#544 and will conflict with it when we try to merge both :)

Fixes prettier#200
Feb 2, 2017
Make comments inside of MemberExpression look good (#556)
5042168 
This is using the same technique as #544 and will conflict with it when we try to merge both :)

Fixes #200
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 8, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area:comments Issues with how Prettier prints comments 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
Milestone
No milestone
Development

No branches or pull requests
5 participants
@dmnd @jlongster @tnhu @amasad