Skip to content

attributed_text: switch to structured Error + ErrorKind with bo… #458

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

attributed_text: switch to structured Error + ErrorKind with bo… #458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 91 additions & 63 deletions attributed_text/src/attributed_text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,7 @@ use alloc::vec::Vec;
use core::fmt::Debug;
use core::ops::Range;

use crate::TextStorage;

/// The errors that might happen as a result of [applying] an attribute.
///
/// [applying]: AttributedText::apply_attribute
#[derive(Copy, Clone, Debug, PartialEq)]
#[non_exhaustive]
pub enum ApplyAttributeError {
/// The bounds given were invalid.
///
/// TODO: Store some data about this here.
InvalidBounds,
/// The range has `start > end`.
InvalidRange,
/// Either `start` or `end` is not on a UTF-8 character boundary.
NotOnCharBoundary,
}

impl core::fmt::Display for ApplyAttributeError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
Self::InvalidBounds => f.write_str("attribute range is out of bounds"),
Self::InvalidRange => f.write_str("attribute range start is greater than end"),
Self::NotOnCharBoundary => {
f.write_str("attribute range must align to UTF-8 char boundaries")
}
}
}
}

impl core::error::Error for ApplyAttributeError {}
use crate::{Endpoint, Error, TextStorage};

/// A block of text with attributes applied to ranges within the text.
#[derive(Debug)]
Expand All @@ -62,20 +32,33 @@ impl<T: Debug + TextStorage, Attr: Debug> AttributedText<T, Attr> {
}

/// Apply an `attribute` to a `range` within the text.
pub fn apply_attribute(
&mut self,
range: Range<usize>,
attribute: Attr,
) -> Result<(), ApplyAttributeError> {
pub fn apply_attribute(&mut self, range: Range<usize>, attribute: Attr) -> Result<(), Error> {
let text_len = self.text.len();
if range.start > range.end {
return Err(ApplyAttributeError::InvalidRange);
return Err(Error::invalid_range(range.start, range.end, text_len));
}
if range.start > text_len || range.end > text_len {
return Err(ApplyAttributeError::InvalidBounds);
return Err(Error::invalid_bounds(range.start, range.end, text_len));
}
if !self.text.is_char_boundary(range.start) || !self.text.is_char_boundary(range.end) {
return Err(ApplyAttributeError::NotOnCharBoundary);
if !self.text.is_char_boundary(range.start) {
return Err(Error::not_on_char_boundary(
&self.text,
range.start,
range.end,
text_len,
Endpoint::Start,
range.start,
));
}
if !self.text.is_char_boundary(range.end) {
return Err(Error::not_on_char_boundary(
&self.text,
range.start,
range.end,
text_len,
Endpoint::End,
range.end,
));
}
self.attributes.push((range, attribute));
Ok(())
Expand Down Expand Up @@ -114,7 +97,8 @@ impl<T: Debug + TextStorage, Attr: Debug> AttributedText<T, Attr> {

#[cfg(test)]
mod tests {
use super::{ApplyAttributeError, AttributedText};
use crate::{AttributedText, Endpoint, ErrorKind};
use alloc::format;
use alloc::vec::Vec;

#[derive(Debug, PartialEq)]
Expand All @@ -128,8 +112,8 @@ mod tests {
let t = "Hello!";
let mut at = AttributedText::new(t);

assert_eq!(at.apply_attribute(1..3, TestAttribute::Keep), Ok(()));
assert_eq!(at.apply_attribute(2..5, TestAttribute::Remove), Ok(()));
assert!(at.apply_attribute(1..3, TestAttribute::Keep).is_ok());
assert!(at.apply_attribute(2..5, TestAttribute::Remove).is_ok());

assert!(at.attributes_at(0).collect::<Vec<_>>().is_empty());
}
Expand All @@ -143,32 +127,76 @@ mod tests {
let t = "Hello!";
let mut at = AttributedText::new(t);

assert_eq!(at.apply_attribute(0..3, TestAttribute::Keep), Ok(()));
assert_eq!(at.apply_attribute(0..6, TestAttribute::Keep), Ok(()));
assert_eq!(
at.apply_attribute(4..3, TestAttribute::Keep),
Err(ApplyAttributeError::InvalidRange)
);
assert_eq!(
at.apply_attribute(0..7, TestAttribute::Keep),
Err(ApplyAttributeError::InvalidBounds)
);
assert_eq!(
at.apply_attribute(7..8, TestAttribute::Keep),
Err(ApplyAttributeError::InvalidBounds)
);
assert!(at.apply_attribute(0..3, TestAttribute::Keep).is_ok());
assert!(at.apply_attribute(0..6, TestAttribute::Keep).is_ok());
match at.apply_attribute(4..3, TestAttribute::Keep) {
Err(e) => {
assert_eq!(e.kind(), ErrorKind::InvalidRange);
let msg = format!("{}", e);
assert!(msg.contains("4..3"));
assert!(msg.contains("start > end") || msg.contains("invalid range"));
Copy link
Member

@tomcur tomcur Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect to always keep saying both that the range is invalid, and why it's invalid?

Suggested change
assert!(msg.contains("start > end") || msg.contains("invalid range"));
assert!(msg.contains("start > end"));
assert!(msg.contains("invalid range"));

}
_ => panic!("expected InvalidRange"),
}
match at.apply_attribute(0..7, TestAttribute::Keep) {
Err(e) => {
assert_eq!(e.kind(), ErrorKind::InvalidBounds);
let msg = format!("{}", e);
assert!(msg.contains("0..7"));
assert!(msg.contains("len 6"));
}
_ => panic!("expected InvalidBounds"),
}
match at.apply_attribute(7..8, TestAttribute::Keep) {
Err(e) => {
assert_eq!(e.kind(), ErrorKind::InvalidBounds);
let msg = format!("{}", e);
assert!(msg.contains("7..8") || msg.contains("8"));
Copy link
Member

@tomcur tomcur Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above, this perhaps should be &&, but at the same time, as written the second expression is implied by the first.

}
_ => panic!("expected InvalidBounds"),
}
}

#[test]
fn not_on_char_boundary() {
// "é" is 2 bytes in UTF-8; index 1 is not a boundary
// "é" is 2 bytes in UTF-8; index 1 is not a boundary.
let t = "éclair";
let mut at = AttributedText::new(t);
assert_eq!(
at.apply_attribute(1..2, TestAttribute::Keep),
Err(ApplyAttributeError::NotOnCharBoundary)
);
// Invalid start boundary at 1
match at.apply_attribute(1..2, TestAttribute::Keep) {
Err(e) => {
assert_eq!(e.kind(), ErrorKind::NotOnCharBoundary);
let b = e.boundary().expect("boundary info");
assert_eq!(b.which, Endpoint::Start);
assert_eq!(b.index, 1);
assert_eq!(b.char_start, 0);
assert_eq!(b.char_end, 2);
let msg = format!("{}", e);
assert!(msg.contains("range 1..2"));
assert!(msg.contains("start"));
assert!(msg.contains("index 1"));
assert!(msg.contains("char 0..2"));
}
_ => panic!("expected NotOnCharBoundary for start"),
}
// Invalid end boundary at 1
match at.apply_attribute(0..1, TestAttribute::Keep) {
Err(e) => {
assert_eq!(e.kind(), ErrorKind::NotOnCharBoundary);
let b = e.boundary().expect("boundary info");
assert_eq!(b.which, Endpoint::End);
assert_eq!(b.index, 1);
assert_eq!(b.char_start, 0);
assert_eq!(b.char_end, 2);
let msg = format!("{}", e);
assert!(msg.contains("range 0..1"));
assert!(msg.contains("end"));
assert!(msg.contains("index 1"));
assert!(msg.contains("char 0..2"));
}
_ => panic!("expected NotOnCharBoundary for end"),
}
// Using proper boundaries is OK
assert_eq!(at.apply_attribute(0..2, TestAttribute::Keep), Ok(()));
assert!(at.apply_attribute(0..2, TestAttribute::Keep).is_ok());
}
}
Loading