Skip to content

error messages: composable formatting for warnings - #13568

Merged
Octachron merged 7 commits into
ocaml:trunkfrom
Octachron:formatted_warnings
Nov 19, 2024
Merged

Octachron merged 7 commits into
ocaml:trunkfrom
Octachron:formatted_warnings

Conversation

@Octachron

Copy link
Copy Markdown
Member

This PR enables composable formatting for warning messages.

Before this PR warning messages were rendered as strings before being sent to the error report printer.
Consequently, warning messages could not really use styling nor formatting hints in a reliable way.

This PR switches warning messages to render to Format_doc.t rather than string (like error messages) to enable both options.

A painful source of complexity for this PR as described in #12965 is that the format used for error message reports

──────────────────
  Location
┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈
  Header │ Main
         │ msg
         │
         │
┈┈┈┈┈┈┈┈┈╧════════
Suberrors
──────────────────

does not work well for warnings because the width of the header for warning is unbounded and often quite large.

To solve this formatting issue, this PR takes the alternative path of implementing a new format for warnings and alerts

───────────────────
  Location
───────────────────
  Header ┊ Main msg
┈┈┈┈┈┈┈┈┈┘
  ┊ 
  ┊
  ┊
  ┊
──╧════════════════
Suberrors
───────────────────

while keeping the old format for error messages. I expect this to be a (temporarily?) work-around to decouple the formatting of warnings from the change of format for all error messages.

@goldfirere goldfirere left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I did not review every error message change, but this looks reasonable overall; I don't think fine-grained review of messages is necessary here, trusting @Octachron to have examined examples of output changes as he implemented this.

Comment thread parsing/location.ml Outdated
Comment thread parsing/location.ml Outdated
Format.pp_close_tbox ()
) ()
in
let new_format self ppf report =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
let new_format self ppf report =
let warning_format self ppf report =

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Renamed, thanks !

@Octachron

Copy link
Copy Markdown
Member Author

I add tweaked few warnings to add more break hints in text (3861f3b), missing spaces (e7b8e7d), and fix strange newlines at the start of some inline code blocks(ea22238).

I am planning to merge soon to avoid conflicts.

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