From 536ecdfe673fc18663c840ac160f9fccb6ef81e8 Mon Sep 17 00:00:00 2001 From: Anthony Keenan Date: Thu, 6 Sep 2018 09:37:30 +0100 Subject: [PATCH] CORDA-1760 CLI UX guidelines (#3686) * Create CLI UX Guidelines * More tweaking * Address review comments * More tweaks * Add recommendation for generating and installing completion files * Update documentation to include base class information * Use ExitCodes base class --- docs/source/cli-ux-guidelines.rst | 144 +++++++++++++++++++++++++++++ docs/source/contributing-index.rst | 1 + docs/source/contributing.rst | 15 +-- 3 files changed, 146 insertions(+), 14 deletions(-) create mode 100644 docs/source/cli-ux-guidelines.rst diff --git a/docs/source/cli-ux-guidelines.rst b/docs/source/cli-ux-guidelines.rst new file mode 100644 index 0000000000..a4917b1620 --- /dev/null +++ b/docs/source/cli-ux-guidelines.rst @@ -0,0 +1,144 @@ +.. highlight:: kotlin +.. raw:: html + + + + +CLI UX Guide +============ + +Command line options +-------------------- + +Command line utilities should use picocli (http://picocli.info) to provide a unified interface and follow the conventions in the picocli documentation, some of the more important of which are repeated below. + +Option names +~~~~~~~~~~~~ + +* Options should be specified on the command line using a double dash, e.g. ``--parameter``. +* Options that consist of multiple words should be separated via hyphens e.g. ``--my-multiple-word-parameter-name``. + +Short names +~~~~~~~~~~~ + +* Where possible a POSIX style short option should be provided for ease of use (see http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02). + + * These should be prefixed with a single hyphen. + * For example ``-V`` for ``--verbose``, ``-d`` for ``--dev-mode``. + * Consider adding short options for commands that would be ran regularly as part of troubleshooting/operational processes. + * Short options should not be used for commands that would be used just once, for example initialising/registration type tasks. + +* The picocli interface allows combinations of options without parameters, for example, ```-v`` and ```-d`` can be combined as ``-vd``. + +Positional parameters +~~~~~~~~~~~~~~~~~~~~~ + +* Parameters specified without an option should ideally all be part of a list. + + * For example, in ``java -jar test.jar file1 file2 file3``, the parameters file1, file2 and file3 should be a list of files that are all acted on together (e.g. a list of CorDapps). + +* Avoid using positional parameters to mean different things, which involves someone remembering in which order things need to be specified. + + * For example, avoid ``java -jar test.jar configfile1 cordapp1 cordapp2`` where parameter 1 is the config file and any subsequent parameters are the CorDapps. + * Use ``java -jar test.jar cordapp1 cordapp2 --config-file configfile1`` instead. + +Standard options +~~~~~~~~~~~~~~~~ + +* A ``--help`` option should be provided which details all possible options with a brief description and any short name equivalents. A ``-h`` short option should also be provided. +* A ``--version`` option that should output the version number of the software. A ``-V`` short option should also be provided. +* A ``--logging-level`` option should be provided which specifies the logging level to be used in any logging files. Acceptable values should be ``DEBUG``, ``TRACE``, ``INFO``, ``WARN`` and ``ERROR``. +* ``--verbose`` and ``--log-to-console`` options should be provided (both equivalent) which specifies that logging output should be displayed in the console. + A ``-v`` short option should also be provided. +* A ``--install-shell-extensions`` option should be provided that creates and installs a bash completion file. + + +Defaults +~~~~~~~~ + +* Flags should have sensible defaults. +* Boolean flags should always default to false. Specifying the flag without a parameter should set it to true. For example ``--use-something` should be equal to ``--use-something=true`` and no option should be equal to ``--my-flag=false``. +* Do a bit of work to figure out reasonable defaults. Nobody likes having to set a dozen flags before the tool will cooperate. + +Adding a new option +~~~~~~~~~~~~~~~~~~~ + +* Boolean options should start with is, has or with. For example, ``--is-cheesy``, ``--with-cheese``, ``--has-cheese-on``. +* Any new options must be documented in the docsite and via the ``--help`` screen. +* Never use acronyms in option names and try and make them as descriptive as possible. + +Parameter stability +~~~~~~~~~~~~~~~~~~~ + +* Avoid removing parameters. If, for some reason, a parameter needs to be renamed, add a new parameter with the new name and deprecate the old parameter, or alternatively keep both versions of the parameter. + + +The ``CordaCliWrapper`` base class +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The ``CordaCliWrapper`` base class from the ``cliutils`` module should be used as a base where practicable, this will provide a set of default options out of the box. +In order to use it, create a class containing your command line options using the syntax provided at (see the `picocli`_ website for more information) + + +.. container:: codeset + + .. sourcecode:: kotlin + + import net.corda.cliutils.ExitCodes + import net.corda.cliutils.CordaCliWrapper + + class UsefulUtilityExitCodes: ExitCodes { + companion object { + val APPLICATION_SPECIFIC_ERROR_CODE: Int = 100 + } + } + + class UsefulUtility : CordaCliWrapper( + "useful-utility", // the alias to be used for this utility in bash. When --install-shell-extensions is run + // you will be able to invoke this command by running from the command line + "A command line utility that is super useful!" // A description of this utility to be displayed when --help is run + ) { + @Option(names = ["--extra-usefulness", "-e"], // A list of the different ways this option can be referenced + description = ["Use this option to add extra usefulness"] // Help description to be displayed for this option + ) + private var extraUsefulness: Boolean = false // This default option will be shown in the help output + + override fun runProgram(): Int { // override this function to run the actual program + try { + // do some stuff + } catch (KnownException: ex) { + return UsefulUtilityExitCodes.APPLICATION_SPECIFIC_ERROR_CODE // return a special exit code for known exceptions + } + + return UsefulUtilityExitCodes.SUCCESS // this is the exit code to be returned to the system inherited from the ExitCodes base class + } + } + + +Then in your ``main()`` method: + +.. container:: codeset + + .. sourcecode:: kotlin + + import net.corda.cliutils.start + + fun main(args: Array) { + UsefulUtility().start(args) + } + + + +Application behavior +-------------------- + +* Set exit codes using exitProcess. + + * Zero means success. + * Other numbers mean errors. + +* Setting a unique error code (starting from 1) for each thing that can conceivably break makes your tool shell-scripting friendly. +* Make sure all exit codes are documented with recommended remedies where applicable. +* Your ``--help`` text or other docs should ideally include examples. Writing examples is also a good way to find out if your program requires a dozen flags to do anything. +* Don’t print logging output to the console unless the user requested it via a ``-–verbose`` flag (conventionally shortened to ``-v``). Logs should be either suppressed or saved to a text file during normal usage, except for errors, which are always OK to print. +* Don't print stack traces to the console. Stack traces can be added to logging files, but the user should see as meaningful error description as possible. \ No newline at end of file diff --git a/docs/source/contributing-index.rst b/docs/source/contributing-index.rst index 1187c1af22..67a3b10f7c 100644 --- a/docs/source/contributing-index.rst +++ b/docs/source/contributing-index.rst @@ -12,6 +12,7 @@ of contributing to Corda. building-corda testing codestyle + cli-ux-guidelines building-the-docs api-scanner contributing-flow-state-machines \ No newline at end of file diff --git a/docs/source/contributing.rst b/docs/source/contributing.rst index 6e4516dd05..5ac581af65 100644 --- a/docs/source/contributing.rst +++ b/docs/source/contributing.rst @@ -65,20 +65,7 @@ make merging more complicated. Things to consider when writing CLI apps ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -* Set exit codes using ``exitProcess``. Zero means success. Other numbers mean errors. Setting a unique error code - (starting from 1) for each thing that can conceivably break makes your tool shell-scripting friendly - -* Do a bit of work to figure out reasonable defaults. Nobody likes having to set a dozen flags before the tool will - cooperate - -* Your ``--help`` text or other docs should ideally include examples. Writing examples is also a good way to find out - that your program requires a dozen flags to do anything - -* Flags should have sensible defaults - -* Don’t print logging output to the console unless the user requested it via a ``–verbose`` flag (conventionally - shortened to ``-v``) or a ``–log-to-console`` flag. Logs should be either suppressed or saved to a text file during - normal usage, except for errors, which are always OK to print +Make sure any changes to CLI applications conform to the :doc:`cli-ux-guidelines`. Testing the changes -------------------