CL for readability review

views/ime/character_composer.h

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef VIEWS_IME_CHARACTER_COMPOSER_H_
 #define VIEWS_IME_CHARACTER_COMPOSER_H_
 #pragma once
 
 #include <vector>

[brettw, 2011/06/29 20:31:56]

Put a blank line after this (separating the "sections" of the headers like you
did in the .cc file):
http://www/eng/doc/cppguide.xml?external=y&showone=Names_and_Order_of_Include...

[hashimoto, 2011/06/30 07:19:22]

Fixed

 #include "base/basictypes.h"
 #include "base/string_util.h"
 
 namespace views {
 
 // A class to recognize compose and dead key sequence.
 // Outputs composed character.
 //
 // TODO(hashimoto): support unicode character composition starting with
 // Ctrl-Shift-U. http://crosbug.com/15925
 class CharacterComposer {
  public:
   CharacterComposer();
   ~CharacterComposer();
 
   void Reset();
 
   // Filters keypress.  Returns true if the keypress is handled.

[brettw, 2011/06/29 20:31:56]

Can you clarify in this comment what "handled" means? Does that mean it was a
dead key and it was added to the buffer, or that it wasn't relevant to the
character composer, etc.?

[hashimoto, 2011/06/30 07:19:22]

Fixed

   bool FilterKeyPress(unsigned int keycode);
 
   // Returns a string consisting of composed character.
   // Empty is returned when there is no composition result.
   const string16& GetComposedCharacter() const {

[brettw, 2011/06/29 20:31:56]

This is valid according to the style guide, but I think in Chrome it's normal to
use the composed_character() style of naming inline getters (unless the function
is virtual or something).

[hashimoto, 2011/06/30 07:19:22]

Fixed

     return composed_character_;
   }
 
  private:
   std::vector<unsigned int> compose_buffer_;

[brettw, 2011/06/29 20:31:56]

Can you document these two members? They aren't blindingly obvious to me just
looking at this header.

[hashimoto, 2011/06/30 07:19:22]

Fixed

   string16 composed_character_;
 
   DISALLOW_COPY_AND_ASSIGN(CharacterComposer);
 };
 
 }  // namespace views
 
 #endif  // VIEWS_IME_CHARACTER_COMPOSER_H_
+//