fix: make hashtag behaviour more consistent

src/hashtag.rs

@@ -1,41 +1,43 @@
+use nostr_sdk::{Alphabet, Tag};
 use std::cmp::Ordering;
 use std::fmt::{Display, Formatter};
-use std::ops::Deref;
+use std::hash::{Hash, Hasher};
-use itertools::Itertools;
-use nostr_sdk::{Alphabet, Tag};
 
 pub fn is_hashtag(tag: &Tag) -> bool {
     tag.single_letter_tag()
         .is_some_and(|letter| letter.character == Alphabet::T)
 }
 
+/// This exists so that Hashtags can easily be matched without caring about case
+/// but displayed in their original case
 #[derive(Clone, Debug)]
-pub struct Hashtag(pub String);
+pub struct Hashtag {
-
+    value: String,
-impl Display for Hashtag {
+    lowercased: String,
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.0)
-    }
 }
 
 impl Hashtag {
-    pub fn content(&self) -> &str { &self.0 }
     pub fn matches(&self, token: &str) -> bool {
-        self.0.contains(&token.to_ascii_lowercase())
+        self.lowercased.contains(&token.to_ascii_lowercase())
+    }
+}
+
+impl Display for Hashtag {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.value)
+    }
+}
+
+impl Hash for Hashtag {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        state.write(self.lowercased.as_bytes());
     }
 }
 
 impl Eq for Hashtag {}
 impl PartialEq<Self> for Hashtag {
     fn eq(&self, other: &Self) -> bool {
-        self.0.to_ascii_lowercase() == other.0.to_ascii_lowercase()
+        self.lowercased == other.lowercased
-    }
-}
-impl Deref for Hashtag {
-    type Target = String;
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
     }
 }
 impl TryFrom<&Tag> for Hashtag {
@@ -43,24 +45,28 @@ impl TryFrom<&Tag> for Hashtag {
 
     fn try_from(value: &Tag) -> Result<Self, Self::Error> {
         value.content().take_if(|_| is_hashtag(value))
-            .map(|s| Hashtag(s.trim().to_string()))
+            .map(|s| Hashtag::from(s))
             .ok_or_else(|| "Tag is not a Hashtag".to_string())
     }
 }
 impl From<&str> for Hashtag {
     fn from(value: &str) -> Self {
-        Hashtag(value.trim().to_string())
+        let val = value.trim().to_string();
+        Hashtag {
+            lowercased: val.to_ascii_lowercase(),
+            value: val,
+        }
     }
 }
 impl From<&Hashtag> for Tag {
     fn from(value: &Hashtag) -> Self {
-        Tag::hashtag(&value.0)
+        Tag::hashtag(&value.lowercased)
     }
 }
 
 impl Ord for Hashtag {
     fn cmp(&self, other: &Self) -> Ordering {
-        self.0.to_ascii_lowercase().cmp(&other.0.to_ascii_lowercase()) 
+        self.lowercased.cmp(&other.lowercased)
         // Wanted to do this so lowercase tags are preferred,
         // but is technically undefined behaviour
         // because it deviates from Eq implementation
@@ -72,7 +78,7 @@ impl Ord for Hashtag {
 }
 impl PartialOrd for Hashtag {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.0.to_ascii_lowercase().cmp(&other.0.to_ascii_lowercase()))
+        Some(self.lowercased.cmp(&other.lowercased))
     }
 }
 
@@ -81,9 +87,10 @@ fn test_hashtag() {
     assert_eq!("yeah", "YeaH".to_ascii_lowercase());
     assert_eq!("yeah".to_ascii_lowercase().cmp(&"YeaH".to_ascii_lowercase()), Ordering::Equal);
 
+    use itertools::Itertools;
     let strings = vec!["yeah", "YeaH"];
     let mut tags = strings.iter().cloned().map(Hashtag::from).sorted_unstable().collect_vec();
-    assert_eq!(strings, tags.iter().map(Hashtag::deref).collect_vec());
+    assert_eq!(strings, tags.iter().map(ToString::to_string).collect_vec());
     tags.sort_unstable();
-    assert_eq!(strings, tags.iter().map(Hashtag::deref).collect_vec());
+    assert_eq!(strings, tags.iter().map(ToString::to_string).collect_vec());
 }

src/main.rs

@@ -584,7 +584,7 @@ async fn main() -> Result<()> {
 
                     Some('+') =>
                         match arg {
-                            Some(arg) => tasks.add_tag(arg.to_string()),
+                            Some(arg) => tasks.add_tag(arg),
                             None => {
                                 tasks.print_hashtags();
                                 if tasks.has_tag_filter() {
@@ -710,7 +710,7 @@ async fn main() -> Result<()> {
                                 tasks.get_filtered(pos, |t| {
                                     transform(&t.event.content).contains(&remaining) ||
                                         t.list_hashtags().any(
-                                            |tag| tag.contains(&remaining))
+                                            |tag| tag.matches(&remaining))
                                 });
                             if filtered.len() == 1 {
                                 tasks.move_to(filtered.into_iter().next());

src/tasks.rs

@@ -243,12 +243,8 @@ impl TasksRelay {
             .filter(|t| t.pure_state() != State::Closed)
     }
 
-    pub(crate) fn all_hashtags(&self) -> impl Iterator<Item=String> {
+    pub(crate) fn all_hashtags(&self) -> BTreeSet<Hashtag> {
-        self.nonclosed_tasks()
+        self.nonclosed_tasks().flat_map(|t| t.list_hashtags()).collect()
-            .flat_map(|t| t.list_hashtags())
-            .sorted_unstable()
-            .dedup()
-            .map(|h| h.0)
     }
 
     /// Dynamic time tracking overview for current task or current user.
@@ -727,7 +723,7 @@ impl TasksRelay {
     pub(crate) fn print_hashtags(&self) {
         println!(
             "Hashtags of all known tasks:\n{}",
-            self.all_hashtags().join(" ").italic()
+            self.all_hashtags().into_iter().join(" ").italic()
         );
     }
 
@@ -751,10 +747,10 @@ impl TasksRelay {
         self.tags.extend(tags);
     }
 
-    pub(crate) fn add_tag(&mut self, tag: String) {
+    pub(crate) fn add_tag(&mut self, tag: &str) {
         self.view.clear();
         info!("Added tag filter for #{tag}");
-        let tag = Hashtag(tag);
+        let tag = Hashtag::from(tag);
         self.tags_excluded.remove(&tag);
         self.tags.insert(tag);
     }
@@ -762,11 +758,11 @@ impl TasksRelay {
     pub(crate) fn remove_tag(&mut self, tag: &str) {
         self.view.clear();
         let len = self.tags.len();
-        self.tags.retain(|t| !t.starts_with(tag));
+        self.tags.retain(|t| !t.matches(tag));
         if self.tags.len() < len {
-            info!("Removed tag filters starting with {tag}");
+            info!("Removed tag filters containing {tag}");
         } else {
-            self.tags_excluded.insert(Hashtag(tag.to_string()).into());
+            self.tags_excluded.insert(Hashtag::from(tag).into());
             info!("Excluding #{tag} from view");
         }
     }
@@ -1784,21 +1780,30 @@ mod tasks_test {
         let parent = tasks.make_task("parent #tag1");
         tasks.move_to(Some(parent));
         let sub = tasks.make_task("sub #oi # tag2");
-        assert_eq!(tasks.all_hashtags().collect_vec(), vec!["oi", "tag1", "tag2"]);
+        assert_eq!(tasks.all_hashtags(), ["oi", "tag1", "tag2"].into_iter().map(Hashtag::from).collect());
         tasks.make_note("note with #tag3 # yeah");
-        assert_eq!(tasks.all_hashtags().collect_vec(), vec!["oi", "tag1", "tag2", "tag3", "yeah"]);
+        let all_tags = ["oi", "tag1", "tag2", "tag3", "yeah"].into_iter().map(Hashtag::from).collect();
+        assert_eq!(tasks.all_hashtags(), all_tags);
 
         tasks.custom_time = Some(Timestamp::now());
         tasks.update_state("Finished #YeaH # oi", State::Done);
         assert_eq!(tasks.get_by_id(&parent).unwrap().list_hashtags().collect_vec(), ["tag1", "YeaH", "oi", "tag3", "yeah"].map(Hashtag::from));
-        assert_eq!(tasks.all_hashtags().collect_vec(), vec!["oi", "tag1", "tag2", "tag3", "YeaH"]);
+        assert_eq!(tasks.all_hashtags(), all_tags);
 
         tasks.custom_time = Some(now());
         tasks.update_state("Closing Down", State::Closed);
         assert_eq!(tasks.get_by_id(&sub).unwrap().pure_state(), State::Closed);
         assert_eq!(tasks.get_by_id(&parent).unwrap().pure_state(), State::Closed);
         assert_eq!(tasks.nonclosed_tasks().next(), None);
-        assert_eq!(tasks.all_hashtags().next(), None);
+        assert_eq!(tasks.all_hashtags(), Default::default());
+    }
+    
+    #[test]
+    fn test_tags() {
+        let mut tasks = stub_tasks();
+        tasks.update_tags(["dp", "yeah"].into_iter().map(Hashtag::from));
+        tasks.remove_tag("Y");
+        assert_eq!(tasks.tags, ["dp"].into_iter().map(Hashtag::from).collect());
     }
 
     #[test]
@@ -1854,7 +1859,7 @@ mod tasks_test {
         assert_tasks!(tasks, [pin, test, parent]);
         tasks.set_view_depth(1);
         assert_tasks!(tasks, [pin, test]);
-        tasks.add_tag("tag".to_string());
+        tasks.add_tag("tag");
         assert_tasks!(tasks, [test]);
         assert_eq!(
             tasks.filtered_tasks(None, true),
@@ -2042,7 +2047,7 @@ mod tasks_test {
         tasks.view_depth = 9;
         assert_tasks!(tasks, [t111, t12]);
 
-        tasks.add_tag("tag".to_string());
+        tasks.add_tag("tag");
         tasks.view_depth = 0;
         assert_tasks!(tasks, [t11]);
         tasks.search_depth = 0;