Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(699)

Issue 3146005: Do not commit buffer in reset function. (Closed)

Created:
10 years, 4 months ago by Peng
Modified:
9 years, 5 months ago
Reviewers:
Zachary Kuznia
CC:
chromium-os-reviews_chromium.org
Base URL:
http://src.chromium.org/git/ibus-chewing.git
Visibility:
Public.

Description

Do not commit buffer in reset function. BUG=chromium-os:4792 TEST=manually

Patch Set 1 #

Total comments: 1

Patch Set 2 : clear ENGINE_STATUS_NEED_COMMIT flag #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -4 lines) Patch
M src/IBusChewingEngine.gob View 1 1 chunk +5 lines, -4 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Peng
10 years, 4 months ago (2010-08-10 10:45:34 UTC) #1
Zachary Kuznia
http://codereview.chromium.org/3146005/diff/1/2 File src/IBusChewingEngine.gob (right): http://codereview.chromium.org/3146005/diff/1/2#newcode943 src/IBusChewingEngine.gob:943: ibus_chewing_engine_clear_status_flag(self, ENGINE_STATUS_FORCE_COMMIT); Should the ENGINE_STATUS_NEED_COMMIT be cleared here, too?
10 years, 4 months ago (2010-08-10 10:52:58 UTC) #2
Peng
On 2010/08/10 10:52:58, Zachary Kuznia wrote: > http://codereview.chromium.org/3146005/diff/1/2 > File src/IBusChewingEngine.gob (right): > > http://codereview.chromium.org/3146005/diff/1/2#newcode943 ...
10 years, 4 months ago (2010-08-11 10:51:05 UTC) #3
Zachary Kuznia
10 years, 4 months ago (2010-08-11 10:54:43 UTC) #4
LGTM

On 2010/08/11 10:51:05, Peng wrote:
> On 2010/08/10 10:52:58, Zachary Kuznia wrote:
> > http://codereview.chromium.org/3146005/diff/1/2
> > File src/IBusChewingEngine.gob (right):
> > 
> > http://codereview.chromium.org/3146005/diff/1/2#newcode943
> > src/IBusChewingEngine.gob:943: ibus_chewing_engine_clear_status_flag(self,
> > ENGINE_STATUS_FORCE_COMMIT);
> > Should the ENGINE_STATUS_NEED_COMMIT be cleared here, too?
> 
> Yeah. I updated the CL. Please check it.

Powered by Google App Engine
This is Rietveld 408576698