search

sienori / simple-translate

WebExtensions for translating text on web pages
http://simple-translate.sienori.com/
Mozilla Public License 2.0
1.12k stars 126 forks source link

fixing terms issue #507

Closed ttrasn closed 1 year ago

ttrasn commented 1 year ago

fix issue : https://github.com/sienori/simple-translate/issues/505 and https://github.com/sienori/simple-translate/issues/506

sienori commented 1 year ago

Thanks for the PR! It seems that many unnecessary differences are being generated. Could you please correct them to the minimum difference?

ttrasn commented 1 year ago

sure @sienori , I'll update it now

ttrasn commented 1 year ago

Hi @sienori , please check it

pouyacodes commented 1 year ago

Thanks for the PR!

It would be nice if we don't just check for undefined. if for whatever reason the e.terms has falsy values then return an empty string.

e.terms ? e.terms.join(', ') : ''

or it would be better if we don't set resultData.candidateText at all if there is no terms/entry:

if (result.data.dict) {
    const dict = result.data.dict.terms || result.data.dict.entry
    resultData.candidateText = dict ? getCandidateText(dict) : ""
}

Obviously getCandidateText is not implemented and this is just my idea. I think we have to check both terms/entry in getCandidateText(dict) for some backward compatibility.

ttrasn commented 1 year ago

I believe checking for undefined is enough. for other possible values the old code should work fine.

sienori commented 1 year ago

Thanks!