Repository: valohai/valohai-cli

Issue:
Step parameters with spaces don't work
Passing step parameters with spaces does't work, it does use such default parameters fine though.

Maybe we should require a separate `id` attribute in params for this? Or should we just slugify the name? Or enforce more strict naming for parameters?

Example below.

===========

```
(valohai-cli) bog:darknet-example ruksi$ vh exec run --step="Generate text" --'Textual Seed'=1
Usage: vh execution run [OPTIONS] [ARGS]...

Error: no such option: --Textual Seed
```

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/valohai/valohai-cli/pull/33?src=pr&el=h1) Report
> Merging [#33](https://codecov.io/gh/valohai/valohai-cli/pull/33?src=pr&el=desc) into [master](https://codecov.io/gh/valohai/valohai-cli/commit/b909441d803e87ff45f51d34e40f3aed396bd1a8?src=pr&el=desc) will **increase** coverage by `0.09%`.
> The diff coverage is `91.8%`.

[![Impacted file tree graph](https://codecov.io/gh/valohai/valohai-cli/pull/33/graphs/tree.svg?src=pr&token=xqgKRx94XH&width=650&height=150)](https://codecov.io/gh/valohai/valohai-cli/pull/33?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
+ Coverage   89.39%   89.49%   +0.09%     
==========================================
  Files          79       81       +2     
  Lines        2056     2113      +57     
  Branches      262      271       +9     
==========================================
+ Hits         1838     1891      +53     
- Misses        131      133       +2     
- Partials       87       89       +2
```


| [Impacted Files](https://codecov.io/gh/valohai/valohai-cli/pull/33?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [valohai\_cli/utils/cli\_utils.py](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvdXRpbHMvY2xpX3V0aWxzLnB5) | `70.58% <ø> (ø)` | |
| [valohai\_cli/utils/\_\_init\_\_.py](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvdXRpbHMvX19pbml0X18ucHk=) | `81.31% <ø> (ø)` | |
| [valohai\_cli/commands/execution/run.py](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvY29tbWFuZHMvZXhlY3V0aW9uL3J1bi5weQ==) | `81.51% <100%> (+2.87%)` | :arrow_up: |
| [valohai\_cli/commands/project/link.py](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvY29tbWFuZHMvcHJvamVjdC9saW5rLnB5) | `91.48% <100%> (ø)` | :arrow_up: |
| [valohai\_cli/yaml\_wizard.py](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkveWFtbF93aXphcmQucHk=) | `95.23% <100%> (ø)` | :arrow_up: |
| [tests/commands/execution/test\_run.py](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree#diff-dGVzdHMvY29tbWFuZHMvZXhlY3V0aW9uL3Rlc3RfcnVuLnB5) | `100% <100%> (ø)` | :arrow_up: |
| [valohai\_cli/git.py](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvZ2l0LnB5) | `88.23% <50%> (+0.73%)` | :arrow_up: |
| [valohai\_cli/utils/levenshtein.py](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvdXRpbHMvbGV2ZW5zaHRlaW4ucHk=) | `80% <80%> (ø)` | |
| [valohai\_cli/utils/friendly\_option\_parser.py](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvdXRpbHMvZnJpZW5kbHlfb3B0aW9uX3BhcnNlci5weQ==) | `90% <90%> (ø)` | |
| ... and [1 more](https://codecov.io/gh/valohai/valohai-cli/pull/33/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/valohai/valohai-cli/pull/33?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/valohai/valohai-cli/pull/33?src=pr&el=footer). Last update [b909441...8a10837](https://codecov.io/gh/valohai/valohai-cli/pull/33?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/valohai_cli/commands/execution/run.py b/valohai_cli/commands/execution/run.py
index d266ee9..83527ec 100644
--- a/valohai_cli/commands/execution/run.py
+++ b/valohai_cli/commands/execution/run.py
@@ -1,3 +1,5 @@
+import re
+
 import click
 from click.exceptions import BadParameter
 from click.globals import get_current_context
@@ -9,10 +11,29 @@ import valohai_cli.git as git  # this import style required for tests
 from valohai_cli.adhoc import create_adhoc_commit
 from valohai_cli.api import request
 from valohai_cli.ctx import get_project
-from valohai_cli.messages import success, warn, error
+from valohai_cli.utils.friendly_option_parser import FriendlyOptionParser
+from valohai_cli.messages import success, warn
 from valohai_cli.utils import humanize_identifier, match_prefix
 
 
+def sanitize_name(name):
+    return re.sub(r'[_ ]', '-', name)
+
+
+def generate_sanitized_options(name):
+    seen = set()
+    for choice in (
+        '--%s' % name,
+        '--%s' % sanitize_name(name),
+        ('--%s' % sanitize_name(name)).lower(),
+    ):
+        if ' ' in choice:
+            continue
+        if choice not in seen:
+            seen.add(choice)
+            yield choice
+
+
 class RunCommand(click.Command):
     """
     A dynamically-generated subcommand that has Click options for parameters and inputs.
@@ -47,7 +68,7 @@ class RunCommand(click.Command):
         self.image = image
         self.watch = bool(watch)
         super(RunCommand, self).__init__(
-            name=step.name.lower().replace(' ', '-'),
+            name=sanitize_name(step.name.lower()),
             callback=self.execute,
             add_help_option=True,
         )
@@ -65,9 +86,7 @@ class RunCommand(click.Command):
         """
         assert isinstance(parameter, Parameter)
         option = click.Option(
-            param_decls=[
-                '--%s' % parameter.name.replace('_', '-'),
-            ],
+            param_decls=list(generate_sanitized_options(parameter.name)),
             required=(parameter.default is None and not parameter.optional),
             default=parameter.default,
             help=parameter.description,
@@ -85,9 +104,7 @@ class RunCommand(click.Command):
         """
         assert isinstance(input, Input)
         option = click.Option(
-            param_decls=[
-                '--%s' % input.name.replace('_', '-'),
-            ],
+            param_decls=list(generate_sanitized_options(input.name)),
             required=(input.default is None and not input.optional),
             default=input.default,
             metavar='URL',
@@ -154,6 +171,14 @@ class RunCommand(click.Command):
 
         return commit
 
+    def make_parser(self, ctx):
+        parser = super(RunCommand, self).make_parser(ctx)
+        # This is somewhat naughty, but allows us to easily hook into here.
+        # Besides, FriendlyOptionParser does inherit from OptionParser anyway,
+        # and just overrides that one piece of behavior...
+        parser.__class__ = FriendlyOptionParser
+        return parser
+
 
 @click.command(context_settings=dict(ignore_unknown_options=True), add_help_option=False)
 @click.argument('step')
diff --git a/valohai_cli/commands/project/link.py b/valohai_cli/commands/project/link.py
index 430c986..f699516 100644
--- a/valohai_cli/commands/project/link.py
+++ b/valohai_cli/commands/project/link.py
@@ -1,7 +1,7 @@
 import click
 
 from valohai_cli.api import request
-from valohai_cli.cli_utils import prompt_from_list
+from valohai_cli.utils.cli_utils import prompt_from_list
 from valohai_cli.commands.project.create import create_project
 from valohai_cli.consts import yes_option
 from valohai_cli.ctx import get_project, set_project_link
diff --git a/valohai_cli/git.py b/valohai_cli/git.py
index 47e4664..95c7776 100644
--- a/valohai_cli/git.py
+++ b/valohai_cli/git.py
@@ -1,3 +1,4 @@
+import os
 import subprocess
 
 from valohai_cli.exceptions import NoGitRepo
@@ -10,9 +11,10 @@ def check_git_output(args, directory):
             cwd=directory,
             shell=False,
             stderr=subprocess.STDOUT,
+            env=dict(os.environ, LC_ALL='C'),
         )
     except subprocess.CalledProcessError as cpe:
-        if cpe.returncode == 128 and 'Not a git repository' in cpe.output.decode():
+        if cpe.returncode == 128 and 'not a git repository' in cpe.output.decode().lower():
             raise NoGitRepo(directory)
         raise
 
diff --git a/valohai_cli/utils.py b/valohai_cli/utils/__init__.py
similarity index 100%
rename from valohai_cli/utils.py
rename to valohai_cli/utils/__init__.py
diff --git a/valohai_cli/cli_utils.py b/valohai_cli/utils/cli_utils.py
similarity index 100%
rename from valohai_cli/cli_utils.py
rename to valohai_cli/utils/cli_utils.py
diff --git a/valohai_cli/utils/friendly_option_parser.py b/valohai_cli/utils/friendly_option_parser.py
new file mode 100644
index 0000000..0c8c134
--- /dev/null
+++ b/valohai_cli/utils/friendly_option_parser.py
@@ -0,0 +1,26 @@
+from click import OptionParser, NoSuchOption
+
+from .levenshtein import levenshtein
+
+
+class FriendlyOptionParser(OptionParser):
+    """
+    A friendlier version of OptionParser that uses Levenshtein distances to figure out
+    if the user has just misspelled an option name.
+    """
+    def _match_long_opt(self, opt, explicit_value, state):
+        try:
+            return super(FriendlyOptionParser, self)._match_long_opt(opt, explicit_value, state)
+        except NoSuchOption as nse:
+            if not nse.possibilities:
+                # No possibilities were guessed, so attempt some deeper magic
+                nse.possibilities = [
+                    word
+                    for word
+                    in self._long_opt
+                    if levenshtein(
+                        word.lower().lstrip('-'),
+                        nse.option_name.lower().lstrip('-'),
+                    ) <= 4
+                ]
+            raise
diff --git a/valohai_cli/utils/levenshtein.py b/valohai_cli/utils/levenshtein.py
new file mode 100644
index 0000000..940b974
--- /dev/null
+++ b/valohai_cli/utils/levenshtein.py
@@ -0,0 +1,24 @@
+#!/usr/bin/env python
+
+# From http://hetland.org/coding/python/levenshtein.py, which is in the public domain.
+
+
+def levenshtein(a, b):
+    "Calculates the Levenshtein distance between a and b."
+    n, m = len(a), len(b)
+    if n > m:
+        # Make sure n <= m, to use O(min(n,m)) space
+        a, b = b, a
+        n, m = m, n
+
+    current = range(n + 1)
+    for i in range(1, m + 1):
+        previous, current = current, [i] + [0] * n
+        for j in range(1, n + 1):
+            add, delete = previous[j] + 1, current[j - 1] + 1
+            change = previous[j - 1]
+            if a[j - 1] != b[i - 1]:
+                change = change + 1
+            current[j] = min(add, delete, change)
+
+    return current[n]
diff --git a/valohai_cli/yaml_wizard.py b/valohai_cli/yaml_wizard.py
index b18d36b..a00fdf3 100644
--- a/valohai_cli/yaml_wizard.py
+++ b/valohai_cli/yaml_wizard.py
@@ -5,7 +5,7 @@ import click
 import requests
 import yaml
 
-from valohai_cli.cli_utils import prompt_from_list
+from valohai_cli.utils.cli_utils import prompt_from_list
 from valohai_cli.messages import error, success, warn
 from valohai_cli.utils import find_scripts
 


Test patch:
diff --git a/tests/commands/execution/test_run.py b/tests/commands/execution/test_run.py
index 2b90142..fc78001 100644
--- a/tests/commands/execution/test_run.py
+++ b/tests/commands/execution/test_run.py
@@ -117,3 +117,35 @@ def test_run_no_git(runner, logged_in_and_linked):
     with RunAPIMock(project_id, None, {}):
         output = runner.invoke(run, args, catch_exceptions=False).output
         assert 'is not a Git repository' in output
+
+
+def test_param_input_sanitization(runner, logged_in_and_linked):
+    with open(get_project().get_config_filename(), 'w') as yaml_fp:
+        yaml_fp.write('''
+- step:
+    name: Train model
+    image: busybox
+    command: "false"
+    inputs:
+      - name: Ridiculously Complex Input_Name
+        default: http://example.com/
+    parameters:
+      - name: Parameter With Highly Convoluted Name
+        pass-as: --simple={v}
+        type: integer
+        default: 1
+''')
+    output = runner.invoke(run, ['train', '--help'], catch_exceptions=False).output
+    assert '--Parameter-With-Highly-Convoluted-Name' in output
+    assert '--parameter-with-highly-convoluted-name' in output
+    assert '--Ridiculously-Complex-Input-Name' in output
+    assert '--ridiculously-complex-input-name' in output
+
+
+def test_typo_check(runner, logged_in_and_linked):
+    with open(get_project().get_config_filename(), 'w') as yaml_fp:
+        yaml_fp.write(CONFIG_YAML)
+    args = ['train', '--max-setps=80']  # Oopsy!
+    output = runner.invoke(run, args, catch_exceptions=False).output
+    assert '(Possible options:' in output
+    assert '--max-steps' in output