Restore compatibility with docopt 0.6.2 docstrings

[h4l]

This is my proposal to fix #33. (With the assumption that compatibility with docopt is a goal of docopt-ng.)

This PR adds two new functions, parse_docstring_sections() and parse_options(); and uses them to parse docstrings accepted by docopt 0.6.2, while retaining docopt-ng's improvements to supported syntax.

Currently, docopt-ng parses option-defaults using a strategy that was in docopt's master branch, but considered unstable by the author, and was not released in docopt. It looks for option descriptions in an "options:" section, which is ended on the first blank line. This has the side-effect that options defined in a man-page style — with blank lines in-between — are not found. Neither are options outside an options: section (docopt allows options to follow the usage with no section heading).

parse_docstring_sections() is used to separate the usage section from the rest of the docstring. The text before the usage is ignored. The usage body (without its header) is parsed for the argument pattern and the usage header with its body is used to print the usage summary help. The text following the usage is parsed for options descriptions, using parse_options(), which supports option the description syntax of both docopt and the current docopt-ng.

Note that docopt 0.6.2 recognises option descriptions in the text prior to the usage section, but this change does not, as it seems like an unintended side-effect of the previous parser's implementation, and seems unlikely to be used in practice.

The testcases have two cases added for docopt 0.6.2 compatibility.

The first commit ("Fix test for missing arg before --") could be merged separately, but my final commit's tests depends on it.

[h4l]

Just pushed a change to import Sequence from typing instead of collections.abc as that won't work on Python 3.7.

[h4l]

Related issue in the docopt repo: https://github.com/docopt/docopt/issues/259

[NickCrews]

@h4l sorry I just left a few thoughts but I don't think I'm done with a review yet. Let me look at this a bit more before you go and rework things, in case I find anything else

[h4l]

Oh ok, no probs. No rush.

[codecov[bot]]

Codecov Report

Merging #36 (1cdb833) into master (6b8f84e) will decrease coverage by 0.00%. The diff coverage is 100.00%.

:exclamation: Current head 1cdb833 differs from pull request most recent head abf13dd. Consider uploading reports for the commit abf13dd to get more accurate results

@@            Coverage Diff             @@
##           master      #36      +/-   ##
==========================================
- Coverage   99.79%   99.79%   -0.01%     
==========================================
  Files           1        1              
  Lines         492      487       -5     
==========================================
- Hits          491      486       -5     
  Misses          1        1              

Impacted Files	Coverage Δ
docopt/__init__.py	99.79% <100.00%> (-0.01%)	:arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[h4l]

CI should pass now, I fixed the 3.7/3.8 compatibility issue.

I should have covered everything, but need to discuss the -not-an-option thing and behaviour of the usage_pattern regex.

[h4l]

Oh, also, fixing the testcases.docopt tests revealed an option description parsing bug which turned out to be a pre-existing issue, see commit 45f2a19 here.

[h4l]

The simple fixes are not-yet-squashed fixup commits, I can rebase/squash to cleanup the history before merging as and when we're ready.

[NickCrews]

@h4l those fixups look great! Just a few more tiny requests. The large one ("what terminates an --option?") can be a separate PR if you want to just get this in (My head is starting to hurt trying to keep all of these changes in my head) but the small ones should be included in this PR. Squashing the fixups would be nice, but if it's a pain then I'm fine with them staying in there.

[h4l]

@NickCrews Great, thanks for all your help reviewing this, it's really helpful. And sorry to make things trickier with that fix.

That's the "fix: handle options with descriptions on new line" commit, right? I'd be happy to take it out for another PR. The only trouble is that one of the testcases.docopt tests fails because of it — the --bar option gets mis-parsed in this example:

r"""Usage: prog [options]

global options: --foo
local options: --baz
               --bar
other options:
 --egg
 --spam
-not-an-option-
"""

We could take out the fix and mark this test as xfail to correct later.

FWIW the change shouldn't actually change the definition of how far an option description (the part with the [default: ]) extends, it only changes where the initial part (-f , --file FILE) ends.

What's happening is that other and options: are parsed as being arguments for --bar. So bar becomes default None/null, which causes the test to fail, because it expects it to be False. The specific example in this test doesn't happen with docopt-ng master because on master the other options: line is a section heading which stops --bar from seeing it. With this PR, options: headings are no longer significant, so --bar can consume it.

But the same general issue can occur on master:

In [1]: from docopt import parse_defaults

In [2]: parse_defaults("""
   ...: options:
   ...:  -f
   ...:  BLAH BLAH --other-option
   ...: """)
Out[2]: [Option('-f', '--other-option', 1, None)]

I don't think this fix should affect backwards compatibility, as the initial part this affects was always intended be on a single line, and putting them on multiple lines only "works" using this contrived syntax (the BLAH BLAH here stops the --other-option becoming a second option definition). And if the BLAH BLAH is indented more than 1 space, it doesn't count as an argument to -f either.

[h4l]

@NickCrews I fixed those two things. Let me know what you'd like to do with the option defaults newline fix.

I pushed a separate branch to my repo with the fixups squashed out as an example: docopt-0-6-2-compat_squashed.

[h4l]

@NickCrews I've updated this PR's branch to the squashed & rebased version (same as #38) so you can merge here if you're happy.

[NickCrews]

Brilliant, thank you so much @h4l! I'll close #38 as it was superseded by this.

OK, so now I'm trying to summarize what were the changes, to determine if version number should be bumped as breaking, and what to put in changelog:

1. BREAKING Now any line starting with - or -- is treated as an option. This means that some things that did not used to be treated as options, now ARE treated as options: a. lines before usage b. non-indented --options c. lines not inside the options: section
2. NONBREAKING now allow for blank lines between options
3. NONBREAKING now allow options descriptions to be immediately wrapped, don't have to have a two-space separator
4. BREAKING (?) different errors raised for empty usage section

Am I missing anything? Woudl you describe this any differently?

[h4l]

Fantastic, thanks for your help with this @NickCrews, I really appreciate it, especially as this turned out to be quite a significant change!

I'd want to frame the changes in the context of restoring compatibility with 0.6.2, to make sure people understand why the behaviour has changed. This also implies the changes shouldn't be backwards incompatible/breaking for people who were using original docopt.

Can also link to #33 which was fixed by this PR.

---

1. This looks good.

2. Yep.

3. I'd phrase this a little differently — it's more of a fix for an edge case that most likely nobody would have encountered. Could be summarised as:

   Options in the options: section without an argument, with descriptions on the following line, indented with only one space, are no longer interpreted as having an argument.

   If you want to include an example, docopt-ng would previously interpret --foo as having an argument here:

   options:
       --foo
    Enable the foo behaviour. (There's one space before "Enable", and 
      "Enable" was interpreted as the argument of --foo.)
   

   Whereas with this definition, --foo would be a regular boolean option as expected:

   options:
       --foo
     Enable the foo behaviour. (2 space before "Enable".)
   

   Now, docopt-ng interprets --foo as being a regular boolean option with both of these examples .

4. The actual exception type is the same (DocoptLanguageError), just the exception's message has changed a bit. __all__ doesn't advertise DocoptLanguageError at the moment, so I don't think this constitutes an API change from the POV of a developer using docopt-ng. Previously, a usage section without any content would be treated as if it didn't exist, so the error would be that no usage section was found. Now the error says that a usage section was found, but is empty.

---

If you want to be very strict, in the PR we also stopped allowing usage section headings without a regex word break before usage: (like your sausage: example). I don't think this really needs mentioning though.

While rebasing the other PR I notice we've also fixed #9 here. That could be phrased as:

• docs with "options:" headings containing 3 or more words no longer fail with an AssertionError. (e.g. "Very Advanced Options:")

[h4l]

Because it's still v0, under semver, the version bump could be minor if you'd rather not do v1 yet.

[NickCrews]

Awesome, thanks for helping me wrap my head around it. I'll bump from 0.8.1 to 0.9.0 and add this summary to the CHANGELOG

[h4l]

Great, thanks for handling that!