Skip to content

Commit

Permalink
Add wrapping behaviour to code labels (#116)
Browse files Browse the repository at this point in the history 
Our error reporter had no wrapping behaviour for code labels: they would
happily go as far right as they wanted, often overflowing the assumed
width of the terminal they are being printed on.

This PR changes that behaviour in two ways:
- Labels are no longer forced to be right-positioned to the code span
they refer to. If it helps to keep them on one line, we now position
them to the left or centred (with this order of preference).
- If they would necessarily overflow the maximum allowed width, we
perform text wrapping.

Getting this right was... tricky. But we have a fairly extensive
collection of error messages in our UI tests and the new version seems
to work as expected.
  • Loading branch information
@LukeMathWalker
LukeMathWalker committed Dec 13, 2023
1 parent 18cd144 commit 44cad26
Show file tree
Hide file tree
Showing 44 changed files with 255 additions and 154 deletions.
4 changes: 2 additions & 2 deletions doc_examples/quickstart/05-error.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ ERROR:
│ 13 │ bp.route(GET, "/api/ping", f!(crate::routes::status::ping));
│ 14 │ bp.route(GET, "/api/greet/:name", f!(crate::routes::greet::greet));
│ · ───────────────┬───────────────
│ · ╰── The request handler was registered here
│ · The request handler was registered here ──╯
│ 15 │ bp
│ ╰────
│ ╭─[demo/src/routes/greet.rs:10:1]
│ 10 │
│ 11 │ pub fn greet(params: RouteParams<GreetParams>, user_agent: UserAgent) -> Response {
│ · ────┬────
│ · ╰── I don't know how to construct an instance of this input parameter
│ · I don't know how to construct an instance of this input parameter
│ 12 │ if let UserAgent::Unknown = user_agent {
│ ╰────
│ help: Register a constructor for `demo::user_agent::UserAgent`
Expand Down
3 changes: 2 additions & 1 deletion libs/Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 9 additions & 9 deletions ...x_cli/tests/ui_tests/blueprint/common/cannot_return_the_unit_type/expectations/stderr.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
│ 44 │ let mut bp = Blueprint::new();
│ 45 │ bp.constructor(f!(crate::constructor), Lifecycle::Singleton);
│ ·  ───────────┬──────────
│ · ╰── The constructor was registered here
│ · ╰── The constructor was registered here
│ 46 │ bp.constructor(
│ ╰────

Expand All @@ -19,7 +19,7 @@
│ 46 │ bp.constructor(
│ 47 │ f!(crate::fallible_constructor_building_unit),
│ ·  ──────────────────────┬──────────────────────
│ · ╰── The constructor was registered here
│ · ╰── The constructor was registered here
│ 48 │ Lifecycle::RequestScoped,
│ ╰────

Expand All @@ -33,7 +33,7 @@
│ 56 │ bp.route(GET, "/home", f!(crate::handler));
│ 57 │ bp.route(GET, "/unit", f!(crate::unit_handler));
│ ·  ───────────┬───────────
│ · ╰── The request handler was registered here
│ · The request handler was registered here
│ 58 │ bp.route(GET, "/fallible_unit", f!(crate::fallible_unit_handler))
│ ╰────

Expand All @@ -47,7 +47,7 @@
│ 57 │ bp.route(GET, "/unit", f!(crate::unit_handler));
│ 58 │ bp.route(GET, "/fallible_unit", f!(crate::fallible_unit_handler))
│ ·  ────────────────┬───────────────
│ · ╰── The request handler was registered here
│ · The request handler was registered here ──╯
│ 59 │ .error_handler(f!(crate::error_handler));
│ ╰────

Expand All @@ -61,7 +61,7 @@
│ 52 │
│ 53 │ bp.wrap(f!(crate::unit_wrapping_middleware));
│ ·  ─────────────────┬─────────────────
│ · ╰── The wrapping middleware was registered here
│ · The wrapping middleware was registered here
│ 54 │ bp.wrap(f!(crate::fallible_wrapping_middleware))
│ ╰────

Expand All @@ -75,7 +75,7 @@
│ 53 │ bp.wrap(f!(crate::unit_wrapping_middleware));
│ 54 │ bp.wrap(f!(crate::fallible_wrapping_middleware))
│ ·  ───────────────────┬───────────────────
│ · ╰── The wrapping middleware was registered here
│ · The wrapping middleware was registered here
│ 55 │ .error_handler(f!(crate::error_handler));
│ ╰────

Expand All @@ -89,7 +89,7 @@
│ 54 │ bp.wrap(f!(crate::fallible_wrapping_middleware))
│ 55 │ .error_handler(f!(crate::error_handler));
│ ·  ────────────┬───────────
│ · ╰── The error handler was registered here
│ · The error handler was registered here
│ 56 │ bp.route(GET, "/home", f!(crate::handler));
│ ╰────

Expand All @@ -103,7 +103,7 @@
│ 58 │ bp.route(GET, "/fallible_unit", f!(crate::fallible_unit_handler))
│ 59 │ .error_handler(f!(crate::error_handler));
│ ·  ────────────┬───────────
│ · ╰── The error handler was registered here
│ · The error handler was registered here
│ 60 │ bp
│ ╰────

Expand All @@ -117,6 +117,6 @@
│ 50 │ bp.constructor(f!(crate::fallible_constructor), Lifecycle::RequestScoped)
│ 51 │ .error_handler(f!(crate::error_handler));
│ ·  ────────────┬───────────
│ · ╰── The error handler was registered here
│ · The error handler was registered here
│ 52 │
│ ╰────
10 changes: 5 additions & 5 deletions ...i_tests/blueprint/common/output_type_must_implement_into_response/expectations/stderr.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
│ 37 │ .error_handler(f!(crate::error_handler));
│ 38 │ bp.route(GET, "/home", f!(crate::handler))
│ ·  ─────────┬────────
│ · ╰── The request handler was registered here
│ · The request handler was registered here
│ 39 │ .error_handler(f!(crate::error_handler));
│ ╰────
│  help: Implement `pavex::response::IntoResponse` for
Expand All @@ -22,7 +22,7 @@
│ 33 │ let mut bp = Blueprint::new();
│ 34 │ bp.wrap(f!(crate::wrapping_middleware))
│ ·  ───────────────┬──────────────
│ · ╰── The wrapping middleware was registered here
│ · ╰── The wrapping middleware was registered here
│ 35 │ .error_handler(f!(crate::error_handler));
│ ╰────
│  help: Implement `pavex::response::IntoResponse` for
Expand All @@ -37,7 +37,7 @@
│ 34 │ bp.wrap(f!(crate::wrapping_middleware))
│ 35 │ .error_handler(f!(crate::error_handler));
│ ·  ────────────┬───────────
│ · ╰── The error handler was registered here
│ · The error handler was registered here
│ 36 │ bp.constructor(f!(crate::request_scoped), Lifecycle::RequestScoped)
│ ╰────
│  help: Implement `pavex::response::IntoResponse` for
Expand All @@ -52,7 +52,7 @@
│ 38 │ bp.route(GET, "/home", f!(crate::handler))
│ 39 │ .error_handler(f!(crate::error_handler));
│ ·  ────────────┬───────────
│ · ╰── The error handler was registered here
│ · The error handler was registered here
│ 40 │ bp
│ ╰────
│  help: Implement `pavex::response::IntoResponse` for
Expand All @@ -67,7 +67,7 @@
│ 36 │ bp.constructor(f!(crate::request_scoped), Lifecycle::RequestScoped)
│ 37 │ .error_handler(f!(crate::error_handler));
│ ·  ────────────┬───────────
│ · ╰── The error handler was registered here
│ · The error handler was registered here
│ 38 │ bp.route(GET, "/home", f!(crate::handler))
│ ╰────
│  help: Implement `pavex::response::IntoResponse` for
Expand Down
24 changes: 12 additions & 12 deletions ...rint/constructors/constructors_input_parameters_cannot_be_generic/expectations/stderr.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
│ 27 │ let mut bp = Blueprint::new();
│ 28 │ bp.constructor(f!(crate::generic_constructor), Lifecycle::RequestScoped);
│ ·  ───────────────┬──────────────
│ · ╰── The constructor was registered here
│ · ╰── The constructor was registered here
│ 29 │ bp.constructor(
│ ╰────
│ ╭─[src/lib.rs:1:1]
│ 1 │ pub fn generic_constructor<T>(generic_input: GenericType<T>) -> u8 {
│ ·  ┬ ─┬
│ · │ ╰── ..because it is not used here
│ · ╰── I can't infer this..
│ · │ ..because it is not used here ──╯
│ · ╰── I can't infer this..
│ 2 │ todo!()
│ ╰────
│  help: Specify the concrete type(s) for the problematic generic
Expand All @@ -37,15 +37,15 @@
│ 29 │ bp.constructor(
│ 30 │ f!(crate::doubly_generic_constructor),
│ ·  ──────────────────┬──────────────────
│ · ╰── The constructor was registered here
│ · ╰── The constructor was registered here
│ 31 │ Lifecycle::RequestScoped,
│ ╰────
│ ╭─[src/lib.rs:1:1]
│ 1 │ pub fn doubly_generic_constructor<T, S>(i1: GenericType<T>, i2: GenericType<S>) -> u16 {
│ ·  ┬ ┬ ─┬─
│ · │ │ ╰── ..because they are not used here
│ · │ ╰── I can't infer this..
│ · ╰── I can't infer this..
│ · │ │ ..because they are not used here ──╯
│ · │ ╰── I can't infer this..
│ · ╰── I can't infer this..
│ 2 │ todo!()
│ ╰────
│  help: Specify the concrete type(s) for the problematic generic
Expand All @@ -66,22 +66,22 @@
│ 33 │ bp.constructor(
│ 34 │ f!(crate::triply_generic_constructor),
│ ·  ──────────────────┬──────────────────
│ · ╰── The constructor was registered here
│ · ╰── The constructor was registered here
│ 35 │ Lifecycle::RequestScoped,
│ ╰────
│ ╭─[src/lib.rs:1:1]
│ 1 │ pub fn triply_generic_constructor<T, S, U>(
│ ·  ┬ ┬ ┬
│ · │ │ ╰── I can't infer this..
│ · │ ╰── I can't infer this..
│ · ╰── I can't infer this..
│ · │ │ ╰── I can't infer this..
│ · │ ╰── I can't infer this..
│ · ╰── I can't infer this..
│ 2 │ i1: GenericType<T>,
│ ╰────
│ ╭─[src/lib.rs:4:1]
│ 4 │ i3: GenericType<U>,
│ 5 │ ) -> u32 {
│ ·  ─┬─
│ · ╰── ..because they are not used here
│ · ╰── ..because they are not used here
│ 6 │ todo!()
│ ╰────
│  help: Specify the concrete type(s) for the problematic generic
Expand Down
8 changes: 4 additions & 4 deletions ...onstructors/output_type_of_constructors_cannot_be_a_naked_generic/expectations/stderr.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@
│ 23 │ let mut bp = Blueprint::new();
│ 24 │ bp.constructor(f!(crate::naked), Lifecycle::RequestScoped);
│ ·  ────────┬───────
│ · ╰── The constructor was registered here
│ · ╰── The constructor was registered here
│ 25 │ bp.constructor(f!(crate::fallible_naked), Lifecycle::RequestScoped)
│ ╰────
│ ╭─[src/lib.rs:1:1]
│ 1 │ pub fn naked<T>() -> T {
│ ·  ┬
│ · ╰── The invalid output type
│ · ╰── The invalid output type
│ 2 │ todo!()
│ ╰────
│  help: Can you return a concrete type as output?
Expand All @@ -35,13 +35,13 @@
│ 24 │ bp.constructor(f!(crate::naked), Lifecycle::RequestScoped);
│ 25 │ bp.constructor(f!(crate::fallible_naked), Lifecycle::RequestScoped)
│ ·  ────────────┬────────────
│ · ╰── The constructor was registered here
│ · ╰── The constructor was registered here
│ 26 │ .error_handler(f!(crate::error_handler));
│ ╰────
│ ╭─[src/lib.rs:1:1]
│ 1 │ pub fn fallible_naked<T>() -> Result<T, FallibleError> {
│ ·  ────────────┬───────────
│ · ╰── The invalid output type
│ · ╰── The invalid output type
│ 2 │ todo!()
│ ╰────
│  help: Can you return a concrete type as output?
Expand Down
Loading

0 comments on commit 44cad26

Please sign in to comment.