Skip to content

Commit a158fb7

Browse files
committed
refactor(shopping_list): compact_checked takes ingredient names
Previously `compact_checked(entries, &ShoppingList)` walked the list looking for `Ingredient` items. That works only if the caller's list is fully expanded — callers that persist shopping lists as recipe *references* (the on-disk `.shopping-list` format, CookCLI, UniFFI clients) always got an empty ingredient set, so every compaction silently dropped every checked entry. Change the contract to be honest: callers pass an iterable of the ingredient names the user currently sees. That shifts the expansion responsibility back to the layer that has parser context. Makes `collect_ingredient_names` public for callers whose lists happen to contain `Ingredient` items directly. FFI wrapper takes `&[String]`.
1 parent 71c5d56 commit a158fb7

3 files changed

Lines changed: 82 additions & 27 deletions

File tree

bindings/src/lib.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -668,23 +668,27 @@ pub fn write_shopping_check_entry(
668668
shopping_list::write_check_entry_impl(entry)
669669
}
670670

671-
/// Compacts a checked log against a shopping list
671+
/// Compacts a checked log against the set of currently-visible ingredient
672+
/// names.
672673
///
673674
/// Removes entries for ingredients that are no longer in the shopping list,
674675
/// and collapses the log so each ingredient appears at most once.
675676
///
676677
/// # Arguments
677678
/// * `entries` - The current check log entries
678-
/// * `list` - The current shopping list to compact against
679+
/// * `current_ingredients` - The fully-aggregated ingredient names as shown
680+
/// to the user. A raw on-disk `ShoppingList` only stores recipe
681+
/// references, so callers must expand recipes first and pass the
682+
/// resulting ingredient names here.
679683
///
680684
/// # Returns
681685
/// A compacted list of check entries
682686
#[uniffi::export]
683687
pub fn compact_shopping_checked(
684688
entries: &[shopping_list::CheckEntry],
685-
list: &shopping_list::ShoppingList,
689+
current_ingredients: &[String],
686690
) -> Vec<shopping_list::CheckEntry> {
687-
shopping_list::compact_checked_impl(entries, list)
691+
shopping_list::compact_checked_impl(entries, current_ingredients)
688692
}
689693

690694
uniffi::setup_scaffolding!();

bindings/src/shopping_list.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,16 +163,23 @@ pub fn write_check_entry_impl(entry: &CheckEntry) -> Result<String, String> {
163163
String::from_utf8(buf).map_err(|e| e.to_string())
164164
}
165165

166-
/// Compact a checked log against a shopping list.
166+
/// Compact a checked log against the ingredient names currently in the
167+
/// shopping list.
168+
///
169+
/// Callers should pass the fully-aggregated ingredient names the user
170+
/// actually sees. A raw on-disk [`ShoppingList`] usually contains only
171+
/// recipe references, not ingredients, so it cannot be used directly.
167172
pub fn compact_checked_impl(
168173
entries: &[CheckEntry],
169-
list: &ShoppingList,
174+
current_ingredients: &[String],
170175
) -> Vec<CheckEntry> {
171176
let original_entries: Vec<OriginalCheckEntry> =
172177
entries.iter().map(OriginalCheckEntry::from).collect();
173-
let original_list = OriginalShoppingList::from(list);
174-
sl::compact_checked(&original_entries, &original_list)
175-
.iter()
176-
.map(CheckEntry::from)
177-
.collect()
178+
sl::compact_checked(
179+
&original_entries,
180+
current_ingredients.iter().map(String::as_str),
181+
)
182+
.iter()
183+
.map(CheckEntry::from)
184+
.collect()
178185
}

src/shopping_list.rs

Lines changed: 60 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -406,22 +406,34 @@ pub fn write_check_entry(entry: &CheckEntry, mut w: impl std::io::Write) -> std:
406406
}
407407
}
408408

409-
/// Compact a checked log against a shopping list: keep only `+ name` entries
410-
/// for ingredients that are checked AND still present in the shopping list.
411-
pub fn compact_checked(
412-
entries: &[CheckEntry],
413-
list: &ShoppingList,
414-
) -> Vec<CheckEntry> {
409+
/// Compact a checked log against the set of ingredient names currently in
410+
/// the shopping list: keep only `+ name` entries for ingredients that are
411+
/// still present.
412+
///
413+
/// `current_ingredients` must be the actual ingredient names as they would
414+
/// be rendered to the user — after aggregating referenced recipes, applying
415+
/// any pantry/aisle filtering, etc. A raw on-disk `ShoppingList` usually
416+
/// contains only recipe *references* (not `Ingredient` items), so callers
417+
/// that persist lists that way must expand them first. Passing an iterator
418+
/// rather than a `ShoppingList` makes this contract explicit.
419+
///
420+
/// Matching is case-insensitive; returned `+ name` entries preserve the
421+
/// lowercased form stored in `checked_set`.
422+
pub fn compact_checked<'a, I>(entries: &[CheckEntry], current_ingredients: I) -> Vec<CheckEntry>
423+
where
424+
I: IntoIterator<Item = &'a str>,
425+
{
415426
let current = checked_set(entries);
416-
let list_ingredients = collect_ingredient_names(list);
427+
let list_ingredients: std::collections::HashSet<String> = current_ingredients
428+
.into_iter()
429+
.map(|n| n.to_lowercase())
430+
.collect();
417431

418-
let mut compacted = Vec::new();
419-
for name in &current {
420-
// Keep only if the ingredient still exists in the shopping list
421-
if list_ingredients.iter().any(|n| n.to_lowercase() == *name) {
422-
compacted.push(CheckEntry::Checked(name.clone()));
423-
}
424-
}
432+
let mut compacted: Vec<CheckEntry> = current
433+
.into_iter()
434+
.filter(|name| list_ingredients.contains(name))
435+
.map(CheckEntry::Checked)
436+
.collect();
425437
compacted.sort_by(|a, b| {
426438
// Only `Checked` entries are ever pushed above, but match both arms
427439
// so this stays correct if the collection logic ever changes.
@@ -434,7 +446,12 @@ pub fn compact_checked(
434446
}
435447

436448
/// Collect all ingredient names from a shopping list (recursively).
437-
fn collect_ingredient_names(list: &ShoppingList) -> Vec<String> {
449+
///
450+
/// This only finds `Ingredient` items. Lists that only store recipe
451+
/// *references* will produce an empty result — in that case, caller must
452+
/// expand the references via their own recipe parser before feeding names
453+
/// into [`compact_checked`].
454+
pub fn collect_ingredient_names(list: &ShoppingList) -> Vec<String> {
438455
let mut names = Vec::new();
439456
collect_ingredient_names_from_items(&list.items, &mut names);
440457
names
@@ -806,13 +823,40 @@ salt
806823
#[test]
807824
fn compact_removes_stale() {
808825
let list = parse("salt\npepper\n").unwrap();
826+
let names = collect_ingredient_names(&list);
809827
let entries = parse_checked("+ salt\n+ garlic\n");
810-
let compacted = compact_checked(&entries, &list);
828+
let compacted = compact_checked(&entries, names.iter().map(String::as_str));
811829
// garlic is not in list, should be dropped
812830
assert_eq!(compacted.len(), 1);
813831
assert!(matches!(&compacted[0], CheckEntry::Checked(n) if n == "salt"));
814832
}
815833

834+
#[test]
835+
fn compact_accepts_arbitrary_name_iter() {
836+
// Typical use: caller has already aggregated ingredient names from
837+
// recipe references and passes them in directly.
838+
let entries = parse_checked("+ salt\n+ pepper\n+ garlic\n");
839+
let names = ["salt", "pepper"];
840+
let compacted = compact_checked(&entries, names.iter().copied());
841+
assert_eq!(compacted.len(), 2);
842+
let kept: Vec<&str> = compacted
843+
.iter()
844+
.map(|e| match e {
845+
CheckEntry::Checked(n) | CheckEntry::Unchecked(n) => n.as_str(),
846+
})
847+
.collect();
848+
assert!(kept.contains(&"salt"));
849+
assert!(kept.contains(&"pepper"));
850+
assert!(!kept.contains(&"garlic"));
851+
}
852+
853+
#[test]
854+
fn compact_is_case_insensitive_against_names() {
855+
let entries = parse_checked("+ Salt\n");
856+
let compacted = compact_checked(&entries, ["SALT"].iter().copied());
857+
assert_eq!(compacted.len(), 1);
858+
}
859+
816860
#[test]
817861
fn write_check_entry_roundtrip() {
818862
let entries = vec![

0 commit comments

Comments
 (0)