brendanzab / codespan

Beautiful diagnostic reporting for text-based programming languages.
Apache License 2.0
1.08k stars 58 forks source link

Added generic iterator methods #341

Open yankana opened 2 years ago

yankana commented 2 years ago

It's unnecessary to require allocated vectors instead of just any iterator. With this change, you can use with_labels_iter and with_notes_iter in the example code (and in other code) with arrays instead of vectors to avoid extra heap allocations. The other methods were left untouched so this isn't a breaking change.

Johann150 commented 2 years ago

Thanks for your contribution!

Since Vec<String> is compatible with impl IntoIterator<Item = String>, and likewise for the other function, I think we could even just replace the existing methods instead of creating new ones.

yankana commented 2 years ago

Since Vec<String> is compatible with impl IntoIterator<Item = String>, and likewise for the other function, I think we could even just replace the existing methods instead of creating new ones.

That would be a breaking change because changing a method from using a concrete type to a generic/opaque type would break code that relies on the compiler inferring the type to be a vector. Example:

// `collect::<Vec<String>>` is inferred here, but with `impl IntoIterator<Item = String>`, the compiler cannot infer
let diagnostic = Diagnostic::error().with_notes(string_iter.collect());