Repository: PyThaiNLP/nlpo3 (language: python)

Issue:
[Python] load_dict() can't deal with blank line
If a dictionary file has a blank file, will get this error:
```
thread '<unnamed>' panicked at 'range end index 4 out of range for slice of length 0',
/nlpo3-1.3.1/src/fixed_bytes_str/four_bytes.rs:296:30
```
[Python] load_dict() can't deal with blank line
If a dictionary file has a blank file, will get this error:
```
thread '<unnamed>' panicked at 'range end index 4 out of range for slice of length 0',
/nlpo3-1.3.1/src/fixed_bytes_str/four_bytes.rs:296:30
```


Discussion / hints:
Looks like `read_line()` in `create_dict_trie()` still retain new line character. This may make `is_empty()` useless.
Looks like `read_line()` in `create_dict_trie()` still retain new line character. This may make `is_empty()` useless.

Fix patch:
diff --git a/README.md b/README.md
index a0181ac..03df2fe 100644
--- a/README.md
+++ b/README.md
@@ -77,6 +77,10 @@ cargo build --release
 Check `target/` for build artifacts.
 
 
+## Development documents
+
+- [Notes on custom string](src/NOTE_ON_STRING.md)
+
 ## Issues
 
 Please report issues at https://github.com/PyThaiNLP/nlpo3/issues
diff --git a/src/NOTE_ON_STRING.md b/src/NOTE_ON_STRING.md
index 3cba864..b0b7332 100644
--- a/src/NOTE_ON_STRING.md
+++ b/src/NOTE_ON_STRING.md
@@ -19,4 +19,6 @@ with \x00 one time.
 For "space" in regex, it is padded with \x00\x00\x00.
 
 
-[More on UTF-8](https://en.wikipedia.org/wiki/UTF-8)
+## References
+- [Rust String indexing and internal representation](https://doc.rust-lang.org/book/ch08-02-strings.html#indexing-into-strings)
+- Read more about [UTF-8](https://en.wikipedia.org/wiki/UTF-8) at Wikipedia.
diff --git a/src/fixed_bytes_str/four_bytes.rs b/src/fixed_bytes_str/four_bytes.rs
index 451623e..e0c9318 100644
--- a/src/fixed_bytes_str/four_bytes.rs
+++ b/src/fixed_bytes_str/four_bytes.rs
@@ -1,3 +1,5 @@
+/// Functions dealing with a custom four-byte string.
+/// For more details, see src/NOTE_ON_STRING.md
 use std::{
     error::{self, Error},
     fmt::Display,
@@ -136,20 +138,43 @@ pub fn rfind_space_char_index(custom_text: &CustomStringBytesSlice) -> Option<us
     None
 }
 
+/// Check if a white space (including left-to-right and right-to-left marks)
 fn is_whitespace(custom_bytes: &CustomStringBytesSlice) -> bool {
     matches!(
         custom_bytes,
-        [0, 0, 0, 9]
-            | [0, 0, 0, 10]
-            | [0, 0, 0, 11]
-            | [0, 0, 0, 12]
-            | [0, 0, 0, 13]
-            | [0, 0, 0, 32]
-            | [0, 0, 194, 133]
-            | [0, 0xe2, 0x80, 0x8e]
-            | [0, 0xe2, 0x80, 0x8f]
-            | [0, 0xe2, 0x80, 0xa8]
-            | [0, 0xe2, 0x80, 0xa9]
+        [0, 0, 0, 9] // Character tabulation (HT) (\t) U+0009
+            | [0, 0, 0, 10] // Line feed (LF) (\n) U+000A
+            | [0, 0, 0, 11] // Line tabulation (VT) U+000B
+            | [0, 0, 0, 12] // Form feed (FF) (\f) U+000C
+            | [0, 0, 0, 13] // Carriage return (CR) (\r) U+000D
+            | [0, 0, 0, 32] // Space U+0020
+            | [0, 0, 194, 133] // Next line (NEL) U+0085
+            | [0, 0, 194, 160] // No-break space (NBSP) U+00A0
+            | [0, 0xe1, 0x9a, 0x80] // Ogham space mark U+1680
+            | [0, 0xe1, 0xa0, 0x8e] // Mongolian vowel separator U+180E
+            | [0, 0xe2, 0x80, 0x80] // En quad U+2000
+            | [0, 0xe2, 0x80, 0x81] // Em quad U+2001
+            | [0, 0xe2, 0x80, 0x82] // En space U+2002
+            | [0, 0xe2, 0x80, 0x83] // Em space U+2003
+            | [0, 0xe2, 0x80, 0x84] // Three-per-em space U+2004
+            | [0, 0xe2, 0x80, 0x85] // Four-per-em space U+2005
+            | [0, 0xe2, 0x80, 0x86] // Six-per-em space U+2006
+            | [0, 0xe2, 0x80, 0x87] // Figure space U+2007
+            | [0, 0xe2, 0x80, 0x88] // Punctuation space U+2008
+            | [0, 0xe2, 0x80, 0x89] // Thin space U+2009
+            | [0, 0xe2, 0x80, 0x8a] // Hair space U+200A
+            | [0, 0xe2, 0x80, 0x8b] // Zero width space (ZWSP) U+200B
+            | [0, 0xe2, 0x80, 0x8c] // Zero width non-joiner (ZWNJ) U+200C
+            | [0, 0xe2, 0x80, 0x8d] // Zero width joiner (ZWJ) U+200D
+            | [0, 0xe2, 0x80, 0x8e] // Left-to-right mark U+200E *(control character)
+            | [0, 0xe2, 0x80, 0x8f] // Right-to-left mark U+200F *(control character)
+            | [0, 0xe2, 0x80, 0xa8] // Line separator U+2028
+            | [0, 0xe2, 0x80, 0xa9] // Paragraph seperator U+2029
+            | [0, 0xe2, 0x80, 0xaf] // Narrow no-break space U+202F
+            | [0, 0xe2, 0x81, 0x9f] // Medium mathematical space U+205F
+            | [0, 0xe2, 0x81, 0xa0] // Word joiner U+2060
+            | [0, 0xe3, 0x80, 0x80] // Ideographic space U+3000
+            | [0, 0xef, 0xbb, 0xbf] // Zero width no-break space U+FEFF
     )
 }
 
@@ -230,9 +255,11 @@ pub trait FixedLengthCustomString<T: Sized + FixedLengthCustomString<T>> {
     fn get_original_string(&self) -> &[u8];
 }
 
-/// The content inside this string is a vector of bytes - ALWAYS with length % 4 == 0
+/// The content inside this string is a vector of bytes,
+/// ALWAYS with length % 4 == 0
 ///     
-/// Every character is a valid utf-8 encoded byte padded left with 0 to make every character takes 4 bytes.
+/// Every character is a valid utf-8 encoded byte padded left with 0
+/// to make every character takes 4 bytes.
 ///
 /// For example, Thai characters which use 3 bytes are represented by
 /// [0, valid_first_byte, valid_second_byte, valid_third_byte].
@@ -263,14 +290,13 @@ impl CustomString {
         let length = content.len() / BYTES_PER_CHAR;
         Self {
             content: Arc::new(content),
-
             start: 0,
             end: length,
             chars_content,
         }
     }
 
-    /// returns a sub-slice  from full content
+    /// Returns a sub-slice from full content
     pub fn raw_content(&self) -> &[u8] {
         self.content
             .as_slice()
@@ -297,16 +323,19 @@ impl CustomString {
 
     pub fn trim(&self) -> Self {
         let mut new_content: &[u8] = &self.content;
-        while is_whitespace(&new_content[0..BYTES_PER_CHAR]) {
+
+        while (new_content.len() > 0) && is_whitespace(&new_content[0..BYTES_PER_CHAR]) {
             // trim left
             new_content = &new_content[BYTES_PER_CHAR..];
         }
 
-        while is_whitespace(&new_content[(new_content.len() - BYTES_PER_CHAR)..]) {
-            // trim left
+        while (new_content.len() > 0)
+            && is_whitespace(&new_content[(new_content.len() - BYTES_PER_CHAR)..])
+        {
+            // trim right
             new_content = &new_content[..(new_content.len() - BYTES_PER_CHAR)];
-            // new_content.drain((self.content.len()-BYTES_PER_CHAR)..(self.content.len()));
         }
+
         let length = new_content.len() / BYTES_PER_CHAR;
 
         Self {
@@ -425,8 +454,8 @@ fn check_slice() {
 }
 
 #[test]
-fn test_byte() {
-    let long_text = [
+fn test_bytes() {
+    let text = [
         "ไต้หวัน (แป่ะเอ๋ยี้: Tâi-oân; ไต่อวัน) หรือ ไถวาน ",
         "(อักษรโรมัน: Taiwan; จีนตัวย่อ: 台湾; จีนตัวเต็ม: 臺灣/台灣; พินอิน: ",
         "Táiwān; ไถวาน) หรือชื่อทางการว่า สาธารณรัฐจีน (จีนตัวย่อ: 中华民国; ",
@@ -437,6 +466,16 @@ fn test_byte() {
         "ท้องที่ดังกล่าวเรียกรวมกันว่า \"พื้นที่ไต้หวัน\" (臺灣地區)\n",
     ]
     .join("");
-    let custom_string = CustomString::new(&long_text);
+    let custom_string = CustomString::new(&text);
     assert_eq!(custom_string.full_string_bytes_len() % 4, 0);
 }
+
+#[test]
+fn test_trim() {
+    assert_eq!(CustomString::new(" ").trim().is_empty(), true);
+    assert_eq!(CustomString::new("  ").trim().is_empty(), true);
+    assert_eq!(CustomString::new("\n").trim().is_empty(), true);
+    assert_eq!(CustomString::new("  \t\n ").trim().is_empty(), true);
+    assert_eq!(CustomString::new(" abc ").trim().chars_len(), 3);
+    assert_eq!(CustomString::new(" aก  ").trim().full_string_bytes_len(), 8); // 2 chars * 4 bytes
+}
diff --git a/src/tokenizer/dict_reader.rs b/src/tokenizer/dict_reader.rs
index f8fa8b8..5bf0705 100644
--- a/src/tokenizer/dict_reader.rs
+++ b/src/tokenizer/dict_reader.rs
@@ -20,9 +20,7 @@ pub fn create_dict_trie(source: DictSource) -> Result<Trie, Box<dyn Error>> {
                     let mut line = String::with_capacity(50);
                     let mut dict: Vec<CustomString> = Vec::with_capacity(600);
                     while reader.read_line(&mut line).unwrap() != 0 {
-                        if !line.is_empty() {
-                            dict.push(CustomString::new(&line));
-                        }
+                        dict.push(CustomString::new(&line));
                         line.clear();
                     }
                     dict.shrink_to_fit();
diff --git a/src/tokenizer/trie.rs b/src/tokenizer/trie.rs
index 61a51e5..3aaa173 100644
--- a/src/tokenizer/trie.rs
+++ b/src/tokenizer/trie.rs
@@ -126,9 +126,7 @@ impl Trie {
             root: TrieNode::new(),
         };
         for word in words.iter() {
-            if !word.is_empty() {
-                instance.add(word);
-            }
+            instance.add(word);
         }
         instance
     }
@@ -139,17 +137,21 @@ impl Trie {
 
     pub fn add(&mut self, word: &CustomString) {
         let stripped_word = word.trim();
-        self.words.insert(stripped_word.raw_content().into());
-        let current_cursor = self.root.borrow_mut();
-        current_cursor.add_word(stripped_word.raw_content());
+        if !stripped_word.is_empty() {
+            self.words.insert(stripped_word.raw_content().into());
+            let current_cursor = self.root.borrow_mut();
+            current_cursor.add_word(stripped_word.raw_content());
+        }
     }
 
     pub fn remove(&mut self, word: &CustomString) {
         let stripped_word = word.trim();
-        let stripped_word_raw = stripped_word.raw_content();
-        if self.words.contains(stripped_word_raw) {
-            self.remove_word_from_set(stripped_word_raw);
-            self.root.remove_word(stripped_word_raw);
+        if !stripped_word.is_empty() {
+            let stripped_word_raw = stripped_word.raw_content();
+            if self.words.contains(stripped_word_raw) {
+                self.remove_word_from_set(stripped_word_raw);
+                self.root.remove_word(stripped_word_raw);
+            }
         }
     }
 
diff --git a/src/tokenizer/trie_char.rs b/src/tokenizer/trie_char.rs
index 91e6177..275ce8d 100644
--- a/src/tokenizer/trie_char.rs
+++ b/src/tokenizer/trie_char.rs
@@ -101,9 +101,7 @@ impl TrieChar {
             root: TrieNode::new(),
         };
         for word in words.iter() {
-            if !word.is_empty() {
-                instance.add(word);
-            }
+            instance.add(word);
         }
         instance
     }
@@ -115,16 +113,20 @@ impl TrieChar {
 
     pub fn add(&mut self, word: &CustomString) {
         let stripped_word = word.trim();
-        self.words.insert(stripped_word.raw_content().into());
-        let current_cursor = self.root.borrow_mut();
-        current_cursor.add_word(&stripped_word);
+        if !stripped_word.is_empty() {
+            self.words.insert(stripped_word.raw_content().into());
+            let current_cursor = self.root.borrow_mut();
+            current_cursor.add_word(&stripped_word);
+        }
     }
 
     pub fn remove(&mut self, word: &CustomString) {
         let stripped_word = word.trim();
-        if self.words.contains(stripped_word.raw_content()) {
-            self.remove_word_from_set(&stripped_word);
-            self.root.remove_word(&stripped_word); // remove from node
+        if !stripped_word.is_empty() {
+            if self.words.contains(stripped_word.raw_content()) {
+                self.remove_word_from_set(&stripped_word);
+                self.root.remove_word(&stripped_word); // remove from node
+            }
         }
     }
 


Test patch:
diff --git a/tests/data/dict_with_empty_line.txt b/tests/data/dict_with_empty_line.txt
new file mode 100644
index 0000000..792245b
--- /dev/null
+++ b/tests/data/dict_with_empty_line.txt
@@ -0,0 +1,6 @@
+ปฏิวัติ
+ปฏิรูป
+ปฏิเสธ 
+
+ปฏิบัติ
+ ปฏิรูป 
diff --git a/tests/test_tokenizer.rs b/tests/test_tokenizer.rs
index 7e577e3..60915db 100644
--- a/tests/test_tokenizer.rs
+++ b/tests/test_tokenizer.rs
@@ -6,6 +6,14 @@ const SECOND_TEXT: &str =
     "อาชญากรรมทางการแพทย์.. หลอกลวงคนไข้ผ่าตัด ตัดหมอนรองข้อเข่าอำพราง รพ.กรุงเทพภูเก็ตปลอมเวชระเบียน ตอนที่๑.";
 const DEFAULT_DICT_PATH: &str = "/words_th.txt"; // relative to cargo
 
+#[test]
+fn test_dict_with_empty_line() {
+    const DICT_PATH: &str = "/tests/data/dict_with_empty_line.txt";
+    let mut relative_test_dict_path = env!("CARGO_MANIFEST_DIR").to_string();
+    relative_test_dict_path.push_str(DICT_PATH);
+    let _tokenizer = NewmmTokenizer::new(&relative_test_dict_path);
+}
+
 #[test]
 fn test_long_text_byte_tokenizer() {
     let mut relative_test_dict_path = env!("CARGO_MANIFEST_DIR").to_string();