Skip to content

feat: format list-like objects with comments #154

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

Merged
merged 6 commits into from
Nov 27, 2024

Conversation

QuadnucYard
Copy link
Collaborator

@QuadnucYard QuadnucYard commented Nov 23, 2024

This PR rewrites the pretty conversion of list-like objects, including arrays, dicts, destructions, and parameters (and closure definitions).
It supports comments within these objects. Each item can be attached with comments at the front or back.

It also fixes:

  • An array with one item is prevented from being split into multiple lines when the column is narrow.
  • Patterns in destructuring may be lost after formatting.

Except that, the styles remain unchanged (if formattable before).

Some examples:

#(
   555,  // 67654
          999  /* 65543*/ ,
    123,
 /* 65543*/
    // 454456
  /* 1 */     546, /* 65543*/
       777 /* 65543*/,
      /* 65543*/
  789, 1445, 654,

  /* 5 */     666, 777, /* 659 */

)

#(1  ,   2  ,   /* */ 3  )
#(   1 , 2, /* */     3,)
#(   1 ,2 /* */ ,  3 , )
#(1,2,  3, /* */   )
#(1,)
#( /* */1,)
#( 1/* */,)
#( 1,/* */)

#( /* */..() , )
#( ../* */()  ,)
#( ..  () /* */,)
#( ..()  ,/* */  )

may be formatted to

#(
  555, // 67654
  999, /* 65543*/
  123,
  /* 65543*/
  // 454456
  /* 1 */ 546, /* 65543*/
  777, /* 65543*/
  /* 65543*/
  789,
  1445,
  654,
  /* 5 */ 666,
  777, /* 659 */
)

#(1, 2, /* */ 3)
#(1, 2, /* */ 3)
#(1, 2 /* */, 3)
#(1, 2, 3 /* */)
#(1,)
#(/* */ 1,)
#(1 /* */,)
#(1 /* */,)

#(/* */ ..(),)
#(../* */(),)
#(..() /* */,)
#(..() /* */,)

and

#(/* 1 */ // first comment
   a:    /* 2 */
/* 3 */   1/* 4 */     ,   // second comment
    b   /* 5 */    :/* 6 */ 2,/* 7 */c   : 3/* 8 */ ,
   /* 9 */ d/* 10 */
/* 11 */    :/* 12 */

/* 13 */
   /* 14 */ 5/* 15 */
/* 16 */)

to

#(
  /* 1 */
  // first comment
  a: /* 2 */ /* 3 */ 1, /* 4 */ // second comment
  b /* 5 */: /* 6 */ 2,
  /* 7 */ c: 3, /* 8 */
  /* 9 */ d /* 10 */ /* 11 */: /* 12 */ /* 13 */ /* 14 */ 5, /* 15 */
  /* 16 */
)

It also fixes a bug with destruction. Previously,

#let (a, b: (.., d), ..c) = (a:1, b:(c: 4, d: 5))

will be formatted to

#let (a, b: b, ..c) = (a: 1, b: (c: 4, d: 5))

Now the pattern is handled correctly.

@QuadnucYard
Copy link
Collaborator Author

A big challenge is func-call and chain-access. It can be divided into two cases, not considering comments:

#{
  // If we can put everything except the arguments of the last one in one line and the last one is *heavy enough*.
  a.b().c().d(
    e(..)
  )
  
  // Otherwise, chain them.
  a
    .b(..)
    .c(..)
    .d(e(..))
}

We don't want to see an item in the middle split into multiple lines while there exists .a().b().

@QuadnucYard
Copy link
Collaborator Author

Some insights about comment handling.

We can abstract a line as before item, after, where before and after are comments.
When we see:

  • a comment, we add it to free_comments for future use.
  • a list item, if we have any free comments, join them together.
  • a comma, we attach all free comments to the last item we saw (if any).
  • a line break, if there is an item in the same line, attach free comments to the last one.

If the items are in a single line, we put the attached block comment before the comma. Otherwise after the comma. So the comma positions may change.

@Enter-tainer
Copy link
Owner

BTW, is dot chain formatting still a problem? I haven't examine snapshots yet. I can help take a look if the problem still present.

@QuadnucYard
Copy link
Collaborator Author

BTW, is dot chain formatting still a problem? I haven't examine snapshots yet. I can help take a look if the problem still present.

I haven't touched it yet, so the chain formatting is expected to remain unchanged.
I will work on it in the next PR.

@QuadnucYard QuadnucYard marked this pull request as ready for review November 26, 2024 03:03
Copy link
Owner

@Enter-tainer Enter-tainer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just go through all snapshot updates

Copy link
Owner

@Enter-tainer Enter-tainer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Enter-tainer
Copy link
Owner

Enter-tainer commented Nov 26, 2024

It also fixes a bug that an array with one item is not broke into multiple lines when the column is narrow.

For closure parameters, I do not make "one parameter" a special case (although it easy to implement), to align with the format behavior of multiple parameters. So this is a breaking change. Since the structure of a closure has not been handled very well yet, this change may cause dissatisfaction.

BTW please consider editing the PR description to reflect latest changes. thanks!

Copy link
Owner

@Enter-tainer Enter-tainer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Enter-tainer Enter-tainer merged commit 5c81d5f into Enter-tainer:master Nov 27, 2024
11 checks passed
@QuadnucYard QuadnucYard deleted the format-comments-2 branch November 27, 2024 02:40
@QuadnucYard QuadnucYard mentioned this pull request Dec 6, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants