Skip to content

Conversation

@taooceros
Copy link
Member

close #229

@jjw24 jjw24 added the review in progress Indicates that a review is in progress for this PR label Dec 10, 2020
{
var result = WordsHelper.GetPinyin(content, ";");
result = GetFirstPinyinChar(result) + result.Replace(";", "");
var result = WordsHelper.GetPinyin(content, " ");
Copy link
Member

@jjw24 jjw24 Dec 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this change, when i pinyin search '你好' for 'nihao' it doesnt hit as it return splitted up 'ni hao'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anybody would use Chinese to search for pinyin like characters.... If both are chinese, it will be fine because the string to compare will also contain space.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think anybody would use Chinese to search for pinyin like characters.... If both are chinese, it will be fine because the string to compare will also contain space.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so we dont need to worry about chinese to pinyin.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because the reason using pinyin is to make searching faster by reducing the process of typing Chinese, since typing Chinese require extra typing.

private string GetFirstPinyinChar(string content)
{
return string.Concat(content.Split(';').Select(x => x.First()));
return string.Concat(content.Split(' ').Select(x => x.First()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to add remove empty entries otherwise it will throw because you are doing First():
return string.Concat(content.Split(' ', StringSplitOptions.RemoveEmptyEntries).Select(x => x.First()));

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it isn't an issue before because the translated query won't contain something like ;;, but since we now use space to split. I will take a look on the translated query when the original query contain both Chinese and letter and space.

@jjw24
Copy link
Member

jjw24 commented Dec 11, 2020

hmm i think this change solves acronym pinyin search but due to the way result could be constructed, it's not hitting query like 'nihao' or 'ni hao' for my test case, the score is too low for my test example below:
image

although i dont know if what i am using is a valid test example. I dont use a Chinese OS system, can you give me an example to test

@taooceros
Copy link
Member Author

hmm i think this change solves acronym pinyin search but due to the way result could be constructed, it's not hitting query like 'nihao' or 'ni hao' for my test case, the score is too low for my test example below:
image

although i dont know if what i am using is a valid test example. I dont use a Chinese OS system, can you give me an example to test

I believe the weird situation of your testing is due to that the space will be added to all character used for translation due to the Pinyin library characteristic. The situation also exist in current situation, though not significantly which only affect the acronym part. I will contact the author to ask whether he is able to add the split character only to Chinese character instead of all character. Before his response, I will try to find alternative solution.

@taooceros
Copy link
Member Author

@jjw24 I manually set the space instead of using the method provided by the library. It seems works fine, now.

taooceros and others added 3 commits December 11, 2020 22:20
@jjw24 jjw24 added enhancement New feature or request and removed review in progress Indicates that a review is in progress for this PR labels Dec 14, 2020
@jjw24 jjw24 merged commit 0928b05 into Flow-Launcher:dev Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request kind/i18n

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Space between Pinyin translation

3 participants