Skip to content

feat(fmt): wrap collect comments #3786

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
Closed
wants to merge 4 commits into from
Closed

feat(fmt): wrap collect comments #3786

wants to merge 4 commits into from

Conversation

rkrasiuk
Copy link
Collaborator

@rkrasiuk rkrasiuk commented Nov 29, 2022 •
edited
Loading

Motivation

close #3604

Solution

If the wrap_comments option is enabled, collect all adjacent comments before writing them.

The line comment and any subsequent line comment that starts with an alphanumeric character are now collected into a single comment. If the first line starts with - or * characters, any subsequent line comment will be padded with whitespace to align with the first alphanumeric character from the first line.

@rkrasiuk rkrasiuk added T-feature Type: feature C-forge Command: forge Cmd-forge-fmt Command: forge fmt labels Nov 29, 2022
@rkrasiuk rkrasiuk marked this pull request as draft November 29, 2022 11:53
@rkrasiuk
Copy link
Collaborator Author

rkrasiuk commented Dec 5, 2022

pending @mds1 feedback

@mds1
Copy link
Collaborator

mds1 commented Dec 5, 2022

Just tested using the same FractionalPool.sol contract/repo linked in the original issue, I cloned that repo and changed line width from 100 to 80, then ran forge fmt on this branch. Definitely getting there but still a few issues. Some comments:

  1. On L25 of the contract (where the /// @notice line is): There's an extra space before the @ sign, which isn't removed when formatting, but it probably should be
  2. The fourth bullet point in that same comment block spans three lines. After formatting, the second and third lines no longer align with where text starts (similarly, the third bullet wraps to a second line but is not aligned with the text)
  3. The last two words of that last bullet, is completed, get merged into iscompleted after formatting

@rkrasiuk rkrasiuk mentioned this pull request Jan 13, 2023
Closed
2 tasks
@gakonst
Copy link
Member

gakonst commented Jan 17, 2023

@rkrasiuk planning to get this over the line? will keep open if so

@rkrasiuk
Copy link
Collaborator Author

@gakonst yes, ser

@rkrasiuk rkrasiuk mentioned this pull request Jan 19, 2023
Closed
@rkrasiuk
Copy link
Collaborator Author

this needs to be rewritten

@rkrasiuk rkrasiuk closed this Mar 14, 2023
@DaniPopes DaniPopes deleted the rkrasiuk/fmt-wrap-collect-comments branch November 7, 2023 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaniPopes DaniPopes Awaiting requested review from DaniPopes DaniPopes will be requested when the pull request is marked ready for review DaniPopes is a code owner

@klkvr klkvr Awaiting requested review from klkvr klkvr will be requested when the pull request is marked ready for review klkvr is a code owner

@mattsse mattsse Awaiting requested review from mattsse mattsse will be requested when the pull request is marked ready for review mattsse is a code owner

@grandizzy grandizzy Awaiting requested review from grandizzy grandizzy will be requested when the pull request is marked ready for review grandizzy is a code owner

@yash-atreya yash-atreya Awaiting requested review from yash-atreya yash-atreya will be requested when the pull request is marked ready for review yash-atreya is a code owner

@zerosnacks zerosnacks Awaiting requested review from zerosnacks zerosnacks will be requested when the pull request is marked ready for review zerosnacks is a code owner

Assignees
No one assigned
Labels
C-forge Command: forge Cmd-forge-fmt Command: forge fmt T-feature Type: feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug(fmt): comment wrapping isn't always handled properly
3 participants
@rkrasiuk @mds1 @gakonst