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 SingletonCommandLine).
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
-~--~~~~--~~--~--~---
