Translate: display alpha language name with (alpha) suffix

chrome/browser/translate/translate_language_list.h

 // Copyright 2013 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 CHROME_BROWSER_TRANSLATE_TRANSLATE_LANGUAGE_LIST_H_
 #define CHROME_BROWSER_TRANSLATE_TRANSLATE_LANGUAGE_LIST_H_
 
 #include <set>
 #include <string>
 #include <vector>
@@ skipping 10 matching lines @@
 // This class is defined to be owned only by TranslateManager.
 class TranslateLanguageList : public net::URLFetcherDelegate {
  public:
   TranslateLanguageList();
   virtual ~TranslateLanguageList();
 
   // net::URLFetcherDelegate implementation:
   virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
 
   // Fills |languages| with the list of languages that the translate server can
-  // translate to and from.
+  // translate to and from. If alpha language support is enabled, it fills
+  // |languages| with the list of all supporting languages including alpha
+  // languages.
   void GetSupportedLanguages(std::vector<std::string>* languages);
 
   // Returns the language code that can be used with the Translate method for a
   // specified |chrome_locale|.
   std::string GetLanguageCode(const std::string& chrome_locale);
 
-  // Returns true if |language| is supported by the translation server.
+  // Returns true if |language| is supported by the translation server. If alpha
+  // language support is enabled, returns true if |language| is in alpha

[MAD, 2013/05/31 15:14:52]

This comment seems to say that when alpha is enabled we only return true for
alpha languages, I would rephrase by adding "also" as follows:

If alpha language support is enabled, also returns true if |language| is in
alpha...

[Takashi Toyoshima, 2013/05/31 16:37:06]

thanks!

+  // language list.
   bool IsSupportedLanguage(const std::string& language);
 
-  // TODO(toyoshim): Add IsSupportedAlphaLanguage() here.
+  // Returns true if |language| is supported by the translation server as a
+  // alpha language.
+  bool IsAlphaLanguage(const std::string& language);
 
   // Fetches the language list from the translate server. It will not retry
   // more than kMaxRetryLanguageListFetch times.
   void RequestLanguageList();
 
   // static const values shared with our browser tests.
   static const char kLanguageListCallbackName[];
   static const char kTargetLanguagesKey[];
 
  private:
-  // Parses |language_list| and fills |supported_languages_| with the list of
-  // languages that the translate server can translate to and from.
-  void SetSupportedLanguages(const std::string& language_list);
+  // Updates |supported_all_languages_| contains union of |supported_languages_|

[MAD, 2013/05/31 15:14:52]

contains union -> to contain the union

[Takashi Toyoshima, 2013/05/31 16:37:06]

Done.

+  // and |supported-alpha_languages_|.

[MAD, 2013/05/31 15:14:52]

Nit: supported-alpha_languages_ -> supported_alpha_languages_

[Takashi Toyoshima, 2013/05/31 16:37:06]

Done.

+  void UpdateSupportedLanguages();
 
   // The languages supported by the translation server.
   std::set<std::string> supported_languages_;
 
+  // The alpha languages supported by the translation server.
+  std::set<std::string> supported_alpha_languages_;
+
+  // The languages supported by the translation server. It contains union of

[MAD, 2013/05/31 15:14:52]

I would say "All the languages supported..." and add "the" to "It contains the
union of"

[Takashi Toyoshima, 2013/05/31 16:37:06]

Done.

+  // |supported_languages_| and |supported_alpha_languages_|.
+  std::set<std::string> supported_all_languages_;

[MAD, 2013/05/31 15:14:52]

I would rename it to all_supported_languages_.

[Takashi Toyoshima, 2013/05/31 16:37:06]

Done.

+
   // An URLFetcher instance to fetch a server providing supported language list.
-  scoped_ptr<net::URLFetcher> url_fetcher_;
+  scoped_ptr<net::URLFetcher> language_list_fetcher_;
+
+  // An URLFetcher instance to fetch a server providing supported alpha language
+  // list.
+  scoped_ptr<net::URLFetcher> alpha_language_list_fetcher_;
+
+  // A flag to representing if alpha language support is enabled.
+  bool alpha_language_support_is_enabled_;

[MAD, 2013/05/31 15:14:52]

You don't seem to be using that...

[Takashi Toyoshima, 2013/05/31 16:37:06]

Thanks.
I stop using it at the last clean up.
Sorry for mistakes.

 
   DISALLOW_COPY_AND_ASSIGN(TranslateLanguageList);
 };
 
 #endif  // CHROME_BROWSER_TRANSLATE_TRANSLATE_LANGUAGE_LIST_H_