about summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorBenedikt Peetz <benedikt.peetz@b-peetz.de>2025-07-24 16:24:48 +0200
committerBenedikt Peetz <benedikt.peetz@b-peetz.de>2025-07-24 16:24:48 +0200
commit98c3984e1e15a3b98d1eeb191809d1b1ae7be119 (patch)
tree77848adf8974751ebce61fc542c2b6eff9d005aa
parentrefactor(crates/yt/src/ansi_escape_codes): Use better name for `erase_in_disp... (diff)
downloadyt-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.
-rw-r--r--crates/yt/src/storage/db/video/comments/mod.rs36
-rw-r--r--crates/yt/src/storage/db/video/comments/tests.rs28
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),