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

Better printing extensions/attributes inside sequences (and other nested expressions) #2677

Open
davesnx opened this issue Nov 12, 2022 · 0 comments
Labels
Printer things that have to do with turning an AST into Reason code

Comments

@davesnx
Copy link
Member

davesnx commented Nov 12, 2022

Extensions are still a work in progress, there’s a TODO in the code and might appear some edge-cases after this PR #2674.

There’s a bit of ambiguity on extensions/comments/sequences and I lack the vision to have a clear desired output, so let me explain what are the differences with code examples.

Current pp output:

/* Extension with pexp_apply */

[%defer
  cleanup();
]

/* Extension with comment with pexp_apply */

[%defer
  /* 2. comment attached to expr in extension */
  cleanup();
];

/* Let sequence + extension with pexp_apply */

let () = {
  /* random let binding */
  let x = 1;
  /* 1. comment attached to extension */
  [%defer cleanup()];
  /* 3. comment attached to next expr */
  something_else();
};

/* Let sequence + extension with comment with pexp_apply */

let () = {
  /* random let binding */
  let x = 1;
  /* 1. comment attached to extension */
  [%defer
   /* 2. comment attached to expr in extension */
   cleanup()];
  /* 3. comment attached to next expr */
  something_else();
};

Note how being inside a sequence makes formatting different and I think this shoudn’t be the case, but unsure if applying the same formatting would make sense. That’s why I’m openning this issue.

Related: Attributes and other expressions add single space as identation.

@davesnx davesnx added the Printer things that have to do with turning an AST into Reason code label Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Printer things that have to do with turning an AST into Reason code
Projects
None yet
Development

No branches or pull requests

1 participant