CommandLine refactoring and cleanup.

chrome/installer/util/install_util.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // See the corresponding header file for description of the functions in this
 // file.
 
 #include "chrome/installer/util/install_util.h"
 
 #include <shellapi.h>
@@ skipping 341 matching lines @@
       return 0;
     default:
       return status;
   }
 }
 
 // static
 void InstallUtil::MakeUninstallCommand(const std::wstring& exe_path,
                                        const std::wstring& arguments,
                                        CommandLine* command_line) {
-  const bool no_program = exe_path.empty();
-
-  // Return a bunch of nothingness.
-  if (no_program && arguments.empty()) {
-    *command_line = CommandLine(CommandLine::NO_PROGRAM);
-  } else {
-    // Form a full command line string.
-    std::wstring command;
-    command.append(1, L'"')
-        .append(no_program ? L"" : exe_path)
-        .append(L"\" ")
-        .append(arguments);
-
-    // If we have a program name, return this complete command line.
-    *command_line = CommandLine::FromString(command);
-  }
+  // Surround non-empty program name with simple quotes.
+  std::wstring program(!exe_path.empty() ? L"\"" + exe_path + L"\" " : L"\"\"");

[grt (UTC plus 2), 2011/05/11 14:27:32]

No whitespace is put between the pair of double quotes and the arguments when
|exe_path| is empty.  I'm not a fan of operator+ and strings, but I won't fight
the use of it.  If you like this simplified code, would it work to push it
further to:
*command_line = CommandLine::FromString(L"\"" + exe_path + L"\" " + arguments);
Does that do the right thing with all permutations of |exe_path| and |arguments|
being empty?
Also, please make sure that installer_util_unittests pass (there's a test for
this function in there).  Thanks.

[msw, 2011/05/12 00:37:47]

Done and tests pass. This refactoring is entirely optional, so let me know if
you'd rather not have this change. Only the updates to
chrome/installer/util/product_state_unittest.cc are necessary.

+  *command_line = CommandLine::FromString(program + arguments);
 }
 
 std::wstring InstallUtil::GetCurrentDate() {
   static const wchar_t kDateFormat[] = L"yyyyMMdd";
   wchar_t date_str[arraysize(kDateFormat)] = {0};
   int len = GetDateFormatW(LOCALE_INVARIANT, 0, NULL, kDateFormat,
                            date_str, arraysize(date_str));
   if (len) {
     --len;  // Subtract terminating \0.
   } else {
     PLOG(DFATAL) << "GetDateFormat";
   }
 
   return std::wstring(date_str, len);
 }