Commit Graph

421 Commits (75b966c5772fc0ffce2834de9cdca5dbdc9f2095)

Author SHA1 Message Date
Mike Gerwitz 75b966c577 tamer: diagnose: Additional documentation
I had waited to provide more documentation until I was sure that the
abstraction was not going to change significantly; there was a lot of
refactoring in prior commits.

DEV-12151
2022-05-02 09:44:53 -04:00
Mike Gerwitz fc1dad8483 tamer: diagnose::report::Section: Further refactor resolved constructor
This speaks for itself.

DEV-12151
2022-04-29 15:54:38 -04:00
Mike Gerwitz ba0ceddd2d tamer: diagnose::report::Section: Constructor refactoring
This moves construction out of `From` and into separate associated
functions, which can be further simplified in a bit.

We also need unit tests for this, since this still relies on integration
tests due to the cost of the aggressive and tight refactoring iterations.

DEV-12151
2022-04-29 13:10:04 -04:00
Mike Gerwitz 3e04217741 tamer: diagnose::report::Section::maybe_squash_into: Remove syslabel TODO
Previously, when adjacent duplicate spans were both resolved, if one failed,
the other certainly would, which would result in duplicate labels each
squash.  Elided spans do not have syslabels, and so this is no longer a
concern.

DEV-12151
2022-04-29 13:07:51 -04:00
Mike Gerwitz 2ae6df38e7 tamer: diagnose::report: Restore source line preview for invalid UTF-8
This was removed in a previous commit while working on simplifying the
implementation, with the hope of returning to it once things were in a
better place.  They are, so let's bring it back.

DEV-12151
2022-04-29 12:41:56 -04:00
Mike Gerwitz f8dda12fae tamer: diagnose::report: Remove TODOs that are no longer applicable
These relate to the most recent commits.

DEV-12151
2022-04-29 12:34:48 -04:00
Mike Gerwitz 2ce0dbdd84 tamer: diagnose::report::SpanLabel: Remove in favor of separate Level and Label
`SpanLabel` was created during a very early refactoring of this system, and
I've just been fighting with it sense.  This removes it, and simplifies
some things in the process.

It also makes clear that `Level` is never optional and removes the awkward
`Level::default` that was there previously; the default is now the lowest
level, which will always be able to be escalated.

DEV-12151
2022-04-29 12:13:11 -04:00
Mike Gerwitz 9a5a2c4f3f tamer: diagnose::report: Avoid re-resolving adjacent identical spans
This does what the original proof-of-concept implementation did---skip a
span that was just processed, since it'll be squashed into the previous
anyway.  These duplicate spans originate from the diagnostic system when
producing supplemental help information.

DEV-12151
2022-04-29 11:57:50 -04:00
Mike Gerwitz a533244473 tamer: diagnose::report::VisualReporter::render: Avoid mspan collection
This used to be necessary when `Report` stored references to heap-allocated
strings, but `Report` now owns those values itself.

DEV-12151
2022-04-29 09:53:22 -04:00
Mike Gerwitz b0a5265ad3 tamer: diagnose::report::test: Extract into separate file
Tests are large and will be getting larger.  The source will also grow as
it's better documented and cleaned up.  It's getting more difficult to
navigate efficiently and concurrently modify implementation and tests, and
parsing via LSP is getting slower with certain types of changes.

DEV-12151
2022-04-29 09:23:06 -04:00
Mike Gerwitz 5c0e224d3c tamer: diagnose::report: Line numbers in gutter
Alright, starting to settle on an abstraction now, and things are coming
together.  This gives us line numbers in the previously-empty gutter, and
widens the gutter to accommodate.  Gutters are normalized across
sections.  Sections are not yet collapsed for sequential line numbers in the
same context.

Exciting!

Here's an example, on an xmlo file:

error: expected closing tag for `preproc:symtable`
     --> /home/.../foo.xmlo:16:4
      |
   16 |    <preproc:symtable xmlns:map="http://www.w3.org/2005/xpath-functions/map">
      |    ----------------- note: element `preproc:symtable` is opened here

     --> /home/.../foo.xmlo:11326:4
      |
11326 |    </preproc:wrong>
      |    ^^^^^^^^^^^^^^^^ error: expected `</preproc:symtable>`

DEV-12151
2022-04-28 23:53:38 -04:00
Mike Gerwitz 5744e08984 tamer: diagnostic::report: Hoist gutter output into Section
The `Section` itself is now responsible for outputting the gutter, which
puts us in a position to be able to apply consistent formatting without
having to propagate width data to every line variant.
2022-04-28 22:59:13 -04:00
Mike Gerwitz 4e03a367a5 tamer: diagnose::report::SourceLine: Separate variants for each line
Now `SourceLine` _does_ actually correspond to a line of output, which will
allow for better formatting (e.g. collapsing padding) and, importantly,
proper management of gutters.

Note that the seemingly unnecessary `SectionSourceLine` allows for a subtle
consistent formatting for all variants' gutters in `SectionLine`, which will
allow us to hoist that rendering out in the next commit.  The other option
was to include a trailing space for padding and marks, but that is not only
sloppy and undesirable, but asking for confusion, especially in editors (like
mine) that trim trailing whitespace.

DEV-12151
2022-04-28 22:49:35 -04:00
Mike Gerwitz fd1c6430a8 tamer: diagnose::report::SectionSourceLine: {Option<Column>=>Column}
If a column isn't present, it degrades to displaying labels like footnotes
anyway, so this simplifies the system rather than catering to a rare
case.  With that said, this does lose functionality, since it does not
render the source line at all, even though we _could_ do so.

I may re-introduce that rendering after some further refactoring,
specifically for gutters.

DEV-12151
2022-04-28 22:23:58 -04:00
Mike Gerwitz 3a5dcfc016 tamer: diagnose::resolver::SourceLine: {Vec<u8>=>String}
Using a byte vector just makes life more difficult with regard to preparing
the diagnostic reports.  We're already validating UTF-8 data for column
generation, which is necessary for a robust report, so let's just store it
as a String to begin with.

DEV-12151
2022-04-28 22:03:37 -04:00
Mike Gerwitz 838db689ad tamer: diagnose::report: Render labels on mark line
Note that, if a span is first encountered with a mark but with _no_ label,
the first label (if collapsed) will be on the next line.  This allows a span
to be marked without extra visual noise if it's not necessary, and to be
able to trust that it'll stay that way.

Until coloring is introduced, this may or may not be easier to read
depending on context.

This is also not yet taking into account where on the line it begins, and so
may render poorly if the span is at the end of a line.  That will be fixed
later on.

DEV-12151
2022-04-28 16:23:13 -04:00
Mike Gerwitz a197267a2d tamer: xir::flat: Remove closing tag name from label
This is now visible in the diagnostic output.  Example at this point in
time, on an xmlo file for one of our smallest systems:

error: expected closing tag for `preproc:symtable`
  --> /home/.../foo.xmlo:16:4
   |
   |    <preproc:symtable xmlns:map="http://www.w3.org/2005/xpath-functions/map">
   |    -----------------
   = note: element `preproc:symtable` is opened here

  --> /home/.../foo.xmlo:11326:4
   |
   |    </preproc:wrong>
   |    ^^^^^^^^^^^^^^^^
   = error: expected `</preproc:symtable>`

DEV-12151
2022-04-28 15:47:34 -04:00
Mike Gerwitz 33baca113a tamer: diagnose::report: Vary mark character depending on level
Looking more and more Rust-like.  Shameless copy.

TBH I forget what character it uses for help, but it's easy enough to
change.

Also, to be clear: this is modeled after Rust, but it's not a requirement of
mine that it look exactly like it.  I just like the general style; I'll
surely deviate over time, as appropriate (or as I feel like it).

DEV-12151
2022-04-28 15:44:50 -04:00
Mike Gerwitz 8119d1ca0d tamer: diagnose::report: Render span marks under lines
This has the effect of highlighting the columns of the source lines using
'^' as an underline.

The next step will be to have the underline character depend on the
`Level`.

If this commit message doesn't sound all that exciting, given what it
finally achieved after all this time, it's because I'm exhausted, and my
prototype has already taken my excitement.  But this is significant, given
all the work leading up to it.

There is some code cleanup needed and some unit tests that ought to be
written rather than relying on integration, but considering how much this is
being refactored, I don't want to add to that refactoring cost just yet
before gutters are introduced and I know things are settled for now.

DEV-12151
2022-04-28 15:44:49 -04:00
Mike Gerwitz 5db026ed76 tamer: diagnose::report: Initial display of source lines
This has been a lot of refactoring for something that I prototyped a week
ago, and the prototype is still further along in its output formatting (it
has line numbering in gutters and span markings).

But, this has come a long way, and I'm happy with it overall, though I'm not
happy with my slow pace and struggle to maintain focus.  But those are
personal issues.

This leaves a lot to be desired, but at the same time is still really
helpful.  There's a couple notable TODOs regarding pointless allocation and
UTF8 re-checking, but otherwise, the feature-related steps are:

  - Gutters with line numbers; and
  - Marking columns associated with the span.

DEV-12151
2022-04-28 14:33:08 -04:00
Mike Gerwitz 3e06c9aaf3 tamer: diagnose::report: Prepare Section for output of source lines
This lowers the resolved span data into `Section` for display.  The next
step is to actually output it.

DEV-12151
2022-04-28 13:34:05 -04:00
Mike Gerwitz 331aada2bd tamer: diagnose::report::MaybeResolvedSpan: Move up in file
Just rearranging, since this was awkwardly placed relative to where it's
used.

DEV-12151
2022-04-28 11:00:36 -04:00
Mike Gerwitz 6a5a29c2f5 tamer: diagnose::report: Remove Section variants and eagerly squash
Rather than squashing as a separate operation, and explicitly denoting when
it occurred, we'll just always squash, as was done before these changes.  It
doesn't really make sense to make this optional and there's not any value in
keeping the decision around.

This also sets us up favorably for future changes: it creates a vector of
labels, which can be analyzed later to determine how to best lay out marks
and labels.

DEV-12151
2022-04-28 10:30:04 -04:00
Mike Gerwitz c8d919d0cc tamer: diagnose::report: {'l=>'d}
Just renames the lifetime to refer to the `Diagnostic`, rather than a
`Label` returned by it, which was all `'l` was previously used for.

Note that many labels have a `'static` lifetime; this doesn't change that or
somehow cause it to reallocate; the label must life _for at least `'d`_.

DEV-12151
2022-04-27 15:20:16 -04:00
Mike Gerwitz e2c68c5e84 tamer: diagnose::report: Avoid message copy
Rather than rendering the diagnostic `Display` message to a string only to
copy it to yet another buffer later on, this simply stores a reference to
the `Diagnostic` that was provided.  This also adds a type to the `Report`
associating it with the provided `Diagnostic`, which does seem appropriate,
given that the report was produced for it.

I should probably rename '{l=>d} now.

DEV-12151
2022-04-27 15:20:14 -04:00
Mike Gerwitz 3dbab881da tamer: diagnose::report: Produce Report object
Rather than writing to the provided `Write` object, this produces a `Report`
object.  While a lifetime still exists for the diagnostic data (labels,
specifically), I was able to remove the other lifetime resulting from
`ResolvedSpan` by transferring ownership of the data to the `Report`
itself.  Once actual source lines are integrated shortly, `Report` will
include those as well.

This has been a tedious process, but it's coming together.  Hopefully these
commits documenting the progressive and ugly refactoring are found useful by
some reader in the future.

DEV-12151
2022-04-27 15:00:30 -04:00
Mike Gerwitz 3679ff590c tamer: diagnose::report: Remove `L` type parameter
The line number was getting special treatment that is simply not worth the
cost (with regards to how burdensome it is on the type definitions).  This
simplifies things quite a bit.

If we want header customization in the future, we can worry about that in a
different way, or allow the header as a whole to be swapped out, rather than
its constituents.

DEV-12151
2022-04-27 14:23:58 -04:00
Mike Gerwitz 589f5e8c58 tamer: diagnose::report::HeadingLineNum: Compose HeadingColNum
`HeadingColNum` is no longer constructed by `HeadingLineNum`.  This both
narrows the types and required data (e.g. removing dummy values in test
cases), and reduces the coupling (by favoring composition, but still coupled
with the concrete type).

DEV-12151
2022-04-27 11:43:46 -04:00
Mike Gerwitz 7dbe25be05 tamer: diagnose::report::HeadingLineNum: Lower MaybeResolvedSpan
Same as the previous commit with `HeadingColNum`---this removes the
dependency on `MaybeResolvedSpan`.

DEV-12151
2022-04-27 11:28:17 -04:00
Mike Gerwitz 68f9f4d241 tamer: diagnose::report::HeadingColNum: Lower MaybeResolvedSpan
This eliminates `MaybeResolvedSpan` from `HeadingColNum`, along with its
type parameters and lifetimes.

DEV-121251
2022-04-27 11:10:16 -04:00
Mike Gerwitz f29918b5a0 tamer: diagnose::report: Continue refactoring into report components
I'm unhappy with the current state of this, which is why I haven't settled
on docs or unit tests for these changes yet (though note that the
integration tests do cover these changes)---this is still a prototype
refactoring.

In particular, this needs to do more lowering---the `ResolvedSpan` and
`MaybeResolvedSpan` need to be eliminated and lowered into exactly what is
needed so that we can stop reasoning about them and propagating them.

Further, having lines and columns lazily evaluate themselves for
display---based on `MaybeResolvedSpan`---adds extra generics that shouldn't
be necessary; they should be pre-computed and store the concrete data they
need in variants.  Display shouldn't involve computation beyond formatting
of pre-computed data.

That was always the plan, but this refactoring has been incremental.

Anyway: this is in a working and integration-tested state, but it's going to
change.

DEV-12151
2022-04-27 10:48:41 -04:00
Mike Gerwitz e2f9d71c1f tamer: diagnose::report: Refined report components
This generalizes the types a bit more and introduces unit tests.  Note that
these are still also covered by integration tests.

The next step will be to finish generalizing
`<VisualReporter as Reporter>::render`, after which I'll get back to the
task of outputting the source line along with markings and labels.

DEV-12151
2022-04-26 13:26:52 -04:00
Mike Gerwitz d05bcaab03 tamer: {Resolved,Span}::{ctx=>context}: Rename
This is just to provide clarity.  `ctx` is not so widely used that we
benefit from such a short identifier, and it's not worth the cognitive
burden of people unfamiliar with what it may mean.

DEV-12151
2022-04-26 10:52:32 -04:00
Mike Gerwitz 16d76b95d0 tamer: diagnose::resolver::ResolvedSpanData: New trait
This provides the methods originally implemented on `ResolvedSpan` itself,
which will allow for mocking for unit testing.

DEV-12151
2022-04-26 10:46:47 -04:00
Mike Gerwitz 0928427116 tamer: diagnose::resolver::Column::At: Remove
This is redundant with the `Endpoints` variant, although it did read
better.  It's just another case to have to handle.

I was originally going to use `std::ops::RangeInclusive` for `Endpoints`,
however that struct also contains an extra bool indicating whether it was
exhausted (as an iterator), which isn't appropriate for this.

DEV-12151
2022-04-26 10:30:07 -04:00
Mike Gerwitz ec93488365 tamer: diagnost::resolver::ResolvedSpan: Clear methods for all data
This (a) makes it clear the intent of these methods and (b) will allow
introducing a trait for mocking it.

DEV-12151
2022-04-26 10:22:31 -04:00
Mike Gerwitz b9ff7770aa tamer: diagnose::report: Begin refactoring into Display impls
This logic is still covered by the integration tests; I'll be adding unit
tests once it's decoupled to the point where that's possible, which should
be shortly, and after I make sure this is the route I do want to go down.

DEV-12151
2022-04-26 10:14:51 -04:00
Mike Gerwitz c0ace258f0 tamer: diagnose::resolver::SourceLine:: Guarantee non-empty lines
This simplifies types and error handling since we will always have at least
one line, provided that the span is within the range of the context.  To
ensure that, this patch introduces a new error.

DEV-12151
2022-04-22 16:50:16 -04:00
Mike Gerwitz 56b8aec9b7 tamer: diagnose::resolver::test: Extract into own file
There's just a lot here.

DEV-12151
2022-04-22 15:31:12 -04:00
Mike Gerwitz 2e0925627e tamer: diagnose::Label: Introduce lifetime and inner Cow
I did not initially introduce lifetimes because I wasn't sure how the system
was going to evolve, but now lifetimes are going to be needed in a number of
contexts.  The core of TAMER is able to avoid lifetimes in most instances
because of its internment system, but its use is not appropriate for the
diagnostic system's buffers (beyond sourcing strings from already-interned
data).

DEV-12151
2022-04-22 13:23:53 -04:00
Mike Gerwitz aeff7aeed3 tamer: diagnose::test: Extract into own file
This is going to get quite large over time.

DEV-12151
2022-04-22 09:21:18 -04:00
Mike Gerwitz 596c9de85e tamer: diagnose::resolver::SourceLine (line=>num): Rename
`line.line` was rather confounding.

DEV-12151
2022-04-21 15:47:15 -04:00
Mike Gerwitz 5b1f0ab6c6 tamer: diagnostic: Column resolution
Determining the column number is not as simple as performing byte
arithmetic, because certain characters have different widths.  Even if we
only accepted ASCII, control characters aren't visible to the user.

This uses the unicode-width crate as an alternative to POSIX wcwidth, to
determine (hopefully) the number of fixed-width cells that a unicode
character will take up on a terminal.  For example, control characters are
zero-width, while an emoji is likely double-width.  See test cases for more
information on that.

There is also the unicode-segmentation crate, which can handle extended
grapheme clusters and such, but (a) we'll be outputting the line to the
terminal and (b) there's no guarantee that the user's editor displays
grapheme clusters as a single column.  LSP measures in UTF-16,
apparently.  I use both Emacs and Vim from a terminal, so unicode-width
applies to me.  There's too much variation to try to solve that right now.

The columns can be considered a visual span---this gives us enough
information to draw line annotations, which will happen soon.

Here are some useful links:

  - https://hsivonen.fi/string-length/
  - https://unicode.org/reports/tr29/
  - https://github.com/rust-analyzer/rowan/issues/17
  - https://www.reddit.com/r/rust/comments/gpw2ra/how_is_the_rust_compiler_able_to_tell_the_visible/

DEV-10935
2022-04-21 14:27:36 -04:00
Mike Gerwitz e555955450 tamer: span::Span::endpoints_saturated: New method
This gets rid of the `Option` and is used in the diagnostic system (next
commit).

DEV-10935
2022-04-21 14:15:25 -04:00
Mike Gerwitz a22e8e79f7 tamer: diagnose: Integrate resolver for source lines
This does not yet resolve columns, and omits the length of the span, but
it's starting to come together.

This is particularly exciting for me to see because I've been wanting line
numbers in TAME error messages for over a decade.

DEV-10935
2022-04-21 12:34:17 -04:00
Mike Gerwitz 9b4c84de26 tamer: diagnose::resolver: Support rewinding
This does adds support for rewinding the underlying buffer when necessary to
read a span that occurs earlier within the same context (which could also
include the same span read twice).

As part of this change, I cleaned up the code a bit.  Working with this
system can be confusing with the different meanings of the byte offsets and
the different ways of interpreting lines relative to the span that is
provided.  There's not a lot of code here, but it represents a lot of work
to get right.
2022-04-21 12:33:27 -04:00
Mike Gerwitz 1b02e77537 tamer: span (SpanOffsetSize, SpanLenSize): New type aliases
Callers can use these types instead of having to reference globals.

DEV-10935
2022-04-20 09:42:13 -04:00
Mike Gerwitz ab48d79e1f tamer: diagnost::resolver: Initial concept for line resolution
This works, but it's ugly and requires some cleanup.  It shows that there
are some interesting considerations when determining how to best represent
the location of spans to the user in a way that is intuitive.

This is not yet integrated with the reporter, which will require a layer to
load a `Context` from disk.

DEV-10935
2022-04-20 09:42:13 -04:00
Mike Gerwitz a77eb7d937 tamer: span: Minor test refactoring
Just some cleanup based on some new conventions, now that I'm about to make
some changes.

DEV-10935
2022-04-20 09:42:12 -04:00
Mike Gerwitz 725dc3fb54 tamer: tamec: Use diagnostic system for errors
This is a POC, minimal-effort integration that also creates the TamecError
sum type analogous to TameldError.

I'll work on reducing the boilerplate in the future.

A note regarding the type and boilerplate vs. dynamic dispatch, for any
future readers: the purpose of this is to be explicit about the error types
so that the system is self-documenting and it forces and understanding of
its error conditions.  `Box<dyn Error>` is basically "eh idk anything can
happen!", which is not what I'm interested in having.

DEV-10935
2022-04-20 09:42:11 -04:00