openSUSE / mentoring

The openSUSE Developer Mentoring Program
http://101.opensuse.org
60 stars 49 forks source link

Merge cloud input feature into ibus-libpinyin #117

Closed hillwoodroc closed 3 years ago

hillwoodroc commented 4 years ago

Project Title: Merge cloud input feature into ibus-libpinyin

Description: We had a GSoC project about integration cloud pinyin in ibus-libpinyin at 2018. A student got this project and achieved this feature. The code is at https://github.com/openSUSE/ibus-libpinyin. But it was not merged to upstream due to some special demands and methods. We hope somenone can finally finish this project and merge this feature to upstream. What is ibus? The full name of ibus is Input Bus Introduction. It is the default input method framework in SUSE Linux Enterprise and openSUSE GNOME Desktop. ibus-pinyin and ibus-libpinyin work on ibus. ibus and its input method are very important to Asia users of SUSE Linux Enterprise and openSUSE.

Deliverable: Please work on github. We will test you code. If it's stable, we can help you to merge it to upstream.

Mentor: @epico @qiangzhao @hillwoodroc

Skills: C, C++, python3, autotools.

Skill Level: Medium/Hard

Get started:

Inokinoki commented 4 years ago

Hi there, I've succeeded in building the mentioned repo with the cloud input feature. Could you please tell me, what are those special demands and methods that blocked its merge to the upstream? I may be able to do some work :)

hillwoodroc commented 4 years ago
  1. Please use some techique to verify the data returned from third-party web site
  2. Please use asynchronized communication with web site
  3. Please use glib/gnome library to connect to web site
Inokinoki commented 4 years ago

Great appreciation!

For the 3rd one, AFAIK, libsoup is already a lib maintained by Gnome.

I've suffered some crash during the test of sync requests to Baidu cloud input source, with Google source it works well. Since their responses are both JSON, can I use the JSON parser lib in Glib to do some validations and to avoid the crash? (rather than the string splitting solution in the current version)

hillwoodroc commented 4 years ago

I think it is feasible. But I don't know the other two mentors how think it. I suggest you to email us.

epico commented 4 years ago

Yeah, GLib JSON library is one option.

For me, public discussion is okay; other students can also see the information.

qiangzhao commented 4 years ago

Yeah, GLib JSON library is one option.

For me, public discussion is okay; other students can also see the information.

Me too!

epico commented 4 years ago

Here are some partial cloud input TODO list from me.

  1. With GLib JSON library, I think it is better to check the data member in JSON a. if possible please check as many data field as possible;

  2. Please use asynchronized communication with web site a. insert one cloud symbol before web site returning data; b. replace the cloud symbol with the data returned from web site; c. what will happen if multiple web request is sent?

Sorry, I will try to find some time to update the detailed TODO list later.

Inokinoki commented 4 years ago

I've done an initial json validation: https://github.com/Inokinoki/ibus-libpinyin/blob/5bd41ddfd08a85d0857a20ba8957b67d62928115/src/PYPCloudCandidates.cc#L245 Adding more.

For the async request, I'm still investigating how it can work to replace the candidates.

I've already tested the case to send multiple web request in async mode. Maybe it's possible to add a send timestamp as a part of user data, drop the other replies and use the latest one.

I'm also wondering how it can act when a request fails(error request, incomplete response, invalid format). Will it be intelligent to remove the candidates?

epico commented 4 years ago

Could you organize the code into two class for Google and Baidu Parser? It is okay to put the two class in PYPCloudCandidates.cc.

I think timestamp may be dropped by web site, but not very sure. Maybe you could compare the user input returned from the web site.

If the web request fails, we need to notice the user with some symbol. Then we can get issue report from user.

Inokinoki commented 4 years ago

Could you organize the code into two class for Google and Baidu Parser? It is okay to put the two class in PYPCloudCandidates.cc.

I think timestamp may be dropped by web site, but not very sure. Maybe you could compare the user input returned from the web site.

Done: https://github.com/Inokinoki/ibus-libpinyin/blob/5ccf75beaf43a388e78fb3d755e3114a2bbf05aa/src/PYPCloudCandidates.cc#L53

I'm planning to handle the failures and add symbol in CloudCandidates::processCloudResponse. After this, the functionalities should have been completed.

And the file is nearly ready to be reviewed. Don't hesitate if you have any advise.

Then, I'm going to do something with setup ui, to make cloud input more configurable.

Inokinoki commented 4 years ago

What do you think if we have a cloud unicode icon for the candidates like this: enhanced.m_display_string = "☁...".

It can be removed while the candidate is selected, so that the cloud unicode character will not be on the screen.

hillwoodroc commented 4 years ago

I agree.

Inokinoki commented 4 years ago

Done.

Screenshot_20200307_113521

I'm making a pull request for review: https://github.com/libpinyin/ibus-libpinyin/pull/228

And I will create another branch to squash the commits for a clean commit after review.

hillwoodroc commented 4 years ago

It can not work on double pinyin mode. 2020-03-09 11-05-28屏幕截图 2020-03-09 11-04-36屏幕截图

epico commented 4 years ago

Here are more suggestions coming. ;)

  1. Please add one option for some time delay before sending the web request a. I mean if the user is inputting, better delay the web request

  2. Please cancel the web request, if the user changes the input after web request sent a. Maybe soup_session_cancel_message is helpful

For double pinyin, I can provide some code snippets by using libpinyin later.

Feel free to ping me, if you have any questions.

Inokinoki commented 4 years ago

Here are more suggestions coming. ;)

1. Please add one option for some time delay before sending the web request
   a. I mean if the user is inputting, better delay the web request

Good I idea, I was considering it. But what is the best delay? Should it be better to make it configurable?

2. Please cancel the web request, if the user changes the input after web request sent
   a. Maybe soup_session_cancel_message is helpful

Yes, it could be helpful! I didn't realize that can be cancelled as I thought in the sync request flow. I agree that this can bring better experience.

Inokinoki commented 4 years ago

It can not work on double pinyin mode. 2020-03-09 11-05-28屏幕截图 2020-03-09 11-04-36屏幕截图

Fixed. image

epico commented 4 years ago

Yes, I think the time delay should be configurable.

Maybe only configurable in gsettings is also okay.

hillwoodroc commented 4 years ago

I give you two suggestions on the pull request https://github.com/libpinyin/ibus-libpinyin/pull/228/files

Inokinoki commented 4 years ago

Thanks for all your suggestions!

I've submitted a draft proposal for this idea. Don't hesitate if you have any question or advice :)