Skip to content

Commit

Permalink
fix(search): Handle word searches with char codes below 0x3000 (#1607)
Browse files Browse the repository at this point in the history
Words like DVD-ROM were not matching due to the hyphen (represented by a char code below 0x3000) in the middle. In order to include, entries like +α, logic was removed from rikaicontent.ts that skipped lookups when the letter the mouse hovered over was not a normal Japanese char code. (This included other various ranges which didn't show additional impacts as of this PR.)

These changes are thought to have been added for performance reasons which are no longer relevant but if some problem does crop up we can solve it more narrowly and with better documentation at that time.

<details>
  <summary>See word entries affected by this change (122 unique words)</summary>

```ts
[
  "〇×",
  "〇×テスト",
  "+α",
  "○",
  "○×",
  "○×テスト",
  "○○",
  "◎",
  "△",
  "※印",
  "2×4工法",
  "2進-5進",
  "4°",
  "CD-DA",
  "CD-R",
  "CD-ROM",
  "COVID-19",
  "DVD-R",
  "DVD-RAM",
  "DVD-ROM",
  "HANA-BI",
  "Hα線",
  "J-POP",
  "K-1",
  "K-POP",
  "K-T境界",
  "RF-ID",
  "UTF-8",
  "Wi-Fi",
  "ウラン-238",
  "プラスα",
  "マイクロRNA",
  "マッチョ♀ウーマン",
  "Α",
  "Β",
  "Γ",
  "Γ分布",
  "Δ",
  "Ε",
  "Ζ",
  "Η",
  "Θ",
  "Θ関数",
  "Ι",
  "Κ",
  "Λ",
  "Μ",
  "Ν",
  "Ξ",
  "Ο",
  "Π",
  "Ρ",
  "Σ",
  "Τ",
  "Υ",
  "Φ",
  "Χ",
  "Ψ",
  "Ω",
  "α",
  "α-helix",
  "α-ヘリックス",
  "α-メチルトリプタミン",
  "α-リノレン酸",
  "αらせん",
  "αメチルトリプタミン",
  "αリノレン酸",
  "α化",
  "α化米",
  "α線",
  "α波",
  "α分類学",
  "α米",
  "α崩壊",
  "α螺旋",
  "α粒子",
  "β",
  "β-カロチン",
  "β-カロテン",
  "βカロチン",
  "βカロテン",
  "βテスト",
  "βバージョン",
  "β線",
  "β波",
  "β版",
  "β崩壊",
  "β粒子",
  "γ",
  "γ-オリザノール",
  "γアミノ酪酸",
  "γリノレン酸",
  "γ線",
  "γ分布",
  "γ崩壊",
  "δ",
  "ε",
  "ζ",
  "η",
  "θ",
  "θ理論",
  "ι",
  "κ",
  "λ",
  "μ",
  "μ粒子",
  "ν",
  "ξ",
  "ο",
  "π",
  "π中間子",
  "π電子",
  "ρ",
  "σ",
  "σ電子",
  "τ",
  "τ粒子",
  "υ",
  "φ",
  "χ",
  "ψ",
  "ω"
]
```
This list of words was collected by running this code:

```ts
const uniqueWords = [
  ...new Set(
    rcxDict.wordDict
      .trim()
      .split("\n")
      .map((line) => line.split(" ")[0])
  ),
];
const wordsWithCharCodesBelow0x3000 = uniqueWords.filter((word) =>
  [...word].some((char) => char.charCodeAt(0) < 0x3000)
);
console.log(
  "wordsWithCharCodesBelow0x3000",
  JSON.stringify(wordsWithCharCodesBelow0x3000, null, 2)
);
```
</details>

I wrote snapshot tests to make sure none of the words without character codes below 0x3000 were affected by this change. I didn't include those tests in this pull request because they take a while to run (since there are over 200,000 entries in the dictionary) and they make this pull request bigger. I can add those tests in a separate pull request if we think that's a good idea so we can verify diffs of word searches for all words in the dictionary. This would give us more confidence whenever we change the wordSearch function because it shows all the diffs for each search result in the dictionary and it would allow us to drastically simplify the wordSearch function in the future with confidence that we aren't breaking of the previous behavior.

I also wrote code to do a check to make sure all words in the dictionary are searchable and found there is only a 1 unsearchable word in the dictionary now:

```ts
const uniqueWords = [
  ...new Set(
    rcxDict.wordDict
      .trim()
      .split("\n")
      .map((line) => line.split(" ")[0])
  ),
];
const unsearchableWords = uniqueWords.filter(
  (word) =>
    !rcxDict
      .wordSearch(word, false)
      ?.data.find((data) => data.entry.startsWith(word + " "))
);
console.log("unsearchableWords", unsearchableWords); // Logs ['~']
```

So this means after this pull request gets merged, the only unsearchable word entry in the dictionary is "~ [にょろ] /(n) tilde/wave dash/".

This word is a single character with a character code of 65374, which is 0xff5e in hexadecimal (which is above 0x3000, which explains why this pull request doesn't handle this word). After searching through the code for this number 0xff5e, it looks like there's some code that handles this character code that needs to be modified slightly to account for this word in the dictionary. That can be handled in a subsequent pull request to keep this pull request short and simple.

Fixes: #1453
  • Loading branch information
bryanjenningz committed Jun 7, 2023
1 parent 7023f15 commit f94efb7
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 18 deletions.
4 changes: 0 additions & 4 deletions extension/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,10 +348,6 @@ class RcxDict {
continue;
}

if (u <= 0x3000) {
break;
}

// full-width katakana to hiragana
if (u >= 0x30a1 && u <= 0x30f3) {
u -= 0x60;
Expand Down
14 changes: 0 additions & 14 deletions extension/rikaicontent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -848,20 +848,6 @@ class RcxContent {
}
}

//
if (
isNaN(u) ||
(u !== 0x25cb &&
(u < 0x3001 || u > 0x30ff) &&
(u < 0x3400 || u > 0x9fff) &&
(u < 0xf900 || u > 0xfaff) &&
(u < 0xff10 || u > 0xff9d))
) {
this.clearHi();
this.hidePopup();
return -2;
}

// selection end data
const selEndList: { node: CharacterData; offset: number }[] = [];
const text = this.getTextFromRange(rp, ro, selEndList, 13 /* maxlength*/);
Expand Down
6 changes: 6 additions & 0 deletions extension/test/data_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,11 @@ describe('data.ts', function () {
rcxDict.wordSearch('ぼんです', /* doNames= */ false)?.data
).to.not.include.something.like({ entry: /^凡打 .*/ });
});

it('should return results for words with punctuation', function () {
expect(
rcxDict.wordSearch('DVD-ROM', /* doNames= */ false)?.data
).to.include.something.like({ entry: /^DVD-ROM .*/ });
});
});
});
16 changes: 16 additions & 0 deletions extension/test/rikaicontent_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,22 @@ describe('RcxContent', function () {
});
});

it('triggers xsearch message when above non-Japanese text', function () {
const clock = sinon.useFakeTimers();
const span = insertHtmlIntoDomAndReturnFirstTextNode(
'<span>+αtest</span>'
) as HTMLSpanElement;

triggerMousemoveAtElementStart(span);
// Tick the clock forward to account for the popup delay.
clock.tick(1);

expect(chrome.runtime.sendMessage).to.have.been.calledWithMatch({
type: 'xsearch',
text: '+αtest',
});
});

describe('inside text area', function () {
it('triggers xsearch message when above Japanese text', function () {
const clock = sinon.useFakeTimers();
Expand Down

0 comments on commit f94efb7

Please sign in to comment.