-
Notifications
You must be signed in to change notification settings - Fork 902
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update annotate-snippets #6391
Update annotate-snippets #6391
Conversation
@clubby789 Can you provide an example of what the error output looks like before and after this change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for confirming that!
Taking a look at the changelog, these breaking changes stick out to me: Specifically To the best of my knowledge rustfmt has always used byte offsets so before merging I want to test to see if bumping our annotate-snippets dependency fixes anything else. |
cc me on old bump #6018 |
Co-authored-by: printfn <[email protected]>
845edb9
to
131cc27
Compare
I did some testing. This doesn't solve all the annotate-snippets issues, but it solved some. For example, this input from #5888. I don't think we're any worse off by upgrading to pub(crate) fn sanity_needs_ocr<T: HasWord + HasRect>(collection: &[T]) -> bool {
let needs_ocr = collection.is_empty()
// the presence of 'UNICODE REPLACEMENT CHARACTER'
|| collection.iter().any(|c| c.text().contains('�'))
// (yes, empty pages will also be OCR'd)
|| collection_is_void_of_text(collection)
|| is_gibberish(&collection.to_rendered_text());
#[cfg(debug_assertions)]
if needs_ocr {
tracing::trace!(
"sanity check failed, with text: {:?}",
collection.to_rendered_text()
);
}
needs_ocr
} |
This will allow rust-lang/rust to eliminate a duplicate dependency on two different versions of
annotate-snippets
. Extracted from #6366