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
frontend-utils: Made new crate and moved bookmarks & all toml serialization utils to it #15865
Conversation
(side note: the name is kinda clunky as a commit prefix... ugh) |
4ae67c1
to
d84e7fe
Compare
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.
Looks good to me, just a few nits.
fn find_table(toml_document: &mut DocumentMut) -> &mut ArrayOfTables { | ||
if toml_document.contains_array_of_tables("bookmark") { | ||
return toml_document["bookmark"] | ||
.as_array_of_tables_mut() | ||
.expect("type was just verified"); | ||
} | ||
|
||
fn get_underlying_table(&mut self) -> &mut ArrayOfTables { | ||
if self.0.toml_document.contains_array_of_tables("bookmark") { | ||
return self.0.toml_document["bookmark"] | ||
tracing::warn!("missing or invalid bookmark array, recreating.."); | ||
toml_document.insert("bookmark", array()); | ||
toml_document["bookmark"] | ||
.as_array_of_tables_mut() | ||
.expect("type was just verified"); | ||
.expect("type was just created") | ||
} |
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.
side note: not related to the PR but I thought about making a WriteExt and have this as a get_or_create_...
method there.
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.
That'd probably look quite nice! But I'll save it for a followup, yeah? :D
impl<T: Default> Default for DocumentHolder<T> { | ||
fn default() -> Self { | ||
Self { | ||
toml_document: Default::default(), | ||
inner: Default::default(), | ||
} | ||
} | ||
} |
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.
clever 😄 (I meant the Deref, but my hands are not cooperating with me, selected wrong lines)
d84e7fe
to
18ec743
Compare
…rences and bookmarks
18ec743
to
04e17f3
Compare
This crate will eventually serve as the shared code between desktop and android (and any other future frontend), so we don't need to keep reinventing things over and over.
This PR just moves all the serialization utils to it (with cleanups), but followup PRs will add more things such as per-swf launch settings.