diff options
author | Benedikt Peetz <benedikt.peetz@b-peetz.de> | 2025-07-24 16:24:48 +0200 |
---|---|---|
committer | Benedikt Peetz <benedikt.peetz@b-peetz.de> | 2025-07-24 16:24:48 +0200 |
commit | 98c3984e1e15a3b98d1eeb191809d1b1ae7be119 (patch) | |
tree | 77848adf8974751ebce61fc542c2b6eff9d005aa /crates | |
parent | refactor(crates/yt/src/ansi_escape_codes): Use better name for `erase_in_disp... (diff) | |
download | yt-98c3984e1e15a3b98d1eeb191809d1b1ae7be119.zip |
fix(crates/yt/storage/db/videos/comments): Don't always associate a reply with its base
The previous code immediately return from the `find_author_mut` function, as the `return` was not scoped in a closure. We now only return that value, if it should actually be returned, and added a test for that case.
Diffstat (limited to 'crates')
-rw-r--r-- | crates/yt/src/storage/db/video/comments/mod.rs | 36 | ||||
-rw-r--r-- | crates/yt/src/storage/db/video/comments/tests.rs | 28 |
2 files changed, 45 insertions, 19 deletions
diff --git a/crates/yt/src/storage/db/video/comments/mod.rs b/crates/yt/src/storage/db/video/comments/mod.rs index d2249b3..e7bd8f5 100644 --- a/crates/yt/src/storage/db/video/comments/mod.rs +++ b/crates/yt/src/storage/db/video/comments/mod.rs @@ -1,4 +1,4 @@ -use std::{iter, mem}; +use std::mem; use regex::{Captures, Regex}; @@ -131,22 +131,28 @@ impl Comments { comment: &'a mut Comment, reply_target_author: &str, ) -> Option<&'a mut Comment> { - let base_check = if comment.value.author == reply_target_author { - return Some(comment); - } else { - None - }; + // TODO(@bpeetz): This is a workaround until rust has lexiographic lifetime support. <2025-07-18> + fn find_in_replies<'a>( + comment: &'a mut Comment, + reply_target_author: &str, + ) -> Option<&'a mut Comment> { + comment + .replies + .iter_mut() + .rev() + .find_map(|reply: &mut Comment| perform_check(reply, reply_target_author)) + } + let comment_author_matches_target = comment.value.author == reply_target_author; - if comment.replies.is_empty() { - base_check - } else { - for reply in comment.replies.iter_mut().rev() { - if let Some(out) = perform_check(reply, reply_target_author) { - return Some(out); - } - } - base_check + match find_in_replies(comment, reply_target_author) { + Some(_) => Some( + // PERFORMANCE(@bpeetz): We should not need to run this code twice. <2025-07-18> + find_in_replies(comment, reply_target_author) + .expect("We already had a Some result for this."), + ), + None if comment_author_matches_target => Some(comment), + None => None, } } diff --git a/crates/yt/src/storage/db/video/comments/tests.rs b/crates/yt/src/storage/db/video/comments/tests.rs index 138b1b6..3bd1c73 100644 --- a/crates/yt/src/storage/db/video/comments/tests.rs +++ b/crates/yt/src/storage/db/video/comments/tests.rs @@ -189,12 +189,12 @@ fn test_comments_replies_1_level() { #[test] fn test_comments_replies_2_levels() { - let (input, expected) = mk_comments! { + let (input, expected) = mk_comments!( (@singer: "We perform medical studies on animals; Children have lower or similar mental ability as these animals.." (@singer: "Therefore, we should perform these studies on children instead, if we were to follow our own principals" (@james: "This is ridiculous! I will not entertain this thought.") (@singer: "Although one could also use this argument to argue for abortion _after_ birth."))) - }; + ); assert_eq!( Comments::from_raw(input).render(true), @@ -204,13 +204,33 @@ fn test_comments_replies_2_levels() { #[test] fn test_comments_replies_3_levels() { - let (input, expected) = mk_comments! { + let (input, expected) = mk_comments!( (@singer: "We perform medical studies on animals; Children have lower or similar mental ability as these animals.." (@singer: "Therefore, we should perform these studies on children instead, if we were to follow our own principals" (@james: "This is ridiculous! I will not entertain this thought." (@singer: "You know that I am not actually suggesting that? This is but a way to critizise the society")) (@singer: "Although one could also use this argument to argue for abortion _after_ birth."))) - }; + ); + + assert_eq!( + Comments::from_raw(input).render(true), + expected.render(true) + ); +} + +#[test] +fn test_comments_sub_answer_selection() { + let (input, expected) = mk_comments!( + (@coffeewolfproductions9113: "I mean, brothels and sex workers in of themselves are not a bad thing." + (@aikikaname6508: "probably not so much in the 50s, pre contraception") + (@as_ri1mb: "it’s an incredibly sad, degrading line of work, often resulting in self loathing and self-deletion." + (@coffeewolfproductions9113: "Are you speaking from experience?" + (@as_ri1mb: "what an immature response, as expected." + (@coffeewolfproductions9113: "I literally just asked if you were talking from experience."))))) + + ); + + eprintln!("{}", expected.render(true)); assert_eq!( Comments::from_raw(input).render(true), |