-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: fix compare method and uppercase word search bug #28
Conversation
terasum
commented
May 23, 2021
- fix uppercase search comparing bug
- add case sensitive options
f881fde
to
221f6ff
Compare
1. fix uppercase comparing bug 2. add case sensitive options 3. git rename the uppercase Mdict.js to mdict.js
@danjame 你好,我这边提供了解决你的问题的另一种方法,这种方法可以精确匹配大小写词,这种方式通过修改二分查找的比较函数,更加精确效率也更高,方便的话,可以帮忙测试一下吗? |
@terasum 你好,对这个代码我测试了一下:
原来的版本(4.0.7版本及以前)只要把 |
@danjame 也就是说,和预期是相反的咯?
就是 |
@danjame 我原本测试的方式是打包两个
|
@danjame 你描述的:
是正确的预期结果 而:
如果这种情况下,是否还是 |
@terasum 对的,我上面描述有点问题。 所以词库本身的 |
@danjame 现在有这样的问题: |
我看了一下应该是这部分代码的问题:
想问下你描述的:
是怎么做到的呢? |
是的,你的理解没错 |
@danjame 抱歉我这边用的是mac环境,没有办法运行MDXBBuilder,方便给我发一份 |
@danjame 我测试了你提供的词典,确实没有办法搜索到大写的词,但是我打印出特定
|
@danjame 我查询的词是 |
@terasum 我重新打包了词典,你重新下载试试 |
@danjame 新打包的词典依旧没有大写词,我试了一下原来的词典
|
新打包的词典
|
|
@danjame 原来如此,我根据之前的那个
这两种顺序不统一导致在二分查找的时候词的比较会出现问题:
因为 第2种情况:
因为大写字母在最前面的 我觉得 4.0.7 之所以能够查出来是因为大写词是在最前面的,它先找到了大写词,如果大写词在最后面应该也是找不到的 |
@terasum 我对比了一下,发现在打包 mdx 文件的时候:
所以我觉得你可以根据 |
@terasum 而 4.0.7 和之后的版本出现相反的预期结果,也就是因为这个 |
@danjame 好的,我试一下你说的这种方法 |
@danjame 我已经在这个分支上改了一版,代码已经上传,我这边测试没有问题了,可以辛苦你那边再帮忙看看你吗? |
代码不小心被我合并到master了,现在两个分支(master和fix-uppercase)都是 4.0.8 的代码了 |
@terasum 可以,测试没有问题。 |
@danjame 好的,我发布一个新的 NPM 包 |