Skip to content

feat: compact layout for args of func_call#261

Merged
QuadnucYard merged 12 commits intotypstyle-rs:masterfrom
QuadnucYard:compact-layout
Apr 30, 2025
Merged

feat: compact layout for args of func_call#261
QuadnucYard merged 12 commits intotypstyle-rs:masterfrom
QuadnucYard:compact-layout

Conversation

@QuadnucYard
Copy link
Copy Markdown
Collaborator

@QuadnucYard QuadnucYard commented Apr 19, 2025

Resolves #95.
Compact layout is preferred when the args does not contain comments or linebreaks, and the last arg is not binary expression.
Now the flavor of args is ignored. Otherwise, the compact layout will not apply to existing multiline args.
Added a width limit for the initial args except the last, which is a hardcoded 80% width. This does not work well with inline chains

Some problems:

  • We use union and a brute-force flatten, which are not performant.
  • The trailing args in brackets are not counted in the compact layout, so they could exceed the width limit in the new layout.
    • It may look better to break inside the brackets than args. But unfortunately, we cannot unconditionally insert breaks in markup block.
  • If it has two args of the same structure, they may not be formatted the same way.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 19, 2025

📊 Benchmark Performance Report

group                        base                                   pr
-----                        ----                                   --
pretty-cetz-manual           1.00    843.9±7.54µs        ? ?/sec    1.15    969.7±6.32µs        ? ?/sec
pretty-codly                 1.00  1882.7±21.92µs        ? ?/sec    1.10      2.1±0.02ms        ? ?/sec
pretty-cpe                   1.04     13.6±0.16ms        ? ?/sec    1.00     13.1±0.24ms        ? ?/sec
pretty-fletcher-diagram      1.00    533.6±8.52µs        ? ?/sec    1.09    583.1±8.82µs        ? ?/sec
pretty-fletcher-draw         1.00  1283.4±12.58µs        ? ?/sec    1.12  1440.3±16.20µs        ? ?/sec
pretty-tablex                1.00      3.5±0.02ms        ? ?/sec    1.10      3.8±0.02ms        ? ?/sec
pretty-touying-core          1.00      2.2±0.02ms        ? ?/sec    1.13      2.5±0.01ms        ? ?/sec
pretty-touying-utils         1.00  1258.7±12.32µs        ? ?/sec    1.10  1385.0±21.11µs        ? ?/sec
pretty-undergraduate-math    1.00   993.0±16.31µs        ? ?/sec    1.03  1025.5±21.78µs        ? ?/sec

Generated by GitHub Actions on Wed Apr 30 12:57:29 UTC 2025

@QuadnucYard
Copy link
Copy Markdown
Collaborator Author

problems with compact layout:

  • It does not respect the flavor of args, which is more opinionated. Can this lead to bad output?
  • It does not work well with some DSLs that contain linebreaks in args. But in this case, we can simply insert a line comment to disable compact layout.

@QuadnucYard
Copy link
Copy Markdown
Collaborator Author

there are some weird behaviors with pretty, causing compact layout unexpectedly fail to fit
one example is

#{
  {
    {
      cetz.group(
        {
          {
            cetz.draw.set-style(
              fill: if i == 0 { node.fill },
              stroke: if i == 0 { node.fill },
            )
          }
        },
      )
    }
  }
}

modifying almost any piece of code can make the compact layout of cetz.group(...) success

@QuadnucYard
Copy link
Copy Markdown
Collaborator Author

Maybe we can hold on to sticky braces to bypass such problems.
After all, compact layout will not make things worse. It can only degrade to the layout in the current version.

@QuadnucYard QuadnucYard marked this pull request as ready for review April 28, 2025 01:38
@QuadnucYard QuadnucYard requested a review from Copilot April 28, 2025 01:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new "Compact" fold style and adjusts the layout for function call arguments to use a compact layout under specific conditions.

  • Added Compact variant to FoldStyle in style.rs.
  • Updated list and chain layout logic to support the compact layout.
  • Revised function call argument processing and introduced new helper traits in doc_ext.rs, as well as a new args_width method in config.rs.

Reviewed Changes

Copilot reviewed 156 out of 170 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/typstyle-core/src/pretty/style.rs Added the new Compact variant to FoldStyle.
crates/typstyle-core/src/pretty/layout/list.rs Introduced a compact layout branch for list items with conditional grouping and union logic.
crates/typstyle-core/src/pretty/layout/chain.rs Simplified grouping logic for chain layouts.
crates/typstyle-core/src/pretty/func_call.rs Updated fold style selection logic for function call arguments based on expression types.
crates/typstyle-core/src/pretty/doc_ext.rs Added new arena flattening extension traits for document builders.
crates/typstyle-core/src/config.rs Added a new args_width method to compute argument width with a fixed ratio.
Files not reviewed (14)
  • tests/fixtures/articles/snap/book.typ-0.snap: Language not supported
  • tests/fixtures/articles/snap/book.typ-40.snap: Language not supported
  • tests/fixtures/articles/snap/undergraduate-math.typ-0.snap: Language not supported
  • tests/fixtures/articles/snap/undergraduate-math.typ-120.snap: Language not supported
  • tests/fixtures/articles/snap/undergraduate-math.typ-40.snap: Language not supported
  • tests/fixtures/articles/snap/undergraduate-math.typ-80.snap: Language not supported
  • tests/fixtures/packages/snap/cetz-manual.typ-0.snap: Language not supported
  • tests/fixtures/packages/snap/cetz-manual.typ-120.snap: Language not supported
  • tests/fixtures/packages/snap/cetz-manual.typ-40.snap: Language not supported
  • tests/fixtures/packages/snap/cetz-manual.typ-80.snap: Language not supported
  • tests/fixtures/packages/snap/cetz-tree.typ-0.snap: Language not supported
  • tests/fixtures/packages/snap/cetz-tree.typ-120.snap: Language not supported
  • tests/fixtures/packages/snap/cetz-tree.typ-40.snap: Language not supported
  • tests/fixtures/packages/snap/cetz-tree.typ-80.snap: Language not supported

Copy link
Copy Markdown
Collaborator

@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.

snapshots LGTM

@QuadnucYard QuadnucYard merged commit 9a977d3 into typstyle-rs:master Apr 30, 2025
15 checks passed
@QuadnucYard QuadnucYard deleted the compact-layout branch April 30, 2025 13:01
@QuadnucYard QuadnucYard mentioned this pull request May 1, 2025
12 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.

Better handling for last arg in function call

3 participants