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

Keep the comment after where clause that has no predicate (#4649) #4774

Open
wants to merge 1 commit into
base: rustfmt-2.0.0-rc.2
Choose a base branch
from

Conversation

ChinYing-Li
Copy link
Contributor

@ChinYing-Li ChinYing-Li commented Mar 28, 2021

The pull request aims at solving #4649, and formats the comment that occurs before the where-clause; two cases are handled by the modified code:

  • where exists but does not have any predicate: For this scenario, it would be great to have people's input on
  1. the indent of the comment
  2. If we always put the comment in a new line (the current behavior is to always add \n right after where). Several test cases in issue_4649.rs would fail.
  • where does not exist. In this scenario, rustfmt behaves the same as before.

Any suggestion is appreciated!

@ChinYing-Li
Copy link
Contributor Author

Now that I looked through other place where where is formatted, perhaps we should create a function format_comment_after_where to be used?

@ChinYing-Li
Copy link
Contributor Author

If we use the approach in this PR to format where clause in all scenarios, I think #4672 can also be resolved.

@calebcartwright
Copy link
Member

Looks like this now has a lot of CI/test failures. Could you please take a look when you get a chance?

@ChinYing-Li
Copy link
Contributor Author

The idempotent tests were ran against two different config; the current failure output looks like theis:

Mismatch at tests/source/issue-4649.rs:7:
 
     fn bar2(&self, a: T)
     where
-        /* Self: Bar */;
  /* Self: Bar */;
 }
 

Mismatch at tests/source/issue-4001.rs:1:
 fn unit() -> ()
 where
-    /* comment */ {
  /* comment */ {
     ()
 }

But if I change the target snippet to match the outcome above, the failure output became:

Mismatch at tests/source/issue-4649.rs:7:
 
     fn bar2(&self, a: T)
     where
-  /* Self: Bar */;
  /* Self: Bar */;
 }
 

Mismatch at tests/target/issue-4649.rs:7:
 
     fn bar2(&self, a: T)
     where
-  /* Self: Bar */;
         /* Self: Bar */;
 }
 

Mismatch at tests/source/issue-4001.rs:1:
 fn unit() -> ()
 where
-  /* comment */ {
  /* comment */ {
     ()
 }
 

Mismatch at tests/target/issue-4001.rs:1:
 fn unit() -> ()
 where
-  /* comment */ {
     /* comment */ {
     ()
 }

What are some way to circumvent this?

@calebcartwright calebcartwright changed the base branch from master to rustfmt-2.0.0-rc.2 April 30, 2021 00:30
@ytmimi ytmimi added pr-targeting-2.0 This PR is targeting the 2.0 branch p-low labels Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p-low pr-not-reviewed pr-targeting-2.0 This PR is targeting the 2.0 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants