Format GN files in-place by default.

tools/gn/command_format.cc

 // Copyright 2014 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.
 
 #include <stddef.h>
 
 #include <sstream>
 
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "tools/gn/commands.h"
 #include "tools/gn/filesystem_utils.h"
 #include "tools/gn/input_file.h"
 #include "tools/gn/parser.h"
 #include "tools/gn/scheduler.h"
 #include "tools/gn/setup.h"
 #include "tools/gn/source_file.h"
 #include "tools/gn/tokenizer.h"
 
 namespace commands {
 
 const char kSwitchDryRun[] = "dry-run";
 const char kSwitchDumpTree[] = "dump-tree";
-const char kSwitchInPlace[] = "in-place";
 const char kSwitchStdin[] = "stdin";
 
 const char kFormat[] = "format";
 const char kFormat_HelpShort[] =
     "format: Format .gn file.";
 const char kFormat_Help[] =
-    "gn format [--dump-tree] [--in-place] [--stdin] BUILD.gn\n"
+    "gn format [--dump-tree] [--stdin | <build_file> ]\n"

[scottmg, 2016/08/05 17:31:48]

no space between file> and ]. Or probably better to remove the [], since they're
not optional.

     "\n"
     "  Formats .gn file to a standard format.\n"
     "\n"
     "  The contents of some lists ('sources', 'deps', etc.) will be sorted to\n"
     "  a canonical order. To suppress this, you can add a comment of the form\n"
     "  \"# NOSORT\" immediately preceeding the assignment. e.g.\n"
     "\n"
     "  # NOSORT\n"
     "  sources = [\n"
     "    \"z.cc\",\n"
     "    \"a.cc\",\n"
     "  ]\n"
     "\n"
     "Arguments\n"
+    "\n"
     "  --dry-run\n"
     "      Does not change or output anything, but sets the process exit code\n"
     "      based on whether output would be different than what's on disk.\n"
     "      This is useful for presubmit/lint-type checks.\n"
     "      - Exit code 0: successful format, matches on disk.\n"
     "      - Exit code 1: general failure (parse error, etc.)\n"
     "      - Exit code 2: successful format, but differs from on disk.\n"
     "\n"
     "  --dump-tree\n"
     "      For debugging only, dumps the parse tree.\n"
     "\n"
-    "  --in-place\n"
-    "      Instead of writing the formatted file to stdout, replace the input\n"
-    "      file with the formatted output. If no reformatting is required,\n"
-    "      the input file will not be touched, and nothing printed.\n"
-    "\n"
     "  --stdin\n"
-    "      Read input from stdin (and write to stdout). Not compatible with\n"
-    "      --in-place of course.\n"
+    "      Read input from stdin and write to stdout rather than update\n"
+    "      a file in-place.\n"
     "\n"
     "Examples\n"
     "  gn format //some/BUILD.gn\n"
     "  gn format some\\BUILD.gn\n"
     "  gn format /abspath/some/BUILD.gn\n"
     "  gn format --stdin\n";
 
 namespace {
 
 const int kIndentSize = 2;
@@ skipping 917 matching lines @@
   return true;
 }
 
 int RunFormat(const std::vector<std::string>& args) {
   bool dry_run =
       base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchDryRun);
   bool dump_tree =
       base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchDumpTree);
   bool from_stdin =
       base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchStdin);
-  bool in_place =
-    base::CommandLine::ForCurrentProcess()->HasSwitch(kSwitchInPlace);
 
   if (dry_run) {
     // --dry-run only works with an actual file to compare to.
     from_stdin = false;
-    in_place = true;
   }
 
   if (from_stdin) {
     if (args.size() != 0) {
       Err(Location(), "Expecting no arguments when reading from stdin.\n")
           .PrintToStdout();
       return 1;
     }
     std::string input = ReadStdin();
     std::string output;
@@ skipping 18 matching lines @@
   Err err;
   SourceFile file = source_dir.ResolveRelativeFile(Value(nullptr, args[0]),
                                                    &err);
   if (err.has_error()) {
     err.PrintToStdout();
     return 1;
   }
 
   std::string output_string;
   if (FormatFileToString(&setup, file, dump_tree, &output_string)) {
-    if (in_place) {
+    if (dump_tree) {

[scottmg, 2016/08/05 17:31:48]

It's a bit weird that dump_tree means that it doesn't actually format the file.
But doesn't matter either way, just update the docstring to match the new
behaviour if you prefer it this way.

+      // Print nothing else (the tree was already printed).
+    } else if (from_stdin) {

[scottmg, 2016/08/05 17:31:48]

I don't think this is ever reached. If it's from_stdin, the
FormatStringToString() above is used.

+      // Write formatted output to stdout.
+      printf("%s", output_string.c_str());
+    } else {
+      // Update the file in-place.
       base::FilePath to_write = setup.build_settings().GetFullPath(file);
       std::string original_contents;
       if (!base::ReadFileToString(to_write, &original_contents)) {
         Err(Location(), std::string("Couldn't read \"") +
                             to_write.AsUTF8Unsafe() +
                             std::string("\" for comparison.")).PrintToStdout();
         return 1;
       }
       if (dry_run)
         return original_contents == output_string ? 0 : 2;
       if (original_contents != output_string) {
         if (base::WriteFile(to_write,
                             output_string.data(),
                             static_cast<int>(output_string.size())) == -1) {
           Err(Location(),
               std::string("Failed to write formatted output back to \"") +
                   to_write.AsUTF8Unsafe() + std::string("\".")).PrintToStdout();
           return 1;
         }
         printf("Wrote formatted to '%s'.\n", to_write.AsUTF8Unsafe().c_str());
       }
-    } else {
-      printf("%s", output_string.c_str());
     }
   }
 
   return 0;
 }
 
 }  // namespace commands