Skip to content

Commit

Permalink
fix(label-collections): Label collection fixes and cleanup (#343)
Browse files Browse the repository at this point in the history 
* feat(collection): add label(collection) documentation to lib.rs

* feat(collection): remove repeated formatting of label text

Because of a typo, the label text was being formatted multiple times per
label in a collection. With the fix, the text is formatted only once per
collection

* feat(collection): chain iterators rather than extend vector

Since we are going to iterate anyway, instead of growing the label vector,
chain the iterators. This should be more efficient.

In some cases, this also remove a compilation warning about the label
vector being unnecessarily mutable.

* feat(collection): remove unnecessary `OptionalWrapper`

- In a label collection, there is no need for a `None` label. One should
  just not add that label
- Code wise it is also incorrect. The wrapper will be for a vector of
  spans, not for individual spans. It happens to work anyway, but this was
  not the intended use.
  • Loading branch information
@Nahor
Nahor committed Feb 17, 2024
1 parent a18a644 commit 75fea09
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 38 deletions.
74 changes: 36 additions & 38 deletions miette-derive/src/label.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ impl Labels {

pub(crate) fn gen_struct(&self, fields: &syn::Fields) -> Option<TokenStream> {
let (display_pat, display_members) = display_pat_members(fields);
let labels_gen_var = quote! { labels };
let labels = self.0.iter().filter_map(|highlight| {
let Label {
span,
Expand Down Expand Up @@ -194,11 +193,11 @@ impl Labels {
))
})
});
let collections = self.0.iter().filter_map(|label| {
let collections_chain = self.0.iter().filter_map(|label| {
let Label {
span,
label,
ty,
ty: _,
lbl_ty,
} = label;
if *lbl_ty != LabelType::Collection {
Expand All @@ -211,18 +210,17 @@ impl Labels {
quote! { std::option::Option::None }
};
Some(quote! {
let display = #display;
#labels_gen_var.extend(self.#span.iter().map(|label| {
miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(label)
.map(|span| {
use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan};
let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone());
if #display.is_some() && labeled_span.label().is_none() {
labeled_span.set_label(#display)
}
labeled_span
})
}));
.chain({
let display = #display;
self.#span.iter().map(move |span| {
use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan};
let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone());
if display.is_some() && labeled_span.label().is_none() {
labeled_span.set_label(display.clone())
}
Some(labeled_span)
})
})
})
});

Expand All @@ -232,12 +230,13 @@ impl Labels {
use miette::macro_helpers::ToOption;
let Self #display_pat = self;

let mut #labels_gen_var = vec![
let labels_iter = vec![
#(#labels),*
];
#(#collections)*
]
.into_iter()
#(#collections_chain)*;

std::option::Option::Some(Box::new(#labels_gen_var.into_iter().filter(Option::is_some).map(Option::unwrap)))
std::option::Option::Some(Box::new(labels_iter.filter(Option::is_some).map(Option::unwrap)))
}
})
}
Expand All @@ -249,7 +248,6 @@ impl Labels {
|ident, fields, DiagnosticConcreteArgs { labels, .. }| {
let (display_pat, display_members) = display_pat_members(fields);
labels.as_ref().and_then(|labels| {
let labels_gen_var = quote! { labels };
let variant_labels = labels.0.iter().filter_map(|label| {
let Label { span, label, ty, lbl_ty } = label;
if *lbl_ty == LabelType::Collection {
Expand Down Expand Up @@ -282,8 +280,8 @@ impl Labels {
))
})
});
let collections = labels.0.iter().filter_map(|label| {
let Label { span, label, ty, lbl_ty } = label;
let collections_chain = labels.0.iter().filter_map(|label| {
let Label { span, label, ty: _, lbl_ty } = label;
if *lbl_ty != LabelType::Collection {
return None;
}
Expand All @@ -300,18 +298,17 @@ impl Labels {
quote! { std::option::Option::None }
};
Some(quote! {
let display = #display;
#labels_gen_var.extend(#field.iter().map(|label| {
miette::macro_helpers::OptionalWrapper::<#ty>::new().to_option(label)
.map(|span| {
use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan};
let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone());
if #display.is_some() && labeled_span.label().is_none() {
labeled_span.set_label(#display)
}
labeled_span
})
}));
.chain({
let display = #display;
#field.iter().map(move |span| {
use miette::macro_helpers::{ToLabelSpanWrapper,ToLabeledSpan};
let mut labeled_span = ToLabelSpanWrapper::to_labeled_span(span.clone());
if display.is_some() && labeled_span.label().is_none() {
labeled_span.set_label(display.clone());
}
Some(labeled_span)
})
})
})
});
let variant_name = ident.clone();
Expand All @@ -320,11 +317,12 @@ impl Labels {
_ => Some(quote! {
Self::#variant_name #display_pat => {
use miette::macro_helpers::ToOption;
let mut #labels_gen_var = vec![
let labels_iter = vec![
#(#variant_labels),*
];
#(#collections)*
std::option::Option::Some(std::boxed::Box::new(#labels_gen_var.into_iter().filter(Option::is_some).map(Option::unwrap)))
]
.into_iter()
#(#collections_chain)*;
std::option::Option::Some(std::boxed::Box::new(labels_iter.filter(Option::is_some).map(Option::unwrap)))
}
}),
}
Expand Down
52 changes: 52 additions & 0 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
//! - [... handler options](#-handler-options)
//! - [... dynamic diagnostics](#-dynamic-diagnostics)
//! - [... syntax highlighting](#-syntax-highlighting)
//! - [... collection of labels](#-collection-of-labels)
//! - [Acknowledgements](#acknowledgements)
//! - [License](#license)
//!
Expand Down Expand Up @@ -672,6 +673,57 @@
//! [`with_syntax_highlighting`](MietteHandlerOpts::with_syntax_highlighting)
//! method. See the [`highlighters`] module docs for more details.
//!
//! ### ... collection of labels
//!
//! When the number of labels is unknown, you can use a collection of `SourceSpan`
//! (or any type convertible into `SourceSpan`). For this, add the `collection`
//! parameter to `label` and use any type than can be iterated over for the field.
//!
//! ```rust,ignore
//! #[derive(Debug, Diagnostic, Error)]
//! #[error("oops!")]
//! struct MyError {
//! #[label("main issue")]
//! primary_span: SourceSpan,
//!
//! #[label(collection, "related to this")]
//! other_spans: Vec<Range<usize>>,
//! }
//!
//! let report: miette::Report = MyError {
//! primary_span: (6, 9).into(),
//! other_spans: vec![19..26, 30..41],
//! }.into();
//!
//! println!("{:?}", report.with_source_code("About something or another or yet another ...".to_string()));
//! ```
//!
//! A collection can also be of `LabeledSpan` if you want to have different text
//! for different labels. Labels with no text will use the one from the `label`
//! attribute
//!
//! ```rust,ignore
//! #[derive(Debug, Diagnostic, Error)]
//! #[error("oops!")]
//! struct MyError {
//! #[label("main issue")]
//! primary_span: SourceSpan,
//!
//! #[label(collection, "related to this")]
//! other_spans: Vec<LabeledSpan>, // LabeledSpan
//! }
//!
//! let report: miette::Report = MyError {
//! primary_span: (6, 9).into(),
//! other_spans: vec![
//! LabeledSpan::new(None, 19, 7), // Use default text `related to this`
//! LabeledSpan::new(Some("and also this".to_string()), 30, 11), // Use specific text
//! ],
//! }.into();
//!
//! println!("{:?}", report.with_source_code("About something or another or yet another ...".to_string()));
//! ```
//!
//! ## MSRV
//!
//! This crate requires rustc 1.70.0 or later.
Expand Down

0 comments on commit 75fea09

Please sign in to comment.