Skip to content
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

Fix Renaming note fields not updating custom browser appearance #3245

Merged
merged 10 commits into from
Jun 24, 2024
Merged

Fix Renaming note fields not updating custom browser appearance #3245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ec9ab02
Added methods to parse browser templates
jamesnelmore Jun 19, 2024
d84e850
Added method to get parsed browser templates
jamesnelmore Jun 19, 2024
be01ea2
Make field rename check browser templates for field updates
jamesnelmore Jun 19, 2024
fdd260e
Update tests
jamesnelmore Jun 19, 2024
e3a3107
Updated CONTRIBUTORS
jamesnelmore Jun 19, 2024
d6d284a
Formatting
jamesnelmore Jun 19, 2024
347dcf4
Merge branch 'main' into main
jamesnelmore Jun 20, 2024
e735c04
Merge branch 'ankitects:main' into main
jamesnelmore Jun 22, 2024
e18ecfb
Refactored cloze field logic for question template into closure
jamesnelmore Jun 24, 2024
4004012
Refactored cloze field logic for answer template into closure
jamesnelmore Jun 24, 2024
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
1 change: 1 addition & 0 deletions CONTRIBUTORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ Wu Yi-Wei <https://github.com/Ianwu0812>
RRomeroJr <[email protected]>
Xidorn Quan <[email protected]>
Alexander Bocken <[email protected]>
James Elmore <[email protected]>

********************

Expand Down
68 changes: 61 additions & 7 deletions rslib/src/notetype/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,13 +495,18 @@ impl Notetype {
self.reposition_sort_idx();

let mut parsed_templates = self.parsed_templates();
let mut parsed_browser_templates = self.parsed_browser_templates();
let reqs = self.updated_requirements(&parsed_templates);

// handle renamed+deleted fields
if let Some(existing) = existing {
let fields = self.renamed_and_removed_fields(existing);
if !fields.is_empty() {
self.update_templates_for_renamed_and_removed_fields(fields, &mut parsed_templates);
self.update_templates_for_renamed_and_removed_fields(
fields,
&mut parsed_templates,
&mut parsed_browser_templates,
);
}
}
self.config.reqs = reqs;
Expand Down Expand Up @@ -556,24 +561,49 @@ impl Notetype {
&mut self,
fields: HashMap<String, Option<String>>,
parsed: &mut [(Option<ParsedTemplate>, Option<ParsedTemplate>)],
parsed_browser: &mut [(Option<ParsedTemplate>, Option<ParsedTemplate>)],
) {
let first_remaining_field_name = &self.fields.first().unwrap().name;
let is_cloze = self.is_cloze();
for (idx, (q_opt, a_opt)) in parsed.iter_mut().enumerate() {

let q_update_fields = |q_opt: &mut Option<ParsedTemplate>, template_target: &mut String| {
if let Some(q) = q_opt {
q.rename_and_remove_fields(&fields);
if !q.contains_field_replacement() || is_cloze && !q.contains_cloze_replacement() {
q.add_missing_field_replacement(first_remaining_field_name, is_cloze);
}
self.templates[idx].config.q_format = q.template_to_string();
*template_target = q.template_to_string();
}
};

let a_update_fields = |a_opt: &mut Option<ParsedTemplate>, template_target: &mut String| {
if let Some(a) = a_opt {
a.rename_and_remove_fields(&fields);
if is_cloze && !a.contains_cloze_replacement() {
a.add_missing_field_replacement(first_remaining_field_name, is_cloze);
}
self.templates[idx].config.a_format = a.template_to_string();
*template_target = a.template_to_string();
}
};

// Update main templates
for (idx, (q_opt, a_opt)) in parsed.iter_mut().enumerate() {
q_update_fields(q_opt, &mut self.templates[idx].config.q_format);

a_update_fields(a_opt, &mut self.templates[idx].config.a_format);
}

// Update browser templates, if they exist
for (idx, (q_browser_opt, a_browser_opt)) in parsed_browser.iter_mut().enumerate() {
Copy link
Member

Choose a reason for hiding this comment

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

Am I correct in assuming this is basically a copy+paste of the code above? If so, it would be a bit more elegant to factor the common code into a closure, then call the closure once for the main's enumerate(), and once for the browser templates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, I just moved the logic into 2 closures, although I'm not sure if q_update_fields and an_update_fields are the best names. Let me know what you think.

q_update_fields(
q_browser_opt,
&mut self.templates[idx].config.q_format_browser,
);

a_update_fields(
a_browser_opt,
&mut self.templates[idx].config.a_format_browser,
);
}
}

Expand All @@ -583,6 +613,17 @@ impl Notetype {
.map(|t| (t.parsed_question(), t.parsed_answer()))
.collect()
}
fn parsed_browser_templates(&self) -> Vec<(Option<ParsedTemplate>, Option<ParsedTemplate>)> {
self.templates
.iter()
.map(|t| {
(
t.parsed_question_format_for_browser(),
t.parsed_answer_format_for_browser(),
)
})
.collect()
}

fn fix_field_names(&mut self) -> Result<()> {
self.fields.iter_mut().try_for_each(NoteField::fix_name)
Expand Down Expand Up @@ -790,12 +831,17 @@ mod test {
nt_norm.add_template("Card 1", "front {{foo}}", "back {{bar}}");
nt_norm.templates[0].ord = Some(0);
let mut parsed = nt_norm.parsed_templates();
let mut parsed_browser = nt_norm.parsed_browser_templates();

let mut field_map: HashMap<String, Option<String>> = HashMap::new();
field_map.insert("foo".to_owned(), None);
field_map.insert("bar".to_owned(), None);

nt_norm.update_templates_for_renamed_and_removed_fields(field_map, &mut parsed);
nt_norm.update_templates_for_renamed_and_removed_fields(
field_map,
&mut parsed,
&mut parsed_browser,
);
assert_eq!(nt_norm.templates[0].config.q_format, "front {{baz}}");
assert_eq!(nt_norm.templates[0].config.a_format, "back ");

Expand All @@ -815,7 +861,11 @@ mod test {
field_map.insert("foo".to_owned(), None);
field_map.insert("bar".to_owned(), None);

nt_cloze.update_templates_for_renamed_and_removed_fields(field_map, &mut parsed);
nt_cloze.update_templates_for_renamed_and_removed_fields(
field_map,
&mut parsed,
&mut parsed_browser,
);
assert_eq!(nt_cloze.templates[0].config.q_format, "front {{cloze:baz}}");
assert_eq!(nt_cloze.templates[0].config.a_format, "back {{cloze:baz}}");

Expand All @@ -837,7 +887,11 @@ mod test {
let mut field_map: HashMap<String, Option<String>> = HashMap::new();
field_map.insert("bar".to_owned(), None);

nt_cloze.update_templates_for_renamed_and_removed_fields(field_map, &mut parsed);
nt_cloze.update_templates_for_renamed_and_removed_fields(
field_map,
&mut parsed,
&mut parsed_browser,
);
assert_eq!(nt_cloze.templates[0].config.q_format, "front {{cloze:foo}}");
assert_eq!(nt_cloze.templates[0].config.a_format, "back {{cloze:foo}}");
}
Expand Down
7 changes: 7 additions & 0 deletions rslib/src/notetype/templates.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ impl CardTemplate {
ParsedTemplate::from_text(&self.config.a_format).ok()
}

pub(crate) fn parsed_question_format_for_browser(&self) -> Option<ParsedTemplate> {
ParsedTemplate::from_text(&self.config.q_format_browser).ok()
}

pub(crate) fn parsed_answer_format_for_browser(&self) -> Option<ParsedTemplate> {
ParsedTemplate::from_text(&self.config.a_format_browser).ok()
}
pub(crate) fn question_format_for_browser(&self) -> &str {
if !self.config.q_format_browser.is_empty() {
&self.config.q_format_browser
Expand Down