Implement getting HTML on X11 #130
Conversation
lucieleblanc
changed the title
| pub fn from_alt_text(alt_text: String) -> Self { | ||
| Self { alt_text, ..Default::default() } | ||
| } |
There was a problem hiding this comment.
Is this a valid function to expose? If the user doesn't have any HTML in the first place it seems like they'd be better off using set_text() instead.
| @@ -146,6 +146,22 @@ impl<'a> ImageData<'a> { | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Debug, Default)] | |||
There was a problem hiding this comment.
Nit: Let's remove the Default implementation here. It makes it harder to maintain version compatibility in the future.
| /// Fetches HTML from the clipboard and returns it. | ||
| pub fn get_html(&mut self) -> Result<HTMLData, Error> { | ||
| self.get().html() | ||
| } |
There was a problem hiding this comment.
Question: Do you think HTML is a common enough case to add this to the main Clipboard structure? I would prefer to expose this only through the Get builder to keep it tidy.
|
|
||
| let html_result = self.inner.read(&html_format, selection); | ||
| if let Ok(html_data) = html_result { | ||
| let html: String = html_data.bytes.into_iter().map(|c| c as char).collect(); |
There was a problem hiding this comment.
Is this correct? My understanding that HTML can contain UTF-8 so we should just use String::from_utf8 here.
| let alt_text_formats = [ | ||
| self.inner.atoms.UTF8_STRING, | ||
| self.inner.atoms.UTF8_MIME_0, | ||
| self.inner.atoms.UTF8_MIME_1, | ||
| self.inner.atoms.STRING, | ||
| self.inner.atoms.TEXT, | ||
| self.inner.atoms.TEXT_MIME_UNKNOWN, | ||
| ]; |
There was a problem hiding this comment.
Can we find a way to extract this into a reusable construct that get_text can use as well?
| if let Ok(alt_text_data) = alt_text_result { | ||
| data.alt_text = if alt_text_data.format == self.inner.atoms.STRING { | ||
| // ISO Latin-1 | ||
| // See: https://stackoverflow.com/questions/28169745/what-are-the-options-to-convert-iso-8859-1-latin-1-to-a-string-utf-8 | ||
| alt_text_data.bytes.into_iter().map(|c| c as char).collect::<String>() | ||
| } else { | ||
| String::from_utf8(alt_text_data.bytes).map_err(|_| Error::ConversionFailure)? | ||
| }; | ||
| } |
There was a problem hiding this comment.
Same here, because the logic is also duplicated from get_text.
No description provided.