suffix mapping: reduced complexity; fix regression of suffix overriding

src/lib.rs

@@ -27,7 +27,7 @@
 mod fs;
 pub mod style;
 
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::env;
 use std::ffi::OsString;
 use std::fs::{DirEntry, FileType, Metadata};
@@ -143,6 +143,7 @@ impl Indicator {
 }
 
 type FileNameSuffix = String;
+type Priority = usize;
 
 /// Iterator over the path components with their respective style.
 pub struct StyledComponents<'a> {
@@ -221,29 +222,30 @@ impl Colorable for DirEntry {
 const LS_COLORS_DEFAULT: &str = "rs=0:lc=\x1b[:rc=m:cl=\x1b[K:ex=01;32:sg=30;43:su=37;41:di=01;34:st=37;44:ow=34;42:tw=30;42:ln=01;36:bd=01;33:cd=01;33:do=01;35:pi=33:so=01;35:";
 
 #[derive(Default, Debug, Clone)]
-struct SuffixMapping {
-    mappings: HashMap<FileNameSuffix, Option<Style>>,
-    case_sensitive: HashSet<FileNameSuffix>,
+struct SuffixMappingEntry {
+    /// Keys are non-normalized suffixes(variants of the suffix), and values are
+    /// tuples containing an style and their priority.
+    /// We need to store the priority (the index of the entry in the env) here
+    /// because ls has an overriding mechanism. For example,
+    /// *.gz=01;33:*.tar.gz=01;31: and *.tar.gz=01;31:*.gz=01;33: are not the
+    /// same.
+    variants: HashMap<FileNameSuffix, (Option<Style>, Priority)>,
+    // whether the variants are case sensitive or not
+    case_sensitive: bool,
 }
 
-impl SuffixMapping {
-    fn push(&mut self, suffix: FileNameSuffix, style: Option<Style>) {
-        let normalized_suffix = suffix.to_ascii_lowercase();
-        // decides whether new mapping and it's varaints should be treated as case-sensitive
-        let is_case_sensitive = self.mappings.iter().any(|(m_suffix, m_style)| {
-            //any entry that are not exactly same as the new suffix
-            (*m_suffix != suffix)
-                // but is the same string with different letter case
-                && (normalized_suffix == m_suffix.to_ascii_lowercase())
-                // and it's mapped to a different style other than the new style 
-                && (style != *m_style)
+impl SuffixMappingEntry {
+    fn push(&mut self, suffix: FileNameSuffix, style: Option<Style>, priority: Priority) {
+        // Determines if the new mapping and its variants should be treated as
+        // case-sensitive.
+        let is_case_sensitive = self.variants.iter().any(|(v_suffix, (v_style, _))| {
+            //Any entry that are not exactly same as the new suffix,
+            (*v_suffix != suffix)
+                // and it is mapped to a different style other than the new style.
+                && (style != *v_style)
         });
-        if is_case_sensitive {
-            self.case_sensitive.insert(normalized_suffix);
-        } else {
-            self.case_sensitive.remove(&normalized_suffix);
-        }
-        self.mappings.insert(suffix, style);
+        self.case_sensitive = is_case_sensitive;
+        self.variants.insert(suffix, (style, priority));
     }
 }
 
@@ -255,7 +257,7 @@ pub struct LsColors {
     /// Whether Indicator::RegularFile falls back to Indicator::Normal
     /// (see <https://github.com/sharkdp/lscolors/issues/48#issuecomment-1582830387>)
     file_normal_fallback: bool,
-    suffix_mapping: SuffixMapping,
+    suffix_mapping: HashMap<FileNameSuffix, SuffixMappingEntry>,
 }
 
 impl Default for LsColors {
@@ -274,7 +276,7 @@ impl LsColors {
         LsColors {
             indicator_mapping: HashMap::new(),
             file_normal_fallback: true,
-            suffix_mapping: SuffixMapping::default(),
+            suffix_mapping: HashMap::new(),
         }
     }
 
@@ -296,13 +298,12 @@ impl LsColors {
     }
 
     fn add_from_string(&mut self, input: &str) {
-        for entry in input.split(':') {
+        for (idx, entry) in input.split(':').enumerate() {
             let parts: Vec<_> = entry.split('=').collect();
-
             if let Some([entry, ansi_style]) = parts.get(0..2) {
                 let style = Style::from_ansi_sequence(ansi_style);
                 if let Some(suffix) = entry.strip_prefix('*') {
-                    self.suffix_mapping.push(suffix.to_string(), style);
+                    self.add_suffix_entry(suffix.to_string(), style, idx);
                 } else if let Some(indicator) = Indicator::from(entry) {
                     if let Some(style) = style {
                         self.indicator_mapping.insert(indicator, style);
@@ -317,6 +318,18 @@ impl LsColors {
         }
     }
 
+    fn add_suffix_entry(
+        &mut self,
+        suffix: FileNameSuffix,
+        style: Option<Style>,
+        priority: Priority,
+    ) {
+        // We use `to_ascii_lowercase` here to normalize suffix to use as keys.
+        let suffix_normalized = suffix.to_ascii_lowercase();
+        let suffix_mapping_entry = self.suffix_mapping.entry(suffix_normalized).or_default();
+        suffix_mapping_entry.push(suffix, style, priority);
+    }
+
     /// Get the ANSI style for a given path.
     ///
     /// *Note:* this function calls `Path::symlink_metadata` internally. If you already happen to
@@ -444,27 +457,42 @@ impl LsColors {
 
     /// Get the ANSI style for a string. This does not have to be a valid filepath.
     pub fn style_for_str(&self, file_str: &str) -> Option<&Style> {
+        let file_str_len = file_str.len();
         let file_str_normalized = file_str.to_ascii_lowercase();
-        let mut last_matched_style = None;
-        let mut last_matched_len = 0;
-        for (suffix, style) in self.suffix_mapping.mappings.iter() {
-            let suffix_len = suffix.len();
-            if last_matched_len > suffix_len {
-                continue;
-            }
-            let suffix_normalized = suffix.to_ascii_lowercase();
-            let case_sensitive = self
-                .suffix_mapping
-                .case_sensitive
-                .contains(&suffix_normalized);
-            if case_sensitive {
-                if file_str.ends_with(suffix) {
-                    last_matched_len = suffix_len;
-                    last_matched_style = style.as_ref();
+        let mut last_matched_style: Option<&Style> = None;
+        let mut last_matched_priority = 0;
+        for (suffix_normalized, suffix_mapping_entry) in self.suffix_mapping.iter() {
+            // check if there is an entry for the suffix
+            if file_str_normalized.ends_with(suffix_normalized) {
+                // check whether the entry is marked as case sensitive or not
+                if suffix_mapping_entry.case_sensitive {
+                    // then we need to check whether there is a suffix that exactly
+                    // matches in different variants.
+                    // we could do that by finding the suffix variant that appears in the filename,
+                    let file_str_suffix = &file_str[file_str_len - suffix_normalized.len()..];
+                    // and use that to find the corresponding style.
+                    if let Some((style, priority)) =
+                        suffix_mapping_entry.variants.get(file_str_suffix)
+                    {
+                        if *priority >= last_matched_priority {
+                            last_matched_style = style.as_ref();
+                            last_matched_priority = *priority;
+                        }
+                    }
+                } else {
+                    // case doesn't matter, that means every variant has
+                    // the same style so we could just take the first one.
+                    let (style, priority) = suffix_mapping_entry
+                        .variants
+                        .values()
+                        .next()
+                        // it okay to unwrap here, because there should exist atleast one variant.
+                        .unwrap();
+                    if *priority >= last_matched_priority {
+                        last_matched_style = style.as_ref();
+                        last_matched_priority = *priority;
+                    }
                 }
-            } else if file_str_normalized.ends_with(&suffix_normalized) {
-                last_matched_len = suffix_len;
-                last_matched_style = style.as_ref();
             }
         }
         last_matched_style
@@ -595,17 +623,34 @@ mod tests {
     fn style_for_path_uses_correct_ordering() {
         let lscolors = LsColors::from_string("*.foo=01;35:*README.foo=33;44");
 
+        // dummy.foo matches to *.foo without getting overriden.
         let style_foo = lscolors.style_for_path("some/folder/dummy.foo").unwrap();
         assert_eq!(FontStyle::bold(), style_foo.font_style);
         assert_eq!(Some(Color::Magenta), style_foo.foreground);
         assert_eq!(None, style_foo.background);
 
+        // README.foo matches to *README.foo by overriding *.foo
         let style_readme = lscolors
             .style_for_path("some/other/folder/README.foo")
             .unwrap();
         assert_eq!(FontStyle::default(), style_readme.font_style);
         assert_eq!(Some(Color::Yellow), style_readme.foreground);
         assert_eq!(Some(Color::Blue), style_readme.background);
+
+        let lscolors = LsColors::from_string("*README.foo=33;44:*.foo=01;35");
+
+        let style_foo = lscolors.style_for_path("some/folder/dummy.foo").unwrap();
+        assert_eq!(FontStyle::bold(), style_foo.font_style);
+        assert_eq!(Some(Color::Magenta), style_foo.foreground);
+        assert_eq!(None, style_foo.background);
+
+        // README.foo matches to *.foo because *.foo overrides *README.foo
+        let style_readme = lscolors
+            .style_for_path("some/other/folder/README.foo")
+            .unwrap();
+        assert_eq!(FontStyle::bold(), style_readme.font_style);
+        assert_eq!(Some(Color::Magenta), style_readme.foreground);
+        assert_eq!(None, style_readme.background);
     }
 
     #[test]