Skip to content

Tweak the pretty-printing of long function definitions #1562

Merged
WardBrian merged 12 commits intomasterfrom
pretty-printing-fundef-tweaks
Oct 17, 2025
Merged

Tweak the pretty-printing of long function definitions #1562
WardBrian merged 12 commits intomasterfrom
pretty-printing-fundef-tweaks

Conversation

@WardBrian
Copy link
Copy Markdown
Member

@WardBrian WardBrian commented Oct 16, 2025

This applies a similar logic to what was added for function calls in #1513. If the function name is long, start the box for it's arguments earlier to allow it to be raggedly formatted below rather than forcing it to indent all the way to the (.

This also makes it so each argument gets its own line when there are many arguments -- this helps with readability, IMO, and is common in formatters in other languages

Consider this example from @nsiccha:

// current formatting
  array[] int vote_countvote_countvote_countvote_count(array[] int rating,
                                                       array[] int item,
                                                       array[] int rater,
                                                       int I, int J) {
    int N = size(rating);
    array[I] int count_by_item = rep_array(1, I); // index 0:5 by 1:6
    for (n in 1 : N) {
      count_by_item[item[n]] += rating[n];
    }
    array[J + 1] int count = rep_array(0, J + 1);
    for (i in 1 : I) {
      count[count_by_item[i]] += 1;
    }
    return count;
  }
// this pr
  array[] int vote_countvote_countvote_countvote_count(
    array[] int rating,
    array[] int item,
    array[] int rater,
    int I,
    int J
  ) {
    int N = size(rating);
    array[I] int count_by_item = rep_array(1, I); // index 0:5 by 1:6
    for (n in 1 : N) {
      count_by_item[item[n]] += rating[n];
    }
    array[J + 1] int count = rep_array(0, J + 1);
    for (i in 1 : I) {
      count[count_by_item[i]] += 1;
    }
    return count;
  }

Submission Checklist

  • Run unit tests
  • Documentation
    • If a user-facing facing change was made, the documentation PR is here:
    • OR, no user-facing changes were made

Release notes

Improved the formatting of long functions (either with long names or many arguments).

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

@nsiccha
Copy link
Copy Markdown

nsiccha commented Oct 16, 2025

Cool!

@WardBrian WardBrian requested a review from nhuurre October 16, 2025 15:31
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 16, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.27%. Comparing base (1f48cd3) to head (db6213a).
⚠️ Report is 19 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1562   +/-   ##
=======================================
  Coverage   90.27%   90.27%           
=======================================
  Files          65       65           
  Lines        9981     9988    +7     
=======================================
+ Hits         9010     9017    +7     
  Misses        971      971
Files with missing lines Coverage Δ
src/frontend/Pretty_printing.ml 92.85% <100.00%> (+0.17%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nhuurre
nhuurre reviewed Oct 16, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@nhuurre nhuurre left a comment

Choose a reason for hiding this comment

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

I'm not sure, I might like aligning with the function name, like this:

  array[] int vote_countvote_countvote_countvote_count(
                array[] int rating,
                array[] int item,
                array[] int rater,
                int I,
                int J
              ) {
    int N = size(rating);
    ...

@WardBrian
Copy link
Copy Markdown
Member Author

WardBrian commented Oct 16, 2025

@nhuurre the formatting is now as you suggest, and the code is much cleaner owing to your suggestions

Things I'm still not very happy with:

  • long (but not extremely long) return types sometimes look weird
  • the comment situation noted above. I thing we could get away with inserting some custom logic into the pp_args rather than using the pp_list_of helper we use elsewhere?

nhuurre
nhuurre reviewed Oct 16, 2025
View reviewed changes
nhuurre
nhuurre reviewed Oct 16, 2025
View reviewed changes
nhuurre
nhuurre approved these changes Oct 16, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@nhuurre nhuurre left a comment

Choose a reason for hiding this comment

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

This is good enough for the original goal, if you want to tweak tricky comment placement you can open a new pull req.

@WardBrian WardBrian merged commit a09b85c into master Oct 17, 2025
3 checks passed
@WardBrian WardBrian deleted the pretty-printing-fundef-tweaks branch October 17, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nhuurre nhuurre nhuurre approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@WardBrian @nsiccha @nhuurre