[chromium-reviews] Change the use of "Language" in Spell Check files to "SpellCheckLanguage". "L...

Change the use of typedef Language in Spell Check files back to std::string. It is unnecessary, and is conflicting with enum Language definition for compact language detection library.

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=20042

[brettw, 2009-07-01 15:54:13]

http://codereview.chromium.org/150139/diff/24/29
File chrome/browser/spellchecker_common.h (right):

http://codereview.chromium.org/150139/diff/24/29#newcode14
Line 14: typedef std::string SpellCheckLanguage;
Why is the typedef both here and in spellchecker.h? Is spellchecker.h only used
on some platforms?

[sidchat (Google), 2009-07-01 19:33:19]

http://codereview.chromium.org/150139/diff/24/29
File chrome/browser/spellchecker_common.h (right):

http://codereview.chromium.org/150139/diff/24/29#newcode14
Line 14: typedef std::string SpellCheckLanguage;
Yes, these were also used in some places. I have removed these declarations.

-Sid

On 2009/07/01 15:54:14, brettw wrote:
> Why is the typedef both here and in spellchecker.h? Is spellchecker.h only
used
> on some platforms?

[brettw, 2009-07-01 20:19:42]

On 2009/07/01 19:33:19, sidchat (Google) wrote:
> http://codereview.chromium.org/150139/diff/24/29
> File chrome/browser/spellchecker_common.h (right):
> 
> http://codereview.chromium.org/150139/diff/24/29#newcode14
> Line 14: typedef std::string SpellCheckLanguage;
> Yes, these were also used in some places. I have removed these declarations.

They're still there.

[sidchat (Google), 2009-07-01 20:41:15]

What I meant was SpellCheckLanguage is now declared _only_ in
spellchecker_common.h, and not in SpellChecker class. All the guys using this
type are now getting the declaration from spellchecker_common.h only.

-Sid

On 2009/07/01 20:19:42, brettw wrote:
> On 2009/07/01 19:33:19, sidchat (Google) wrote:
> > http://codereview.chromium.org/150139/diff/24/29
> > File chrome/browser/spellchecker_common.h (right):
> > 
> > http://codereview.chromium.org/150139/diff/24/29#newcode14
> > Line 14: typedef std::string SpellCheckLanguage;
> > Yes, these were also used in some places. I have removed these declarations.
> 
> They're still there.

[brettw, 2009-07-01 21:11:15]

This typedef just seems unnecessary. The old "Language" was clearly a bad idea,
and I don't think your name is really any clearer. I would say just replace it
with std::string everywhere and the problem will be fixed. We often use strings
for this type of thing.

[sidchat (Google), 2009-07-01 23:01:20]

OK, I see. Made changes as per your suggestions. Please have a look again.

-Sid

On 2009/07/01 21:11:15, brettw wrote:
> This typedef just seems unnecessary. The old "Language" was clearly a bad
idea,
> and I don't think your name is really any clearer. I would say just replace it
> with std::string everywhere and the problem will be fixed. We often use
strings
> for this type of thing.

[brettw, 2009-07-01 23:11:33]

Thanks! LGTM

http://codereview.chromium.org/150139/diff/60/62
File chrome/browser/spellchecker.cc (right):

http://codereview.chromium.org/150139/diff/60/62#newcode119
Line 119: GetCorrespondingSpellCheckLanguage(const std::string& language) {
Can you wrap this like it used to be?

http://codereview.chromium.org/150139/diff/60/62#newcode337
Line 337: FilePath SpellChecker::GetVersionedFileName(
I think the old wrapping was better here.