Polish the command-line reference.

- change options to print --foo=value
- allow option to specify what "value" should be
- update the documentation for the startup options
- add some indentation to the descriptions
- add some spacing between flag texts

--
MOS_MIGRATED_REVID=125960618

9 files changed, 84 insertions, 61 deletions

diff --git a/site/_sass/style.scss b/site/_sass/style.scss
index 01f71c4a78..b999cc0cd1 100644
--- a/site/_sass/style.scss
+++ b/site/_sass/style.scss
@@ -469,6 +469,15 @@ $toc-color: #757575;
   }
 }
 
+// Command-line Reference
+dt {
+  margin-top: .5em;
+}
+
+dd {
+  margin-left: 2em;
+}
+
 // Roadmap page styles
 .roadmap-col-phase {
   width: 10%;
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/WorkspaceStatusAction.java b/src/main/java/com/google/devtools/build/lib/analysis/WorkspaceStatusAction.java
index e12d6c86c2..90653115b2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/WorkspaceStatusAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/WorkspaceStatusAction.java
@@ -59,6 +59,7 @@ public abstract class WorkspaceStatusAction extends AbstractAction {
     @Option(name = "embed_label",
         defaultValue = "",
         category = "misc",
+        valueHelp = "<string>",
         help = "Embed source control revision or release label in binary")
     public String embedLabel;
 
@@ -66,6 +67,7 @@ public abstract class WorkspaceStatusAction extends AbstractAction {
         defaultValue = "",
         category = "misc",
         converter = OptionsUtils.PathFragmentConverter.class,
+        valueHelp = "<path>",
         help = "A command invoked at the beginning of the build to provide status "
             + "information about the workspace in the form of key/value pairs.  "
             + "See the User's Manual for the full specification.")
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
index 9f80d6ac92..442b53303c 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
@@ -87,9 +87,9 @@ public class BlazeServerStartupOptions extends OptionsBase {
    * any other embedded binaries - anything that ends up in the install_base).
    */
   @Option(name = "install_md5",
-                 defaultValue = "", // NOTE: purely decorative!  See class docstring.
-                 category = "hidden",
-                 help = "This launcher option is intended for use only by tests.")
+      defaultValue = "", // NOTE: purely decorative!  See class docstring.
+      category = "hidden",
+      help = "This launcher option is intended for use only by tests.")
   public String installMD5;
 
   /* Note: The help string in this option applies to the client code; not
@@ -101,12 +101,13 @@ public class BlazeServerStartupOptions extends OptionsBase {
       defaultValue = "null", // NOTE: purely decorative!  See class docstring.
       category = "server startup",
       converter = OptionsUtils.PathFragmentConverter.class,
+      valueHelp = "<path>",
       help = "If set, specifies the output location to which all build output will be written. "
           + "Otherwise, the location will be "
           + "${OUTPUT_ROOT}/_blaze_${USER}/${MD5_OF_WORKSPACE_ROOT}. Note: If you specify a "
           + "different option from one to the next Blaze invocation for this value, you'll likely "
           + "start up a new, additional Blaze server. Blaze starts exactly one server per "
-          + "specified output base. Typically there is one output base per workspace--however, "
+          + "specified output base. Typically there is one output base per workspace - however, "
           + "with this option you may have multiple output bases per workspace and thereby run "
           + "multiple builds for the same client on the same machine concurrently. See "
           + "'blaze help shutdown' on how to shutdown a Blaze server.")
@@ -120,6 +121,7 @@ public class BlazeServerStartupOptions extends OptionsBase {
       defaultValue = "null", // NOTE: purely decorative!  See class docstring.
       category = "server startup",
       converter = OptionsUtils.PathFragmentConverter.class,
+      valueHelp = "<path>",
       help = "The user-specific directory beneath which all build outputs are written; "
           + "by default, this is a function of $USER, but by specifying a constant, build outputs "
           + "can be shared between collaborating users.")
@@ -136,6 +138,7 @@ public class BlazeServerStartupOptions extends OptionsBase {
   @Option(name = "max_idle_secs",
       defaultValue = "" + (3 * 3600), // NOTE: purely decorative!  See class docstring.
       category = "server startup",
+      valueHelp = "<integer>",
       help = "The number of seconds the build server will wait idling before shutting down. Zero "
           + "means that the server will never shutdown.")
   public int maxIdleSeconds;
@@ -182,27 +185,30 @@ public class BlazeServerStartupOptions extends OptionsBase {
   @Option(name = "io_nice_level",
       defaultValue = "-1",  // NOTE: purely decorative!
       category = "server startup",
-      help = "Set a level from 0-7 for best-effort IO scheduling. 0 is highest priority, "
-          + "7 is lowest. The anticipatory scheduler may only honor up to priority 4. "
-          + "Negative values are ignored.")
+      valueHelp = "{-1,0,1,2,3,4,5,6,7}",
+      help = "Only on Linux; set a level from 0-7 for best-effort IO scheduling using the "
+          + "sys_ioprio_set system call. 0 is highest priority, 7 is lowest. The anticipatory "
+          + "scheduler may only honor up to priority 4. If set to a negative value, then Blaze "
+          + "does not perform a system call.")
   public int ioNiceLevel;
 
   @Option(name = "batch_cpu_scheduling",
       defaultValue = "false",  // NOTE: purely decorative!
       category = "server startup",
-      help = "Use 'batch' CPU scheduling for Blaze. This policy is useful for workloads that are "
-          + "non-interactive, but do not want to lower their nice value. "
-          + "See 'man 2 sched_setscheduler'.")
+      help = "Only on Linux; use 'batch' CPU scheduling for Blaze. This policy is useful for "
+          + "workloads that are non-interactive, but do not want to lower their nice value. "
+          + "See 'man 2 sched_setscheduler'. If false, then Blaze does not perform a system call.")
   public boolean batchCpuScheduling;
 
   @Option(name = "blazerc",
-      // NOTE: purely decorative!
-      defaultValue = "In the current directory, then in the user's home directory, the file named "
-         + ".$(basename $0)rc (i.e. .%{product}rc)",
+      defaultValue = "null", // NOTE: purely decorative!
       category = "misc",
+      valueHelp = "<path>",
       help = "The location of the .%{product}rc file containing default values of "
-          + "Blaze command options.  Use /dev/null to disable the search for a "
-          + "%{product}rc file, e.g. in release builds.")
+          + "Blaze command options. By default, Blaze first checks the current directory, then "
+          + "the user's home directory, and then looks for a file named .$(basename $0)rc "
+          + "(i.e. .%{product}rc). Use /dev/null to disable the search for a %{product}rc file, "
+          + "e.g. in release builds.")
   public String blazerc;
 
   @Option(name = "master_blazerc",
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/HostJvmStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/HostJvmStartupOptions.java
index 340d766270..449f4525d5 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/HostJvmStartupOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/HostJvmStartupOptions.java
@@ -17,6 +17,8 @@ package com.google.devtools.build.lib.runtime;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionsBase;
 
+import java.util.List;
+
 /**
  * Options that will be evaluated by the blaze client startup code only.
  *
@@ -29,20 +31,29 @@ public class HostJvmStartupOptions extends OptionsBase {
   @Option(name = "host_jvm_args",
           defaultValue = "", // NOTE: purely decorative!  See BlazeServerStartupOptions.
           category = "host jvm startup",
+          allowMultiple = true,
+          valueHelp = "<jvm_arg>",
           help = "Flags to pass to the JVM executing Blaze.")
-  public String hostJvmArgs;
+  public List<String> hostJvmArgs;
 
   @Option(name = "host_jvm_profile",
           defaultValue = "", // NOTE: purely decorative!  See BlazeServerStartupOptions.
           category = "host jvm startup",
-          help = "Run the JVM executing Blaze in the given profiler. Blaze will search for "
-              + "certain hardcoded paths based on the profiler.")
+          valueHelp = "<profiler_name>",
+          help = "Convenience option to add some profiler/debugger-specific JVM startup flags. "
+              + "Blaze has a list of known values that it maps to hard-coded JVM startup flags, "
+              + "possibly searching some hardcoded paths for certain files.")
   public String hostJvmProfile;
 
   @Option(name = "host_jvm_debug",
-          defaultValue = "false", // NOTE: purely decorative!  See BlazeServerStartupOptions.
+          defaultValue = "null", // NOTE: purely decorative!  See BlazeServerStartupOptions.
           category = "host jvm startup",
-          help = "Run the JVM executing Blaze so that it listens for a connection from a "
-              + "JDWP-compliant debugger.")
-  public boolean hostJvmDebug;
+          help = "Convenience option to add some additional JVM startup flags, which cause "
+              + "the JVM to wait during startup until you connect from a JDWP-compliant debugger "
+              + "(like Eclipse) to port 5005.",
+          expansion = {
+              "--host_jvm_args=-Xdebug",
+              "--host_jvm_args=-Xrunjdwp:transport=dt_socket,server=y,address=5005",
+          })
+  public Void hostJvmDebug;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java b/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java
index 69dc3ab9db..b90ad97aa8 100644
--- a/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java
@@ -107,29 +107,6 @@ public final class OptionsUtils {
   }
 
   /**
-   * Converter from String to PathFragment.
-   *
-   * <p>Complains if the path is not absolute.
-   */
-  public static class AbsolutePathFragmentConverter
-      implements Converter<PathFragment> {
-
-    @Override
-    public PathFragment convert(String input) throws OptionsParsingException {
-      PathFragment pathFragment = new PathFragment(input);
-      if (!pathFragment.isAbsolute()) {
-        throw new OptionsParsingException("Expected absolute path, found " + input);
-      }
-      return pathFragment;
-    }
-
-    @Override
-    public String getTypeDescription() {
-      return "an absolute path";
-    }
-  }
-
-  /**
    * Converts from a colon-separated list of strings into a list of PathFragment instances.
    */
   public static class PathFragmentListConverter
diff --git a/src/main/java/com/google/devtools/common/options/BoolOrEnumConverter.java b/src/main/java/com/google/devtools/common/options/BoolOrEnumConverter.java
index 82b8cdc9e8..3e16f89a51 100644
--- a/src/main/java/com/google/devtools/common/options/BoolOrEnumConverter.java
+++ b/src/main/java/com/google/devtools/common/options/BoolOrEnumConverter.java
@@ -24,7 +24,6 @@ import com.google.devtools.common.options.Converters.BooleanConverter;
  * the underlying enumeration will be taken.
  */
 public abstract class BoolOrEnumConverter<T extends Enum<T>> extends EnumConverter<T>{
-
   private T falseValue;
   private T trueValue;
 
@@ -46,6 +45,7 @@ public abstract class BoolOrEnumConverter<T extends Enum<T>> extends EnumConvert
     this.falseValue = falseValue;
   }
 
+  @Override
   public T convert(String input) throws OptionsParsingException {
     try {
       return super.convert(input);
diff --git a/src/main/java/com/google/devtools/common/options/Option.java b/src/main/java/com/google/devtools/common/options/Option.java
index e269d33910..3b2ca30866 100644
--- a/src/main/java/com/google/devtools/common/options/Option.java
+++ b/src/main/java/com/google/devtools/common/options/Option.java
@@ -25,7 +25,6 @@ import java.lang.annotation.Target;
 @Target(ElementType.FIELD)
 @Retention(RetentionPolicy.RUNTIME)
 public @interface Option {
-
   /**
    * The name of the option ("--name").
    */
@@ -42,6 +41,12 @@ public @interface Option {
   String help() default "";
 
   /**
+   * A short text string to describe the type of the expected value. E.g., <code>regex</code>. This
+   * is ignored for boolean, tristate, boolean_or_enum, and void options.
+   */
+  String valueHelp() default "";
+
+  /**
    * The default value for the option. This method should only be invoked
    * directly by the parser implementation. Any access to default values
    * should go via the parser to allow for application specific defaults.
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
index 6d91dec600..3422f7839d 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java
@@ -815,6 +815,10 @@ class OptionsParserImpl {
         || findConverter(field) instanceof BoolOrEnumConverter;
   }
 
+  static boolean isVoidField(Field field) {
+    return field.getType().equals(Void.class);
+  }
+
   static boolean isSpecialNullDefault(String defaultValueString, Field optionField) {
     return defaultValueString.equals("null") && !optionField.getType().isPrimitive();
   }
diff --git a/src/main/java/com/google/devtools/common/options/OptionsUsage.java b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
index 111a772492..b8c19df800 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsUsage.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsUsage.java
@@ -133,28 +133,37 @@ class OptionsUsage {
   static void getUsageHtml(Field optionField, StringBuilder usage, Escaper escaper) {
     String plainFlagName = optionField.getAnnotation(Option.class).name();
     String flagName = getFlagName(optionField);
+    String valueDescription = optionField.getAnnotation(Option.class).valueHelp();
     String typeDescription = getTypeDescription(optionField);
     Option annotation = optionField.getAnnotation(Option.class);
     usage.append("<dt><code><a name=\"flag--").append(plainFlagName).append("\"></a>--");
-    usage.append(flagName).append("</code>");
+    usage.append(flagName);
+    if (OptionsParserImpl.isBooleanField(optionField)
+        || OptionsParserImpl.isVoidField(optionField)) {
+      // Nothing for boolean, tristate, boolean_or_enum, or void options.
+    } else if (!valueDescription.isEmpty()) {
+      usage.append("=").append(escaper.escape(valueDescription));
+    } else if (!typeDescription.isEmpty()) {
+      // Generic fallback, which isn't very good.
+      usage.append("=&lt;").append(escaper.escape(typeDescription)).append("&gt");
+    }
+    usage.append("</code>");
     if (annotation.abbrev() != '\0') {
       usage.append(" [<code>-").append(annotation.abbrev()).append("</code>]");
     }
-    if (!typeDescription.isEmpty()) {
-      usage.append(" (").append(escaper.escape(typeDescription)).append("; ");
-      if (annotation.allowMultiple()) {
-        // Allow-multiple options can't have a default value.
-        usage.append("may be used multiple times");
+    if (annotation.allowMultiple()) {
+      // Allow-multiple options can't have a default value.
+      usage.append(" multiple uses are accumulated");
+    } else {
+      // Don't call the annotation directly (we must allow overrides to certain defaults).
+      String defaultValueString = OptionsParserImpl.getDefaultOptionString(optionField);
+      if (OptionsParserImpl.isVoidField(optionField)) {
+        // Void options don't have a default.
+      } else if (OptionsParserImpl.isSpecialNullDefault(defaultValueString, optionField)) {
+        usage.append(" default: see description");
       } else {
-        // Don't call the annotation directly (we must allow overrides to certain defaults).
-        String defaultValueString = OptionsParserImpl.getDefaultOptionString(optionField);
-        if (OptionsParserImpl.isSpecialNullDefault(defaultValueString, optionField)) {
-          usage.append("default: see description");
-        } else {
-          usage.append("default: \"").append(escaper.escape(defaultValueString)).append("\"");
-        }
+        usage.append(" default: \"").append(escaper.escape(defaultValueString)).append("\"");
       }
-      usage.append(")");
     }
     usage.append("</dt>\n");
     usage.append("<dd>\n");