|
| 1 | +.. _add-new-command-line-switch: |
| 2 | + |
| 3 | +Add new command line switch |
| 4 | +=========================== |
| 5 | + |
| 6 | +This document is a guide for Modules developers that wish to introduce a new |
| 7 | +command line switch for the :command:`module` command, or to accept an |
| 8 | +existing switch on an additional sub-command. |
| 9 | + |
| 10 | +Two kinds of command line switches exist and they are parsed at different |
| 11 | +places in :file:`modulecmd.tcl`: |
| 12 | + |
| 13 | +- *Top-level switches* (like :option:`--timer` or :option:`--ignore-user-rc`) |
| 14 | + control the overall :command:`module` behavior and apply whatever the |
| 15 | + sub-command. They are usually placed before the sub-command name and are |
| 16 | + parsed in the *Main program* section of :file:`tcl/main.tcl.in`. |
| 17 | +- *Sub-command switches* (like :option:`--tag` or :option:`--no-indepth`) only |
| 18 | + make sense for a limited set of sub-commands. They are usually placed after |
| 19 | + the sub-command name and are parsed in the ``parseModuleCommandArgs`` |
| 20 | + procedure of :file:`tcl/main.tcl.in`, which is also called when |
| 21 | + :mfcmd:`module` command is evaluated within a modulefile and by the |
| 22 | + :command:`ml` command. |
| 23 | + |
| 24 | +Decide first which kind the new switch belongs to, as it determines the core |
| 25 | +code changes to make. Whatever the kind, a switch may be set either before or |
| 26 | +after the sub-command name, so both parsing steps are involved to some extent |
| 27 | +(see below). If the switch is a command line way to set an existing |
| 28 | +configuration option, also read :ref:`add-new-config-option`. |
| 29 | + |
| 30 | +Core code |
| 31 | +--------- |
| 32 | + |
| 33 | +Top-level switch |
| 34 | +^^^^^^^^^^^^^^^^ |
| 35 | + |
| 36 | +#. Add a case for the new switch in the main argument parsing ``switch`` |
| 37 | + statement of the *Main program* section. Both the long form (``--name``) |
| 38 | + and the short form (``-X``), if any, are matched there. The usual pattern |
| 39 | + is to set an ``asked_<name>`` global variable that is read afterward when |
| 40 | + initializing the state or the configuration option the switch relates to: |
| 41 | + ``getConf`` and ``getState`` both look for such ``::asked_<name>`` variable |
| 42 | + to supersede the value of the configuration option or state named |
| 43 | + ``<name>``. When the switch does not map to a configuration option or a |
| 44 | + state, the matching case may directly call ``setState`` or perform the |
| 45 | + expected action. |
| 46 | + |
| 47 | + - File to edit: :file:`tcl/main.tcl.in` |
| 48 | + - Example commits: :ghcommit:`2c251a37` (:option:`--timer`), |
| 49 | + :ghcommit:`64b46c19` (:option:`--ignore-user-rc`) |
| 50 | + |
| 51 | + A switch accepting a value should handle its ``--name=VALUE`` form and, if |
| 52 | + a short form exists, the ``-X VALUE`` form through the ``nextargisval`` |
| 53 | + mechanism. Report a *Missing value* error when no value is given, like |
| 54 | + done for :option:`--output`. Validate the value and report an *Invalid |
| 55 | + option* error if it is not accepted, like done for :option:`--width`. |
| 56 | + |
| 57 | +#. If the switch relates to a state rather than a configuration option, |
| 58 | + declare this state in the ``g_state_defs`` definition array. |
| 59 | + |
| 60 | + - File to edit: :file:`tcl/init.tcl.in` |
| 61 | + - Example commit: :ghcommit:`2c251a37` |
| 62 | + |
| 63 | +#. Update the Tcl code to adapt :command:`module` command behavior depending |
| 64 | + on the switch. This part highly depends on what the switch controls. |
| 65 | + |
| 66 | + - File to edit: file(s) in :file:`tcl/` directory |
| 67 | + |
| 68 | +#. Describe the switch in the usage message reported by ``reportUsage``, in |
| 69 | + its *Switches* section. |
| 70 | + |
| 71 | + - File to edit: :file:`tcl/report.tcl.in` |
| 72 | + - Example commit: :ghcommit:`2c251a37` |
| 73 | + |
| 74 | +Sub-command switch |
| 75 | +^^^^^^^^^^^^^^^^^^ |
| 76 | + |
| 77 | +#. Add a case for the new switch in the argument parsing ``switch`` statement |
| 78 | + of the ``parseModuleCommandArgs`` procedure. Test there that current |
| 79 | + sub-command (``cmd`` argument) supports the switch and raise an |
| 80 | + *Unsupported option* error otherwise, unless the ``ignerr`` argument is |
| 81 | + set (which is the case when :command:`ml` filters out the switches to find |
| 82 | + the sub-command name). Switches that are silently ignored on unsupported |
| 83 | + sub-commands, like :option:`--terse`, are a legacy from Modules 3 and |
| 84 | + should not be a model for new switches. |
| 85 | + |
| 86 | + The parsed value is returned by ``parseModuleCommandArgs`` to its callers. |
| 87 | + When the switch controls something new, add a variable for it in the |
| 88 | + result list returned by the procedure and update every ``lassign`` of this |
| 89 | + result: ``moduleTop`` and ``ml`` in :file:`tcl/main.tcl.in`, ``module`` in |
| 90 | + :file:`tcl/mfcmd.tcl` and ``module-sc`` in :file:`tcl/modscan.tcl`. Then |
| 91 | + update ``formatModuleCommand`` to pass the value as argument to the |
| 92 | + ``cmdModule<Subcmdname>`` procedures of the sub-commands accepting the |
| 93 | + switch. When the switch rather alters an existing state (like |
| 94 | + :option:`--terse` does with ``report_format``), the sub-command code reads |
| 95 | + this state and no new argument is needed. |
| 96 | + |
| 97 | + - File to edit: :file:`tcl/main.tcl.in` |
| 98 | + - Example commits: :ghcommit:`a1fb42fd` (:option:`--tag`), |
| 99 | + :ghcommit:`963b9a2d` (:option:`--no-indepth`), |
| 100 | + :ghcommit:`be2caa12` (``--not-req``) |
| 101 | + |
| 102 | + A switch accepting a value should handle its ``--name=VALUE`` form and, if |
| 103 | + a short form exists, the ``-X VALUE`` form through the ``nextargisval`` |
| 104 | + mechanism. Report a *Missing value* error when no value is given, like |
| 105 | + done for :option:`--tag`. |
| 106 | + |
| 107 | +#. Register the switch in the main argument parsing ``switch`` statement of |
| 108 | + the *Main program* section, so that it is not rejected as an *Invalid |
| 109 | + option* when placed before the sub-command name. Two cases already |
| 110 | + exist for that purpose: one appending the switch to the ``extraargv`` list, |
| 111 | + which moves it after the sub-command name (used by the output formatting |
| 112 | + switches like :option:`--terse` or :option:`--json`), and one appending |
| 113 | + the switch to the ``otherargv`` list, which keeps it where it was found |
| 114 | + (used by switches whose position matters, like :option:`-d` that means |
| 115 | + ``--default`` or ``--delim`` depending on the sub-command). A switch |
| 116 | + accepting its value as a separate argument needs its own case, like |
| 117 | + :option:`-o`, to also move the value. |
| 118 | + |
| 119 | + - File to edit: :file:`tcl/main.tcl.in` |
| 120 | + - Example commit: :ghcommit:`a1fb42fd` |
| 121 | + |
| 122 | +#. Craft in the ``cmdModule<Subcmdname>`` procedure of each supporting |
| 123 | + sub-command the Tcl code corresponding to the switch. |
| 124 | + |
| 125 | + - File to edit: :file:`tcl/subcmd.tcl.in` |
| 126 | + - Example commit: :ghcommit:`3ad5a888` (:option:`--terse` on |
| 127 | + :subcmd:`whatis` and :subcmd:`search`) |
| 128 | + |
| 129 | +#. Describe the switch in the usage message reported by ``reportUsage``: in |
| 130 | + its *Switches* section and in the synopsis of each sub-command accepting |
| 131 | + it. When it is an existing switch that is accepted on an additional |
| 132 | + sub-command, only the sub-command synopsis is updated. |
| 133 | + |
| 134 | + - File to edit: :file:`tcl/report.tcl.in` |
| 135 | + - Example commits: :ghcommit:`a1fb42fd`, :ghcommit:`3ad5a888` |
| 136 | + |
| 137 | +Initialization scripts |
| 138 | +---------------------- |
| 139 | + |
| 140 | +New switch should be referred in the shell completion scripts. Top-level |
| 141 | +switches are listed in the ``comp_opts`` or ``comp_long_opts`` variables of |
| 142 | +:file:`init/Makefile` and sub-command switches in the ``comp_<subcmd>_opts`` |
| 143 | +variable of each sub-command accepting it. These variables are substituted |
| 144 | +in the *bash* and *tcsh* completion scripts, which thus only need an edit if |
| 145 | +no ``comp_<subcmd>_opts`` variable exists yet for a supporting sub-command. |
| 146 | +A switch accepting a value in the ``--name=VALUE`` form is listed twice, as |
| 147 | +``--name`` and ``--name=``. |
| 148 | + |
| 149 | +Files that should be edited to add reference to the new switch: |
| 150 | + |
| 151 | +- :file:`init/Makefile` |
| 152 | +- :file:`init/bash_completion.in` and :file:`init/tcsh_completion.in` (only |
| 153 | + if a supporting sub-command has no ``comp_<subcmd>_opts`` variable yet) |
| 154 | +- :file:`init/fish_completion` |
| 155 | +- :file:`init/zsh-functions/_module.in` |
| 156 | + |
| 157 | +Example commits: |
| 158 | + |
| 159 | +- top-level switch: :ghcommit:`372de708` |
| 160 | +- sub-command switch: :ghcommit:`fae13159` |
| 161 | +- existing switch accepted on additional sub-command: :ghcommit:`3ad5a888` |
| 162 | + |
| 163 | +Linter scripts |
| 164 | +-------------- |
| 165 | + |
| 166 | +Sub-command switches should be described in Nagelfar syntax database if they |
| 167 | +can be used on a :mfcmd:`module` command called within a modulefile context. |
| 168 | + |
| 169 | +File that should be edited to add reference to the new switch: |
| 170 | + |
| 171 | +- :file:`share/nagelfar/syntaxdb_modulefile.tcl` |
| 172 | + |
| 173 | + - add switch name in the ``::option(module <subcmd>)`` list of each |
| 174 | + sub-command accepting it (and its aliases, like ``add`` for ``load``) |
| 175 | + - add a ``::option(module <subcmd> --name)`` entry set to ``1`` if the |
| 176 | + switch accepts a value as a separate argument |
| 177 | + - See `Nagelfar syntax description`_ |
| 178 | + |
| 179 | +.. _Nagelfar syntax description: http://nagelfar.sourceforge.net/syntaxtokens.html |
| 180 | + |
| 181 | +Documentation |
| 182 | +------------- |
| 183 | + |
| 184 | +Man pages and other user documentation have to be updated to describe the |
| 185 | +introduced switch. |
| 186 | + |
| 187 | +Files that should be edited: |
| 188 | + |
| 189 | +- :file:`doc/source/module.rst` (module manpage) |
| 190 | + |
| 191 | + - add switch description with ``option`` anchor under *Command line |
| 192 | + switches* section, in alphabetical order of long form name, listing the |
| 193 | + sub-commands accepting it and with a ``versionadded`` directive |
| 194 | + - for a sub-command switch, add it to the synopsis of each sub-command |
| 195 | + accepting it in the ``subcmd`` directive under *Module Sub-Commands* |
| 196 | + section, describe its effect there and add a ``versionchanged`` |
| 197 | + directive |
| 198 | + - when an existing switch is accepted on an additional sub-command, update |
| 199 | + the list of sub-commands in its ``option`` description and add a |
| 200 | + ``versionchanged`` directive there too |
| 201 | + |
| 202 | +- :file:`doc/source/modulefile.rst` (modulefile manpage) |
| 203 | + |
| 204 | + - if the switch can be used on a :mfcmd:`module` command called within a |
| 205 | + modulefile context, describe it under :mfcmd:`module` modulefile command |
| 206 | + section and add a ``versionchanged`` directive |
| 207 | + |
| 208 | +- :file:`doc/source/changes.rst` |
| 209 | + |
| 210 | + - add switch to the table under *Command line switches* section of |
| 211 | + current Modules major version |
| 212 | + - when an existing switch is accepted on an additional sub-command, add a |
| 213 | + note after the table, under the switch name, instead |
| 214 | + - if the switch can be used on a :mfcmd:`module` command called within a |
| 215 | + modulefile context, also mention it under :mfcmd:`module` in the |
| 216 | + *Modulefile commands* section |
| 217 | + |
| 218 | +- :file:`NEWS.rst` |
| 219 | + |
| 220 | + - add an entry at the end of the current in-development version bullet |
| 221 | + list |
| 222 | + |
| 223 | +Example commits: |
| 224 | + |
| 225 | +- top-level switch: :ghcommit:`be5abb7a` |
| 226 | +- sub-command switch: :ghcommit:`98252083` |
| 227 | +- existing switch accepted on additional sub-command: :ghcommit:`3ad5a888` |
| 228 | + |
| 229 | +Testsuite |
| 230 | +--------- |
| 231 | + |
| 232 | +Non-regression testsuite must be adapted first to ensure existing tests still |
| 233 | +pass then to add specific tests to check the behavior of the added switch and |
| 234 | +ensure overall code coverage does not drop. |
| 235 | + |
| 236 | +#. For a top-level switch, test it in long and short form when placed before |
| 237 | + a sub-command name. |
| 238 | + |
| 239 | + - File to edit: :file:`testsuite/modules.00-init/090-switches.exp` |
| 240 | + |
| 241 | +#. For a sub-command switch, test it placed before and after the sub-command |
| 242 | + name, and test the *Unsupported option* error obtained when used on a |
| 243 | + sub-command that does not accept it. If the switch is instead passed |
| 244 | + through as a regular argument on sub-commands that do not accept it (like |
| 245 | + ``--append`` is), test the resulting error there. |
| 246 | + |
| 247 | + - Files to edit: |
| 248 | + |
| 249 | + - the testsuite file dedicated to the switch (see below) |
| 250 | + - :file:`testsuite/modules.00-init/071-args.exp` (switch passed |
| 251 | + through as a regular argument) |
| 252 | + |
| 253 | + - Example commit: :ghcommit:`9f7a2871` |
| 254 | + |
| 255 | +#. If the switch accepts a value, test the error obtained when value is |
| 256 | + missing or invalid. Error message helper procedures, like |
| 257 | + ``err_misoptval``, are defined in the testsuite initialization file. |
| 258 | + |
| 259 | + - Files to edit: |
| 260 | + |
| 261 | + - :file:`testsuite/modules.00-init/005-init_ts.exp` |
| 262 | + - the testsuite file dedicated to the switch (see below) |
| 263 | + |
| 264 | + - Example commit: :ghcommit:`9f7a2871` |
| 265 | + |
| 266 | +#. If the switch can be used on a :mfcmd:`module` command called within a |
| 267 | + modulefile context, test it from such context. If the Nagelfar syntax |
| 268 | + database has been updated, check with the :subcmd:`lint` sub-command that |
| 269 | + a modulefile using the switch reports no issue. |
| 270 | + |
| 271 | + - Files to edit: |
| 272 | + |
| 273 | + - new test file to create in :file:`testsuite/modules.50-cmds` directory |
| 274 | + - fixture modulefiles to add in the highest-numbered |
| 275 | + :file:`testsuite/modulefiles.N` directory (see |
| 276 | + :ref:`devel-testsuite`) |
| 277 | + - :file:`testsuite/modules.70-maint/420-lint.exp` |
| 278 | + |
| 279 | + - Example commit: :ghcommit:`36dd47f1` |
| 280 | + |
| 281 | +#. Craft specific tests to validate the correct behavior of the switch on |
| 282 | + each sub-command accepting it, combined with the other switches these |
| 283 | + sub-commands accept (for instance the :option:`--json` output mode or the |
| 284 | + :option:`--all` hidden module inclusion). |
| 285 | + |
| 286 | + - File to edit: depends on what is impacted by the new switch, if some |
| 287 | + tests already exist for concerned behavior (existing testsuite file to |
| 288 | + update) or not (new testfile to create, in |
| 289 | + :file:`testsuite/modules.70-maint` for a top-level switch). |
| 290 | + |
| 291 | + - Example commits: :ghcommit:`a083af06` (new test file), |
| 292 | + :ghcommit:`3ad5a888` (existing test files updated) |
| 293 | + |
| 294 | +.. vim:set tabstop=2 shiftwidth=2 expandtab autoindent: |
0 commit comments