icerpc / slicec

The Slice compiler library
Apache License 2.0
13 stars 5 forks source link

Notes aren't Aligned Correctly in Console Output #626

Closed InsertCreativityHere closed 1 year ago

InsertCreativityHere commented 1 year ago

When we emit a diagnostic that has both a snippet and a note we pad the snippet so that the line numbers fit perfectly in front of them (depends on the number of digits in the line number), but we always pad notes with 4 spaces.

We should pad the note by the same amount we do the snippet so they are visually aligned.

Example where they're not aligned:

warning [IncorrectDocComment]: comment has a 'param' tag for 'x', but its operation has no parameter with that name
 --> string-0:5:17
  |
5 |             /// @param x: this is an x
  |                 ----------------------
  |
    = note: operation 'op1' has no parameter named 'x'

The note is floating off to the right

Or in the opposite extreme:

warning [IncorrectDocComment]: comment has a 'param' tag for 'x', but its operation has no parameter with that name
 --> string-0:5234598702345986983745:17
                       |
5234598702345986983745 |             /// @param x: this is an x
                       |                 ----------------------
                       |
    = note: operation 'op1' has no parameter named 'x'

It weirdly hangs to the left.

InsertCreativityHere commented 1 year ago

I think that either the = should be aligned with the | characters above, or the note shouldn't be indented at all. Of the two, I'm starting to lean towards the 2nd.

The first approach really only works well if: 1) There a snippet attached to the message (otherwise we don't print any |) 2) There is no snippet attached to the note (otherwise the indentation just looks weird)

So maybe it's better if note is the same as error and warning. We don't put any characters like = in front of it, and we don't indent it.

InsertCreativityHere commented 1 year ago

Note that Rust uses both. When the above conditions are true, they align the note to the vertical bars:

warning: unused variable: `y`
   --> src/compilation_state.rs:127:17
    |
127 |             let y = &x;
    |                 ^ help: if this is intentional, prefix it with an underscore: `_y`
    |
    = note: `#[warn(unused_variables)]` on by default

Other times it will put it on it's line, with equal footing to errors/warnings:

error: Undefined Behavior: trying to retag from <335603> for SharedReadOnly permission at alloc135820[0x0]
     --> /Users/austin/temp-temp-temp/icerpc/src/utils/ptr_util.rs:116:18
      |
116   |         unsafe { &*self.data.unwrap() }
      |                  ^^^^^^^^^^^^^^^^^^^^
      |                  |
      |                  trying to retag from <335603> for SharedReadOnly permission at alloc135820[0x0], but that tag does not exist in the borrow stack for this location
      |
      = note: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
note: <335603> was created by a SharedReadOnly retag at offsets [0x0..0xa0]
     --> /Users/austin/temp-temp-temp/icerpc/src/utils/ptr_util.rs:50:24
      |
50    |             data: Some(&*self.data),
      |