On Wed, Jan 14, 2009 at 6:22 PM, Evan Martin <e...@chromium.org> wrote: > > Our CommandLine class is very confusing -- it is not a class for > working with command lines, but in fact a stealth singleton that wraps > the command line used to start the process. > Further, since it came from Windows, it does all this string-munging > and quoting that is not necessary on OS X or Linux. > > We need a sane way to construct cross-platform command lines and > invoke subprocesses. > > I propose the following: > 1) For the singleton use case, we change code to use a real singleton > (e.g. CommandLine::Get() or even our Singleton<CommandLine>).
I prefer the ::Get version myself. I was also thinking of CommandLine::ForCurrentProcess() since it might reduce some of the ambiguity that seems to crop up here. > 2) We extend the class to also be useful for generating command lines. > Here's a taste of API (that would be folded into CommandLine): > http://codereview.chromium.org/18073/diff/1/3 > The function names intentionally match the old static function names > above so it's easier to convert old code. > Some callers are already incorrectly (by the current API) using > CommandLine like this. > > If this is ok, I volunteer to fix all callers. > (If you haven't dealt with it before, this area of the code is > embarassingly prone to endless arguments, so I apologize for bringing > this up again.) I'm on the fence about this builder class since it somehow feels like overkill for this application. If we do go with the class, do we really need a vector of strings? It seems like every use (though I haven't looked at all uses) is basically. have a string append some switches use the string so that keeping the vector of strings is just a lot of mallocs and string copies with no purpose. Now that I write this, I think having the class is fine since it provides some useful scoping for these things, but I think it should just keep a string and append to it like we do currently. Brett --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---