Skip to content

Commit

Permalink
fix(Diagnostic): use Format.pp_infinity in string_of_text for OCaml 5…
Browse files Browse the repository at this point in the history
….2 (#149)

Co-authored-by: favonia <[email protected]>
  • Loading branch information
kit-ty-kate and favonia authored Feb 23, 2024
1 parent 5dae6c3 commit b835fac
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 11 deletions.
5 changes: 2 additions & 3 deletions .github/workflows/ocaml.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ jobs:
strategy:
matrix:
include:
- ocaml-compiler: "ocaml-base-compiler.5.0.0"
- ocaml-compiler: "ocaml-base-compiler.5.1.1"
with-doc: true
- ocaml-compiler: "ocaml-base-compiler.5.2.0~alpha1"
with-doc: false
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion asai-examples.opam
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ homepage: "https://github.com/RedPRL/asai"
bug-reports: "https://github.com/RedPRL/asai/issues"
depends: [
"dune" {>= "3.1"}
"ocaml" {>= "5.0"}
"ocaml" {>= "5.2"}
"algaeff" {>= "2.0"}
"bwd" {>= "2.2"}
"menhir" {>= "20220210"}
Expand Down
2 changes: 1 addition & 1 deletion asai-lsp.opam
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ homepage: "https://github.com/RedPRL/asai"
bug-reports: "https://github.com/RedPRL/asai/issues"
depends: [
"dune" {>= "3.1"}
"ocaml" {>= "5.0"}
"ocaml" {>= "5.2"}
"bwd" {>= "2.2"}
"eio" {>= "0.12"}
"eio_main" {>= "0.12"}
Expand Down
2 changes: 1 addition & 1 deletion asai.opam
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ homepage: "https://github.com/RedPRL/asai"
bug-reports: "https://github.com/RedPRL/asai/issues"
depends: [
"dune" {>= "3.1"}
"ocaml" {>= "5.0"}
"ocaml" {>= "5.2"}
"algaeff" {>= "2.0"}
"bwd" {>= "2.2"}
"alcotest" {with-test & >= "1.5"}
Expand Down
6 changes: 3 additions & 3 deletions dune-project
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
(synopsis "A library for constructing and printing compiler diagnostics")
(description "This package offers an implementation of compiler diagnostics and supports multiple handlers for displaying diagnostics to the end user. Currently, the package comes with built-in handlers for GitHub Actions workflow commands and UNIX terminals.")
(depends
(ocaml (>= 5.0))
(ocaml (>= 5.2))
(algaeff (>= 2.0))
(bwd (>= 2.2))
(alcotest (and :with-test (>= 1.5)))
Expand All @@ -27,7 +27,7 @@
(synopsis "LSP handler for the package asai")
(description "This package offers an language server protocol (LSP) handler that can turn an application using the asai package into a LSP server. This is an experimental prototype and very likely its design will significantly change in future versions.")
(depends
(ocaml (>= 5.0))
(ocaml (>= 5.2))
(bwd (>= 2.2))
(eio (>= 0.12))
(eio_main (>= 0.12))
Expand All @@ -39,7 +39,7 @@
(synopsis "Examples of the package asai")
(description "This package offers some examples of the asai package. They are organized into a separate package to reduce the dependencies of the main package.")
(depends
(ocaml (>= 5.0))
(ocaml (>= 5.2))
(algaeff (>= 2.0))
(bwd (>= 2.2))
(menhir (>= 20220210))
Expand Down
2 changes: 1 addition & 1 deletion src/Diagnostic.ml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ let map f d = {d with message = f d.message}
let string_of_text text : string =
let buf = Buffer.create 20 in
let fmt = Format.formatter_of_buffer buf in
let () = Format.pp_set_geometry fmt ~max_indent:2 ~margin:Int.max_int in
let () = Format.pp_set_geometry fmt ~max_indent:2 ~margin:(Format.pp_infinity-1) in

This comment has been minimized.

Copy link
@Octachron

Octachron Feb 26, 2024

There is something strange going on: with max_indent:2, every new box will insert a new line. Is that the intended behavior?

This comment has been minimized.

Copy link
@favonia

favonia Feb 26, 2024

Author Contributor

@Octachron Kind of. All newlines will be removed later (by the code 3 line below), so technically speaking it's just inserting a space. On the other hand, I agree that maybe we can or should set it to some larger value to avoid even the space? In any case, I found the implementation of this function super ugly and any refactoring suggestion would be appreciated. 😅

This comment has been minimized.

Copy link
@Octachron

Octachron Feb 27, 2024

Fixing the geometry, with max_indent 2, will have weird effect on the indentation. A better path might be to build your own formatter with pp_set_out_functions in order to not add newlines or indentation to the buffer at all, rather than removing them at posteriori.

This comment has been minimized.

Copy link
@favonia

favonia Feb 27, 2024

Author Contributor

@Octachron But what should be the value of max_indent, then? Is Format.pp_infinity - 2 recommended?

This comment has been minimized.

Copy link
@Octachron

Octachron Feb 27, 2024

pp_infinity - 2 is probably the best since it means that then the formatting engine will not trigger the newline code path in presence of conditional printing.

text fmt;
Format.pp_print_flush fmt ();
Str.global_replace (Str.regexp "\\([\r\n]+ *\\)+") " " @@
Expand Down
2 changes: 1 addition & 1 deletion src/Explicator.ml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ module Make (Tag : Tag) = struct
let source = SourceReader.load ploc.source in
let eof = SourceReader.length source in
let find_eol i = UserContent.find_eol ~line_breaks (SourceReader.unsafe_get source) (i, eof) in
let[@tailcall] rec go state : (Tag.t option * Range.position) list -> _ =
let rec go state : (Tag.t option * Range.position) list -> _ =
function
| (ptag, ploc) :: ps when state.cursor.line_num = ploc.line_num ->
if ploc.offset > eof then invalid_arg "Asai.Explicator.explicate: beyond eof; use the debug mode";
Expand Down

0 comments on commit b835fac

Please sign in to comment.