Issue Description:
When sdcv found multiple items, whatever your choice is, sdcv will add
double the current history entries to history file. For example, if
current history is "a\nb", you search akjk and there's multiple results,
whatever you choose, even -1, after this is done, the history file will
be rewritten to "a\nb\nakjk\na\nb", note here \n is newline character.
So if you have 500 lines of history, you search akjk and there's
multiple results, you choose -1, after done there's 1001 lines of
history.
How to reproduce:
You can download this dictionary file
https://github.com/skywind3000/ECDICT/releases/download/1.0.28/ecdict-stardict-28.zip
and put into your dictionary directory, on Arch, with AUR, you can
install from https://aur.archlinux.org/packages/stardict-ecdict. Then,
make sure you also add the dictionary name to ~/.config/sdcv_ordering if
you have one. Add some lines to your history file if you do not have.
Then search for "akjk" with sdcv, e,g, `sdcv akjk`, then it will prompt
you to choose, you can choose -1, then ctrl-d to exit. Expected result
is history file add akjk at the end. Actual result is history file now
contain original content + akjk + original content duplicate as I
described in the issue description.
Fix and reasons:
I'm a hobbyist and I'm not a professional, I haven't use C++ for years
so many of my writings is very likely wrong. After some trail and error,
I found that call read_history() multiple times will add repeat
histories to history lists. In the commit d2327e2, a new IReadLine
object is created (note name changes, also note I know this description
of a IReadLine object is wrong). Here's a permalink:
49c8094b53/src/libwrapper.cpp (L418).
The problem of this new IReadLine object `choice_readline` is it called
read_history() again from ./src/readline.cpp constructor, because
there's already a IReadLine object `io` constructed at ./src/sdcv.cpp.
When read_history() is called twice, there's a "history lists" read from
history file first then append from history file again, so when you
destruct IReadLine object with write_history() in ./src/readline.cpp,
the history file contain duplicate content after write. Here are
permalinks:
49c8094b53/src/readline.cpp (L88),
and
49c8094b53/src/readline.cpp (L94).
So my fix is to just to use `io` IReadLine object and not to create a
new `choice_readline` object.
Misc:
During my trial and errors process, I made an example code to show
read_history()'s weird behavior. I did not dig deeper, I just guess
maybe there's some kind of werid history list as mentioned in
https://tiswww.cwru.edu/php/chet/readline/history.html#History-List-Management.
Here's the example code a.c, note you need to include stdio.h,
readline/history.h, and readline/readline.h. I did not include them here
because commit message seems will make them comment.
```c
...
int main (void)
{
rl_readline_name="learn_readline";
using_history();
read_history("/home/xyz/test/learn_readline/history.txt");
write_history("/home/xyz/test/learn_readline/history2.txt");
{
rl_readline_name="learn_readline";
using_history();
read_history("/home/xyz/test/learn_readline/history.txt");
write_history("/home/xyz/test/learn_readline/history3.txt");
}
write_history("/home/xyz/test/learn_readline/history4.txt");
return 0;
}
```
Here's the content of history.txt:
```
a
b
```
After build and run, as you can guess, history2.txt is same as
history.txt. But history3.txt and history4.txt content are:
```
a
b
a
b
```
Signed-off-by: Xiao Pan <xyz@flylightning.xyz>
Make sure we return all of the relevant results, even in cases with
lots of results (larger than ENTR_PER_PAGE in the offset index) and
where you have a synyonym and headword present for the same word.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
They aren't used at all by scdv, and thus aren't tested (meaning that
adaptions to the core lookup algorithms can be complicated because these
methods use them but aren't tested so there's no real way of knowing if
a change has broken the methods or not).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Previously, we would just return the first entry we found that matched
the requested word. This causes issues with dictionaries that have lots
of entries which can be found using the same search string. In these
cases, the user got a completely arbitrary word returned to them rather
than the full set.
While this may seem strange, this is incredibly commonplace in Japanese
and likely several other languages. In Japanese:
* When written using kanji, the same string of characters could refer
to more than one word which may have a completely different meaning.
Examples include 潜る (くぐる、もぐる) and 辛い (からい、つらい).
* When written in kana, the same string of characters can also refer to
more than one word which is written using completely different kanji,
and has a completely different meaning. Examples include きく
(聞く、効く、菊) and たつ (立つ、建つ、絶つ).
In both cases, these are different words in every sense of the word, and
have separate headwords for each in the dictionary. Thus in order to be
completely useful for such dictionaries, sdcv needs to be able to return
every matching word in the dictionary.
The solution is conceptually simple -- return a set containing the
indices rather than just a single index. Since every list we search is
sorted (to allow binary searching), once we find one match we can just
walk backwards and forwards from the match point to find the entire
block of matching terms and add them to the set in linear time. A
std::set is used so that we don't return duplicate results needlessly.
This solution was in practice a bit more complicated because .otf cache
files require a bit more fiddling, and also the ->lookup methods are
used by some callers to find the next entry if no entry was found. But
on the whole it's not too drastic of a change from the previous setup.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Due to the lack of deinflection support in StarDict, users might want to
be able to create a list of possible deinflections and search each one
to see if there is a dictionary entry for that deinflection.
Being able to do this in one sdcv invocation is far more preferable to
calling sdcv once for each candidate due to the performance cost of
doing so. The most obvious language that would benefit from this is
Japanese, but I'm sure other folks would prefer this.
In order to make this use-case better supported -- try to look up every
word in the provided list of words before existing with an error if any
one of the words failed to be looked up.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
the previous approachonly works as long as locales are actually sane
(i.e., the test only passes if you *actually* have the ru_RU.KOI8-R
locale built, which the CI doesn't).
