Closed CAD97 closed 7 months ago
cc list of potentially interested parties:
codespan
)language-reporting
)annotate-snippets
)rowan
+codespan
)Hi @CAD97 !
Thanks for taking a look at this and I'm excited to work together, if we end up deciding that we're aiming for the same shape of the API.
I did a first read of your proposal and only have very rough, unsorted initial thoughts to share so far. Most of them are critical, but please, do not read it as a general criticizm - it's just easier to focus on what I see as potentially incompatible. The code generally looks good!
You seem to use dependencies liberally. I'm of strong opinion that this functionality should be treated as a "foundational crate" per raphlinus terminology and as such should aim to maintain minimal, or even zero if possible, dependency tree. Your proposal has 22 dependencies, annotate-snippets
has zero.
I'm open to add some, if we see a value in doing so, but I'd like to keep it at the very minimum and if possible look for dependencies that themselves don't introduce a long chain of dependencies in return.
Your API seems to resemble more imperative approach much closer than what I'm aiming for with annotate-snippets
. The chain operations Diagnostics::build().code().level().primary().secondary()
feels fairly awkward to me I must admit. I've been working at TC39 on JavaScript at the time when that model was very popular (jQuery!) and I'm not very convinced that it leads to a clean API use and highly-maintainable code (I'm not talking about our code, but the code that uses the API).
In principle, I see what I call Snippet
struct as a data structure. Rust doesn't have a very good way to provide complex slash optional parameter list to constructor, but I came to conclusion that likely in this case we don't need it.
So, instead of making people write Snippet::new(title, description, level, code, &[secondary], ...)
or your Snippet::build().title().description().level().code()
, we can just expose ability to do Snippet { title: None, description: Some(...), level: Some("E203"), ... }
.
It's very clean, well maintained, allows for omitting optional fields with Default
trait, and what's most powerful, can be then wrapped in many different ways to construct an instance.
If I'm not mistaken, the only shortcoming of that approach is lack of inter-field validation allowing one to define a snippet with 5 lines, but an annotation on a sixth.
Initially, that led me to try to squeeze one or more constructors, because I dislike ability to produce internally inconsistent data, but eventually I decided that it's not necessary to fix it. In annotate-snippets
model the struct gets passed to a function which generates a DisplayList
our of it. It's on this step that the validation of the input (Snippet
) takes place and can be rejected.
I really like this model for a foundational functionality of a foundational crate. I'm sure it's possible to build different nicer high-level APIs to aid people in constructing Snippet
or Slice
, but I think we should focus on exposing everything we can now and letting others or ourselves add sugar later.
Your model seems much closer to what codespan
and in result language-reporting
are doing. I would prefer us to avoid starting with that API, while I'm open to get to it later.
annotate-snippets
is intentionally very vague about the core concepts in it. It is meant to be a vague API which can be used for displaying errors, but also for tutorials, helpers, explainers etc. In particular I believe that the range of annotations in them can be very vast and I'd love to end up with something flexible and extendible.
For that reason I'd like to minimize the amount of places in our API that we name after some function. Due to the nature of your proposed API you not only do that, but also add API methods like "primary", "secondary", etc. while at the same time making it a bit less visible what is the relation between them, if it's possible to specify multiple or just one (can I specify multiple "secondary()"? I can! But can I do the same to "primary()"? If so, what it means? And can I specify multiple "level()"? Or will the next one overwrite the previous one?), and so on.
For me, the difference between:
let diagnostic = Diagnostic::build()
.primary(
Annotation::build()
.span(50..777)
.message("mismatched types")
.build(),
)
.code("E0308")
.level(Level::Err)
.secondary(
Annotation::build()
.span(55..69)
.message("expected `Option<String>` because of return type")
.build(),
)
.secondary(
Annotation::build()
.span(76..775)
.message("expected enum `std::option::Option`, found ()")
.build(),
)
.build();
and
let snippet = Snippet {
title: Some(Annotation {
id: Some("E0308"),
label: Some("mismatched types"),
annotation_type: AnnotationType::Error,
}),
slices: &[Slice {
source,
line_start: Some(51),
annotations: vec![
SourceAnnotation {
label: "expected `Option<String>` because of return type",
annotation_type: AnnotationType::Warning,
range: (5, 19),
},
SourceAnnotation {
label: "expected enum `std::option::Option`",
annotation_type: AnnotationType::Error,
range: (23, 725),
},
],
}],
};
is that in the latter, the only meaning is assigned to annotations via annotation_type
, and there may be many different ones including custom ones added by the user.
In the former, the title is decided by a primary
and thus cannot be different than an in-source annotation, the level is defined per slice, not per annotation, and we use the concept of primary/secondary
which would only be extensible via some tertiary
?
annotate-snippets
allows you to define title different than any source annotations, or footer, or multiple footers, multiple titles, multiple annotations, which may or may not overlap. It seems like a fairly low-level approach, but in result very flexible.
Your API seems much more constrain and intended for getting just one style of annotation snippets.
You use a lot of Cow
but I'm not sure how valuable it is. I'm not as convinced to that decision, but I think that in all cases I've been able to find, &str
works well, and I'm not sure if we need an owned messages by the annotation/slice/snippet.
I focus a lot on performance in my rewrite of annotate-snippets
in https://github.com/rust-lang/annotate-snippets-rs/pull/12 . I was unable to compile your PR so I can't measure performance but I think it'd be important to compare.
Finally, and I struggle to ask this since it borderlines NIH-bias which I'd like to avoid, I'm wondering why do you feel the need to design a new API. I asked the authors of codespan
and language-reporting
if they see any shortcomings of my crate and they stated that they don't and the plan to converge on annotate-snippets
API seems reasonable unless we find any limitation of it.
Have you encountered a limitation? Do you dislike annotation-snippets
API? Any other reason?
===
I've been on vacation over this week, but I plan to get back and finish https://github.com/rust-lang/annotate-snippets-rs/pull/12 now. If you believe that there's a value in diverging from its API, I'm happy to discuss the above differences (or any other that you see!) and compare the results!
I don't want to be attached to my API but I have not seen or heard of any problem with it yet, and I find it the most flexible, robust and extendible of all I've seen.
In your code I noticed several ideas which I like, but they're internal rather than API surface and I'd rather see them as PRs against annotate-snippets
than a full new API.
Let me know what you think!
Big apologies: I forgot that I had an out-of-date example in the repository when I posted this; I didn't mean to mislead. I've dropped the builder API (if you look at the API overview in the OP, it's not present) and that's why the example won't compile. There were a few other issues as well because I pushed a WIP checkpoint commit. The implementation is still not quite finished for performance testing as I still need to port one final bit first.
So let me address the points some:
lsp-types
is the only heavy dependency, and should be completely opt-in for the LSP target. The PR now correctly marks the dependency as optional. The LSP conversion could also be pulled out-of-tree if really desired, but the diagnostic layout should be LSP-friendly. Of the other three:
termcolor
(+ wincolor
, winapi-util
, winapi
+friends) is I think the best option for an abstracted color-capable sink (
codespan/
language-reportingagree with me here). @brendanzab says the current use of
ansi_term(which also depends on
winapi+friends) is what currently keeps them from going all-in on
annotate-snippets` (being the lack of injectable custom writer and global state).scopeguard
: highly used leaf crate; could be inlined if really desired.bytecount
: I included it because clippy yelled at me not to do a naive byte count for newline characters. It's also a leaf crate. Given expected source sizes, it might be reasonable to drop it. Is only used in impl SpanResolver for &str
.For some reason I'm having trouble installing cargo-tree
so I can't give a better overview currently.
The builder API used in the example was old and discarded; I'm just allowing record creation much closer to annotate-snippets
now:
let diagnostic = Diagnostic {
primary: Annotation {
span: (50, 777),
level: Level::Err,
message: "mismatched types".into(),
},
code: Some("E0308".into()),
secondary: vec![
Annotation {
span: (55, 69),
level: Level::Info,
message: "expected `Option<String>` because of return type".into(),
},
Annotation {
span: (76, 775),
level: Level::Err,
message: "expected enum `std::option::Option`, found ()".into(),
},
] // can also be borrowed, `vec` for simplicity
.into(),
};
Yeah, the API I've proposed as-is is quite targeted at diagnostics and being compatible with the LSP diagnostic API. I'm perfectly happy to generalize it some more, though.
The intent as currently designed is that the primary
annotation is the "short" annotation (i.e. the one that shows up as the red squiggly before asking for more information), and the secondary
annotations are any related information. Note that the secondary
annotations do not need to be within the primary
span or even from the same Span.origin
; that's just a limitation of my current implementation trying to cobble something together to demonstrate the API.
The main reason I've used Cow
is to help the use case where a consumer wants to build a Diagnostic
/Annotation
list up during some analysis. If they're purely borrowed, the user has to implement a similar structure that's owned to build up the list, then borrow it when pushing it to the sink annotate-snippets
renderer. I'd like to avoid that necessity, but I can be talked down if it's a sticking point, especially if we provide an owned variant separate instead of mushing them together with Cow
. (Basically, I'm not using Cow
as copy-on-write but as maybe-owned.)
Should be on par with cleanup
, as the real work was directly ported from it. Again, port is not quite finished, so can't be measured yet.
I'm happy to find a middle ground, this is mainly to share the results of my experimentation so that we can try to find the best end result. The big parts of this I'd really like to see adopted in some form: 1) A LSP target is practical, 2) delayed Span
resolution to support source highlighting, and 3) a generic WriteColor
target rather than baking in ANSI or no color as the only target implicitly.
I didn't read this past the first paragraph (I hope to do so, once I have more time), but I'd advise against using lsp-types as a dependency, even an optional one, It changes way to often, and I don't think it's worth it make it non-breaking. Rather, I think it's better to vendor just diagnostics related bits of the LSP, which should be stable. See, for example how I've hard-coded setup/tear down messages in lsp-server. It seems like the case where depending on a 3rd party lib doesn't actually by that much
Thanks for the response!
I'm just responding to your points, I'll review more tomorrow:
Yes, I can understand why you aim for lsp-types
. Maybe we want it, I'll have to look deeper.
As for termcolor
vs ansi_term
- I'm not strongly opinionated. I can see us switching if termcolor
is better. I want it to be optional tho.
Others - I'd like to all extra
functionality to be optional. I can see an argument for, say, unicode crate for character boundaries count and breaking, but I believe it should be optional because a fundantional crate is likely to be used often without that extra piece.
I saw things like serde
and serde_json
compiled as part of your PR. I think they should not be necessary.
If they're needed by lsp-types
I will question whether we need lsp-types
:)
Oh, I'm sorry for not reviewing the example vs. code. As I said, I just got back from PTO.
As for your example, it looks much better to me! I still would like to separate title from in-source annotations, because it's easier to reference/copy one from another than to separate if you need them to be different.
As with above - it's easier to specialize later, if the crate allows for generic behavior. I'd like to not dictate what behavior happens on the level of our crate, but rather on the level of some higher level API. Then you can have an API that uses the foundational crate and is specific to generating errors and it picks the "primary" and sets it as the only title, and as the primary annotation and so on.
I can be talked up to incorporate Cow. My main concern about it is that I'm torn on whether the API should be constructable. There's one way to think about it that it should. There's another that there should be some higher level that constructs it (maybe over time) and then spawns the Diagnostics
struct.
As I said, I can definitely be talked into the idea of making the API buildable.
One idea against is that I'm trying to minimize allocations and one way to do that was to cut out all Vec
replacing them with &[]
. That would make the code simpler at the cost that when you need to build, you do this prior. My initial position is that in many cases you would be able to avoid that step so there's a tangible win. But maybe I'm wrong here! I'll investigate!
Cool! Let's both finish our PRs and compare! :)
Oh, awesome! I'm so happy to see actual points listed out this way. This is very helpful, thank you!
annotate-snippets
supports syntax highlighting, and if it doesn't, it's a bug and I'm open to look for ways to fix it!cleanup
branch now. I want it to be agnostic of the styling, and able to support different stylesheets
and even different themes
(think - you build different DisplayList for terminal and different for web browser or some other rich GUI, which is different from just styling it). The last step is the last piece of my cleanup, so I'd like to finish my proposal. I like your updated example much more (d'uh! it's closer to annotate-snippets ;)) and I'm really excited to see you bringing your experience and perspective! Let's finish our PRs and compare them and figure out how to approach it. From your response I feel that we're aiming for close enough goal that we should end up with a single crate, which is awesome (the alternative is also awesome, but less attractive for the bus-factor removal which I care about deeply!).
Onwards! :)
Just a few more notes:
annotate-snippets
's more general snippet annotation.&mut dyn WriteColor
sink is probably my most desired part of this proposal.SpanResolver
to paint the span.The rest is a question of what layout decisions should be at what layer.
Sounds good! I'll look into &mut dyn WriteColor
tomorrow, and then into SpanResolver
. Give me several days and I should have something tangible (either code or opinion at least!)
Is the intention that you could impl Span
for types like codemap::Span
and codespan::Span
that do the "index into concatenation of all files" trick? I don't see how they could provide origin()
without reference to codemap::CodeMap
/ codespan::Files
. Alone, these span types can't provide a filename to display, or even test whether two spans refer to the same file for the Eq
bound.
The requirement that a span's start and end are usize
also precludes implementing Span
for types that store positions as a separate line and column number.
What if the interface exposed column numbers for the library to do its layout computations, but kept Span
opaque? Here's a rough sketch:
trait SpanResolver<Sp> {
type Origin: Eq + Display;
type Line: Copy + Eq + Ord + Into<usize>;
fn resolve(&mut self, span: Sp) -> ResolvedSpan<Origin, Line>;
fn write_source(
&mut self,
w: &mut dyn WriteColor,
file: &Origin,
line: Line,
start_col: usize,
end_col: usize,
) -> io::Result<()>;
fn next_line(&mut self, line: Line) -> Option<Line>;
}
struct ResolvedSpan<Origin, Line> {
file: Origin,
start_line: Line,
start_col: usize,
end_line: Line,
end_col: usize,
}
Line
is generic and not usize
because it's O(n) to index a plain string by line number. An implementation of Line
for str
could cache the byte index of the start of line.
@kevinmehall yes, the intent was that it would be possible to implement Span
for types that map multiple files into one dimensional space. I overlooked that resolution to the origin would have to go through the resolver as well for that to work. Of course, now with annotate-snippets
being targeted lower-level than this sketch originally aimed, I don't think a single render should need to deal with spans from multiple origins at all. (That would be the next level up.)
I don't think a single render should need to deal with spans from multiple origins at all.
rustc
has some diagnostics where the primary and secondary spans are in different files. How would that be handled? For example:
error[E0326]: implemented const `X` has an incompatible type for trait
--> src/lib.rs:5:12
|
5 | const X: () = ();
| ^^ expected u32, found ()
|
::: src/f1.rs:2:13
|
2 | const X: u32;
| --- type in trait
|
= note: expected type `u32`
found type `()`
In annotate-snippets
that's handled by https://github.com/rust-lang/annotate-snippets-rs/blob/master/examples/multislice.rs
Let me drop this here while it's fresh on my mind, though I should admit it's unbaked while I need to run off to bed again.
Here's the input structure I'm experimenting with for a much reduced annotate-snippets
responsibility from the OP, now with documentation!:
I suspect adding a "collection of snippets" type (that if I'm not mistaken, is closer to the current Snippet
than mine, which is closer to Slice
) would be desirable on top of this and as the argument for the render function. That "snippet collection" would also hold the front matter. Or we could even just say that we only care about snippet annotation and the caller should handle the other lines.
EDIT: recording two concerns that this API cannot cover (yet?):
- Alignment of notes not attached to annotation with the separator
- Alignment of the separator between annotated slices.
@matklad
I didn't read this past the first paragraph (I hope to do so, once I have more time), but I'd advise against using lsp-types as a dependency, even an optional one, It changes way to often, and I don't think it's worth it make it non-breaking.
Did you see https://github.com/gluon-lang/lsp-types/issues/117 ? Would let lsp-types go to 1.0 at the cost of having a slightly more awkward API (though in a way, a more honest one).
I just want to say I really appreciate the time being put into this! Would be exciting to converge on something nice.
@CAD97 I'm cool with merging language-reporting
into annotate-snippets
in something like this form. How would you feel about merging me into the Github team? I'd love to continue driving this part of the Rust ecosystem forward in direct terms.
@wycats I'm not on any team right now, I just chose this as a pet project for October (as I'm working on a blog post series that would benefit from it) 😛
I've a second potential API over at https://github.com/CAD97/retort/pull/2 that embraces the sink-layer quality of annotate-snippets more fully. (i.e. a diagnostics library could provide a more LSP-shaped API that renders to annotate-snippets or LSP.) It's also got an amount of experimental support for syntect
themed highlighting of source code, but that's all entirely optional (and probably not all that desirable, I was just playing with the idea, and it should be at the next level up).
I'll update the OP with an adjusted RFC proposal of the adjusted API once I have a chance to write some examples against that API, probably tomorrow sometime.
@CAD97 - I pushed an update to my cleanup
branch which:
io::Write
termcolor
as an option (next to term_ansi
, but this can go away)My next steps are to:
At that point, I'd like to consider merging the PR into master and releasing updated annotate-snippets
.
As for your proposal, I'm not sure where is the best place to fit it - should we first align our codebases before any releases, or is it ok to do this on top of cleanup
?
On the high level, except of feature set, I think we need to decide on which of the APIs is better, the one in cleanup
or your proposal.
The example I work with looks like this:
And the APIs are:
cleanup
:
let snippet = Snippet {
title: Some(Annotation {
id: Some("E0308"),
label: Some("mismatched types"),
annotation_type: AnnotationType::Error,
}),
footer: &[],
slices: &[Slice {
source,
line_start: Some(51),
origin: Some("src/format.rs"),
annotations: &[
SourceAnnotation {
label: "expected `Option<String>` because of return type",
annotation_type: AnnotationType::Warning,
range: 5..19,
},
SourceAnnotation {
label: "expected enum `std::option::Option`",
annotation_type: AnnotationType::Error,
range: 23..725,
},
],
}],
};
@CAD97's:
let snippets = &[
Snippet::Title {
message: Message {
text: &"mismatched types",
level: Level::Error,
},
code: Some(&"E0308"),
},
Snippet::AnnotatedSlice {
slice: Slice {
span: 1..721,
origin: Some(Origin { file: &"src/format.rs", pos: Some((4, Some(5))) }),
spacing: Spacing::TightBelow,
fold: false,
},
annotations: &[
Annotation {
span: 5..19,
message: Message {
text: &"expected `Option<String>` because of return type",
level: Level::Warning,
},
},
Annotation {
span: 23..725,
message: Message {
text: &"expected enum `std::option::Option`",
level: Level::Error,
},
},
],
},
];
Performance wise, @CAD97's PR is 12.375 us
on average to produce it, cleanup
is 9.3012 us
on my laptop (~25% faster).
@zbraniecki my impl uses a lot of dyn Trait
even internally whereas you're using impl Trait
in a lot of the same places; for comparison's sake, could you do a comparison with my modified branch?
It's a tradeoff between monomorphization cost and performance cost. On my branch with the same benchmark, I got [10.700 us 11.047 us 11.435 us]
with dyn
everywhere and [9.8312 us 10.078 us 10.368 us]
(change [-12.587% -7.7650% -3.0543%]
) with impl
everywhere (&dyn DebugAndDisplay
specialized to &str
).
Branch with the adjustment: https://github.com/CAD97/retort/tree/take-two-impl
Note that my version has a few features that cleanup
doesn't as well, the main one being the use of &dyn Display
allowing for use of e.g. &format_args!("expected
{}because of return type", expected_ty)
rather than &format!("expected
{}because of return type")
(assuming your code flow allows for that delayed resolution). I'll update OP with more meaninful comparison by Saturday.
Good analysis! I also noticed that just switching from ftm::Write to io::Write cost me 8.0->9.0
As for which of those trades we should take I'm open to discuss it!
My next steps are to:
- Verify that simple ASCII vs. rich-Unicode output works
@zbraniecki Oooh, does that mean we'll be able to use box drawing characters for the underlines/gutters?
Yeah! Christopher's POC already introduced that and my cleanup pr is designed to support pluggable "renderers".
One other thing that might be interesting to figure out, but may be beyond the scope of this issue, is how to integrate pretty printers and styled output in messages. This would allow for things like type diffs and domain specific errors - something that could be handy for Rust and other languages too, like LALRPOP. This may lead to some interesting questions in regards to how it interacts with the different backends, however.
I've seen Christopher toying with that. I consider it to be out of scope as long as we don't need to alter the input to produce it and I think we don't .
So, worth opening a new issue I think :)
OK, I decided not to update the OP but just to do the comparison here. I've attempted to normalize some naming and removed export locations for the purposes of comparison, as well as used a couple shorthands.
cleanup
APIBased on the fact that with both ansi_term
and termcolor
we're just spitting out ANSI color codes without trying to make sure the environment supports it, I think it might even be better just to vendor the use of ansi_term
and emit the ANSI codes directly, then provide a NoStyle and a DefaultANSI implementation of the style provider. This is, of course, assuming that we want to avoid a public dependency on termcolor
and sinking to a termcolor::Write
, and leaving it up to the user to enable ANSI support in the Windows Console if they want it. (Note: Windows Terminal supports ANSI by default, but Windows Console will remain the default host for backwards compatibility, and ANSI is opt-in there IIRC.)
take-two
proposed APIHaving played around with this API and observed the cleanup
API, I'd now like to propose one more "meet in the middle" API that I think can preserve the simplicity of cleanup
while keeping some of the power of my take-two
. This still includes the delayed Span
resolution, because I think it's the key superpower I'm pushing for. I'll prepare a PR against cleanup
that implements this API, hopefully by the end of this coming Sunday. That will inform the specific functions that input traits have to provide.
struct Snippet<'a, Span> {
title: Option<Title<'a>>,
slices: &'a [Slice<'a, Span>],
}
struct Title<'a> {
code: Option<&'a dyn Display>,
message: Option<Message<'a>>,
}
struct Slice<'a, Span> {
span: Span,
origin: Option<&'a dyn Display>,
space_before: bool,
annotations: &'a [Annotation<'a, Span>],
footer: &'a [Message<'a>],
}
struct Annotation<'a, Span> {
span: Span,
message: Message<'a>,
}
struct Message<'a> {
text: &'a dyn Display,
level: Level,
}
enum Level { Error, Warning, Info, Note, Help }
fn format<'a, Span>(&Snippet<'a, Span>, &[dyn|impl] SpanFormatter<Span>) -> FormattedSnippet<'a, Span>;
struct FormattedSnippet<'a, Span> { .. } // RefIntoIter of structured output lines
struct AsciiRenderer { emit_ansi_color: bool }
fn AsciiRenderer.write<'a, Span>(&FormattedSnippet<'a, Span>, &[mut|impl] dyn io::Write, &[dyn|impl] SpanRenderer<Span>) -> io::Write<()>;
// these are approximate until impl shows what they actually need to do
trait Span: Clone {
type Pos: PartialOrd;
fn start(&self) -> Pos;
fn end(&self) -> Pos;
fn new(&self, start: Pos, end: Pos) -> Self;
}
trait SpanFormatter<Span> {
fn split_lines(&self, Span) -> impl Iterator<Item=Span> + '_;
fn count_columns(&self, Span) -> usize;
}
trait SpanRenderer<Span> {
fn write(&self, &mut [dyn|impl] io::Write, &Span) -> io::Result<()>;
}
impl Span for ops::Range<usize>;
impl SpanFormatter<impl Span<Pos=usize>> for &str;
impl SpanRenderer<impl Span<Pos=usize>> for &str;
Specific benefits of this API:
&str
; it can be anything that can implement Display
. This is especially useful when the origin source isn't a string type but can be converted to one, so the intermediate allocation doesn't have to happen. Playing with take-two
I think this dyn
has negligible effect on runtime, the real killer to my time is dyn
indirection to the span resolvers IIUC.SpanRenderer
to emit ANSI coloring for the displayed spans.FormattedSnippet
is public for alternate renderers to use, such as ones that want to use box drawing characters. The layout work is done (i.e. placing lines where they need to be) and all the renderer does is translate the structured output into io::Write
calls.I note that you've claimed that the cleanup
API would support coloring of the source snippets. Even if we just admit that we only care about ANSI-compatible terminals, the zero-width ANSI control codes would mess up your column calculations and require a re-allocation of the source string with said ANSI control codes in it.
I've made a PR with an implementation of my new proposed API, and honestly, if we acquiesce support for non-ANSI coloring, I think it's the best yet. It even supports both the "stick &str in the slice root, use relative offsets" AND "use offsets into a late-bound source" at the same time!
My benchmark results, with dyn
where I put it by default:
format [take-3 &str] time: [10.768 us 11.084 us 11.448 us]
format [take-3 Range] time: [11.037 us 11.427 us 11.871 us]
format [cleanup] time: [11.008 us 11.398 us 11.828 us]
so basically it's all within noise of each-other, which is great!
As an example of the API, see the big example I wrote on Snippet
:
The API still needs fold support (cleanup does not have it yet) as well as an implementation for printing footer notes (cleanup does not have it yet) as well as anonymizing line numbers (cleanup does not have it yet), and then I think it's feature parity with master
.
PR is up as #14.
@CAD97 - apologies for radio silence. I haven't had a lot of time lately to finish this work, but I feel like I now have all the pieces in place to sort it out and I'll try to take a deeper dive into your proposal and finishing cleanup branch!
Is there a proposed merge()
function for spans or should that be done by the library users? I need to be able to merge spans from different parts of an expression (10 + 20
should be 7 characters, using the spans from 10
, +
, and 20
).
The code for merge is super simple btw: https://github.com/brendanzab/codespan/pull/149
Closing this and #14 in favor of #90, #91, #101, and #102.
If there are things that were missed, I'd recommend creating more specific issues.
In order to address concerns in #11, #12, https://github.com/wycats/language-reporting/issues/6, and probably others.
I've been experimenting with merging the APIs of
codespan
/language-reporting
/annotate-snippets
, and the below API surface is what that I think makes the most sense.Original Proposal
An experimental implementation of the API based on #12 is at https://github.com/CAD97/retort/pull/1 (being pushed within 24 hours of posting, I've got one last bit to "port" but I've got to get to bed now but I wanted to get this posted first). > EDIT: I've reconsidered this API, though the linked PR does implement most of it. I'm sketching a new slightly lower-level design from this one, and the diagnostic layout of this current API will probably be a wrapper library around `annotate-snippets`. (I get to use the `retort` name!)API
```rust use termcolor::WriteColor; trait Span: fmt::Debug + Copy { type Origin: ?Sized + fmt::Debug + Eq; fn start(&self) -> usize; fn end(&self) -> usize; fn new(&self, start: usize, end: usize) -> Self; fn origin(&self) -> &Self::Origin; } trait SpanResolver