On Wed, Jan 14, 2009 at 9:22 PM, Evan Martin <[email protected]> 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.
I'm not sure I follow--it only does that when compiled under Linux, which needs it. Compiled on Mac OS X or Linux, it just creates a vector of wide strings from argc and argv, and parses out switches and values. The AppendXXX methods still deal with a single string, but that's mostly because non-Windows platforms haven't needed them until now (that is, it's an incomplete implementation on non-windows platforms). That said, I think that parsing the incoming command line and constructing outgoing ones are separate tasks, and should be separate classes. > We need a sane way to construct cross-platform command lines and > invoke subprocesses. I'm not sure I follow this either--platforms differ greatly in how subprocesses get invoked and the ways that the parent can communicate with them. But for the moment, sure :-). > 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>). Seems OK, though I'd probably call it something like SystemCommandLine or ProcessCommandLine to make it clear that it refers to the command line that the OS passed to us. > 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.) A working patch is always persuasive :-). I have no objection to taking this approach. --Amanda --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
