chewing / chewing-editor

Cross platform chewing user phrase editor
https://chewing.im/
GNU General Public License v2.0
31 stars 52 forks source link

Phrase can be preserved when the "add" step in "modify" is failed. #149

Closed jeffreyhc closed 8 years ago

jeffreyhc commented 8 years ago

This will close #75 #109

When the "add" step is failed, the function will re-add the original phrase back.

This is my first pull request, wherever I can improve, please tell me.

jserv commented 8 years ago

Please improve your GIT commit messages.

jserv commented 8 years ago

The first line of GIT commit messages should be the summary of the change. Reference: http://chris.beams.io/posts/git-commit/

jeffreyhc commented 8 years ago

Thanks for your advice and reference, but I have some problem of the commit log... After I rewrite the commit log(use git rebase -i), I can't push new record to the master branch because of the diverged. Then I merge the record, and now the record becomes more complicated. Maybe I can re-post a new PR to solve this situation?

Chocobo1 commented 8 years ago

I can't push new record to the master branch because of the diverged

use git push -f

Maybe I can re-post a new PR to solve this situation?

Please don't. And remove the Merge branch .... commit.

jeffreyhc commented 8 years ago

Sorry I don't know how to remove that commit, should I rebase to the fbe5896 then git push -f ?

Thanks for your advice.

Chocobo1 commented 8 years ago

Sorry I don't know how to remove that commit,

search for "git remove commit". I'll let you learn on your own.

should I rebase to the fbe5896 then git push -f ?

rebase should be revert.

jeffreyhc commented 8 years ago

@Chocobo1 After I finish the edit, should I make a new commit or combine this to previous commit?

Chocobo1 commented 8 years ago

@Chocobo1 After I finish the edit, should I make a new commit or combine this to previous commit?

just amend it.

jserv commented 8 years ago

@jeffreyhc : Can you rework the patch upon the review? Also, please read the article How to Write a Git Commit Message carefully.

jeffreyhc commented 8 years ago

@jserv : sorry I don't fully understand. I have modified the code other developers advised before, and referenced that article to write commit message. I think next I can study for the use frequency of userphrase in libchewing and try to integrate use frequency to the re-add userphrase mechanism. But for this commit, I have no idea what I can do further.

jserv commented 8 years ago

@jeffreyhc : I could not find the revised patch here. Ensure the submission of your changes.

jeffreyhc commented 8 years ago

@Chocobo1 Hi, I've undone the style change. Does there anything I need to correct?

jserv commented 8 years ago

@jeffreyhc The statement, "Repair the mechanism of modify phrase in UserphraseView", is meaningless. You should summarize the changes and motivation in the first line of GIT commit messages.

jeffreyhc commented 8 years ago

@jserv How about "Fix the modify mechanism by re-adding deleted phrase"

jserv commented 8 years ago

I would write down as following:

Reclaim the phrase during "add" manipulation

Close #75
jeffreyhc commented 8 years ago

Shall I still keep the body of the commit message? How about modify the message to:

Reclaim the phrase during "add" manipulation

Close #75

The phrase can be preserved when the "add" step in "modify" is failed. After a new phrase is added, the selection of the original phrase in chewing-editor seems to be reset, then the remove() in UserphaseView will lost the remove target. So I check the modified phrase add to the database successfully or not, If the adding is fail, then re-add the original phrase.

See also: #109

jserv commented 8 years ago

@jeffreyhc : Of course, you need message body as well. I will merge your contribution after you push the revised commit.

By the way, check this: https://help.github.com/articles/closing-issues-via-commit-messages/

jserv commented 8 years ago

@jeffreyhc : Separate subject from body with a blank line. Reference: http://chris.beams.io/posts/git-commit/

jeffreyhc commented 8 years ago

@jserv So my commit message should only one blank line between subject and body? I thought the content of body can also be separated by blank line

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 94.382% when pulling 4f2dd9805868119dff8c2c7183b3e38c2b83ae1e on jeffreyhc:master into c987c8c9d93bbd186c62630312f46bed51bb7e2c on chewing:master.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 94.382% when pulling a075b6bfefa6007c3bd9b893478873ae3974afd2 on jeffreyhc:master into c987c8c9d93bbd186c62630312f46bed51bb7e2c on chewing:master.

jserv commented 8 years ago

@jeffreyhc : Thanks for your contribution!