+1 A nice trick for finding if a constant string you're about to add already exists in the codebase is to use http://codesearch.google.com/. For example, to find existing strings that start with javascript, you could use this query: http://www.google.com/codesearch?hl=en&lr=&q=src.chromium.org+\%22javascript+lang:c%2B%2B&sbtn=Search
If you find it exists and it's already a constant you can use, great. If it exists but is not yet a reusable constant, the right thing to do is to move it to where it can be reused. Cheers, Jói On Sat, Sep 12, 2009 at 6:33 PM, Brett Wilson <[email protected]> wrote: > > Did you know that most common URLs like "about:blank" and schemes like > "javascript" are in a nice file of constants for you to use, so you > don't have to duplicate the same data over and over? I didn't think > so. > > I noticed a few places where we'd accumulated a few more hardcoded > URLs since I last looked, and did a patch a week ago. I went to upload > it today and there was a conflict. Somebody had added a *new* > GURL("about:blank") near my change since I had initially wrote my > patch! > > The file is chrome/common/url_constants.h. Examples of use: > > GURL(chrome::kAboutBlankURL). > url.SchemeIs(chrome::kJavascriptScheme); > > Now you know. > > In a related note, I noticed that the platform specific files for the > task manager view *each* have this at the top of them: > const wchar_t* const kAcknowledgements = L"about:credits"; > const wchar_t* const kTOS = L"about:terms"; > It makes me sad that both times the people who ported this file > thought that the best thing would be to copy and paste the Windows > code rather than share them in some clean way. > > I've added the constants to our nice file of URL constants. If you're > adding or reviewing things, please be on the lookout for hardcoded > URLs. It makes it easier to maintain and look up when the ones with > special meaning are all in the same place, and we don't have to rely > on the linker to remove all the duplicated data (especially for dozens > of "about:blank" and "about" scheme checks). > > Brett > > > > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
