Sort entries by primary reading

[khaitruong922]

• Add primary_reading param.
• When click on gloss link, set furigana as primary_reading if exists.
• Entries with reading matches primary_reading will be sorted to top.

Works correctly for Jitendex gloss link format. Don't know if there are any kind of dictionary format which will break this feature, since the reading is depended on the classes naming.

[codspeed-hq]

CodSpeed Performance Report

Merging #1497 will not alter performance

_{Comparing khaitruong922:search-reading (521cb1e) with master (9c5f824)}

Summary

✅ 2 untouched benchmarks

🆕 1 new benchmarks
⁉️ 1 (👁 1) dropped benchmarks

Benchmarks breakdown

	Benchmark	master	khaitruong922:search-reading	Change
👁	Translator.prototype.findTerms - (n=43)	154.8 ms	N/A	N/A
🆕	Translator.prototype.findTerms - (n=45)	N/A	157.8 ms	N/A

[khaitruong922]

Ready for review

• Extract furigana from gloss link and add primary_reading search param

• Sort entries which match primary_reading to top

[stephenmk]

Thank you for working on this. It looks good.

Works correctly for Jitendex gloss link format. Don't know if there are any kind of dictionary format which will break this feature, since the reading is depended on the classes naming.

I don't like this approach. I think it would be better to rely on the dictionary data to specify the ?primary_reading= parameter. I don't think it's a good idea to try to parse the primary reading from the furigana. This increases the code complexity and could lead to unexpected behavior. It's possible that someone may want to manually specify a ?primary_reading= parameter that is not equivalent to the furigana readings within the link.

In other words, I would remove this block of code:

yomitan/ext/js/display/structured-content-generator.js

Lines 459 to 481 in 051a2f0

	if (internal) {
	let query = '';
	if (href.length > 1) {
	let hasFurigana = false;
	let reading = '';
	for (const childNode of text.childNodes) {
	if (childNode instanceof HTMLElement) {
	const furigana = childNode.querySelector('.gloss-sc-rt')?.textContent;
	if (furigana && furigana.length > 0) {
	reading += furigana;
	hasFurigana = true;
	}
	} else {
	reading += childNode.textContent ?? '';
	}
	}
	query = href;
	if (reading.length > 0 && hasFurigana) {
	query += `&primary_reading=${reading}`;
	}
	}
	href = `${location.protocol}//${location.host}/search.html${query}`;
	}

[Kuuuube]

I think it would be better to rely on the dictionary data to specify the ?primary_reading= parameter.

Agreed.

[khaitruong922]

The only place that user could specify the primary_reading is the search page, by modifying the url. I don't think this is the main use case, and no user will be aware of it.

The main purpose of this PR is to extract the reading from furigana, then sort the terms with that reading on top. For example, suppose that my dictionary has 縁 (えん) above 縁 (ふち). When I click on a word link 縁 (ふち), the entry 縁 (ふち) should be above 縁 (えん).

As far as I know, only Jitendex has furigana for word link. I agree that the implementation is quite fragile, we can find better ways to handle this.

[khaitruong922]

I also agree with having the primary_reading specified inside the href field of Jitendex, then we can remove the extract furigana code.

[stephenmk]

I just published a new version of Jitendex that includes the primary_reading parameter in hyperlinks.

[khaitruong922]

Tested with new version of Jitendex. Works great!

My.Video.mp4

[MarvNC]

so much work needed to just pass variables around