Restore compatibility with docopt 0.6.2 docstrings #36

h4l · 2022-08-27T11:23:42Z

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 · 2022-08-27T11:47:09Z

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

h4l · 2022-08-27T11:48:34Z

Related issue in the docopt repo: docopt/docopt#259

NickCrews · 2022-09-06T07:00:38Z

tests/test_docopt.py

-    assert parse_section("usage:", "foo bar fizz buzz") == []
-    assert parse_section("usage:", "usage: prog") == ["usage: prog"]
-    assert parse_section("usage:", "usage: -x\n -y") == ["usage: -x\n -y"]
-    assert parse_section("usage:", usage) == [


@h4l It looks like the constant usage isn't used anymore, should be deleted

Ah yes, I'll get rid of it.

remove unused usage constant

NickCrews · 2022-09-06T07:05:21Z

docopt/__init__.py

+    """
+    usage_pattern = r"""
+    # Any number of lines precede the usage section
+    \A(?P<before_usage>(?:.*\n)*?)


Isn't the ? redundant in the *? ? can't we just have *?

Ah, so this is a bit tricky. I thought the same looking at the original regexes docopt used, I assumed +? could just be *. But the ? acts as a modifier to make the * non-greedy . So the effect here is that we'll match as few lines as possible before a usage: line occurs.

Without the non-greedy operator it'd match as many lines as possible. This is actually still OK for correctness, as the match will initially consume all the lines, then backtrack until it finds a usage: line to match the usage section with, and succeed from there. However the effect of that is that the final usage: line in the doc will be used. With the non-greedy match, the first usage: section gets used.

So it only matters if a doc incorrectly has multiple usage sections. There is some code that tries to check for multiple usage sections, and raises an error if there are multiple (that's lint_docstring in this version). I just tried with the standard greedy match, and lint_docstring currently fails to notice the second usage section in this case, as it currently looks only in the usage body and the section after usage, not in the section before.

The same effect as the non-greedy match can be achieved in this case using a negative lookahead assertion in this before_usage group to not allow it to match lines containing usage:. That seems like a more clear approach, as non-greedy matching is pretty obscure.

Thank you, that makes sense. I think you're right, the negative lookahead is more clear, otherwise I'd want a comment here, and it would be great to not have a comment.

make usage_pattern regex more intuitive by using lookahead instead of non-greedy match

NickCrews · 2022-09-06T07:08:15Z

docopt/__init__.py

+    # Any number of lines precede the usage section
+    \A(?P<before_usage>(?:.*\n)*?)
+    # The `usage:` section header.
+    ^(?P<usage_header>.*usage:)


Probably overkill, but perhaps .*\busage: so that it won't match "ingredients in pork sausage:" ?

Good idea, I'll do it.

NickCrews · 2022-09-06T07:12:19Z

tests/test_docopt.py

+        """,
+    ],
+)
+def test_parse_docstring_sections__reports_invalid_docstrings(invalid_docstring: str):


how about a test case for usage: being present, but nothing after that?

""" program summary usage: """

Done, that test is now in test_lint_docstring.

NickCrews · 2022-09-06T07:17:27Z

docopt/__init__.py

+    # They can be indented with whitespace
+    [ \t]*
+    # The description itself starts with the short or long flag (-x or --xxx)
+    (-\S+?)


Same here, don't we want -\S+? We need one or more non-whitespace after the -?

Same story as the other with the non-greedy modifier, but looking at this again I don't think it's necessary. I think the repetition itself isn't needed. I'll get rid of it.

NickCrews · 2022-09-06T17:12:17Z

@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 · 2022-09-06T17:21:51Z

Oh ok, no probs. No rush.

NickCrews · 2022-09-06T18:25:31Z

tests/testcases.docopt

+    -i, --interactive    interactive picking
+    -p, --patch          select hunks interactively
+"""
+@pytest.mark.xfail


This should be $ prog --interactive?

This will always pass because no test case is actually generated.

Really, we need to fix conftest.py

docopt-ng/tests/conftest.py

Lines 14 to 27 in 88496b8

def parse_test(raw: str):

raw = re.compile("#.*$", re.M).sub("", raw).strip()

if raw.startswith('"""'):

raw = raw[3:]

for fixture in raw.split('r"""'):

doc, _, body = fixture.partition('"""')

cases = []

for case in body.split("$")[1:]:

argv, _, expect = case.strip().partition("\n")

expect = json.loads(expect)

prog, _, argv = argv.strip().partition(" ")

cases.append((prog, argv, expect))

yield doc, cases

Something like

if len(cases) == 0: raise DocoptTestException(f"Each example must have at least one test case starting with '$'. Example: {doc}")

Do you want to add this? I can add in followup commit as well

Ah, oops. That @pytest.mark.xfail line is a mistake too — I was playing with allowing marking cases with pytest marks (to allow xfailing cases, or tagging cases to run a subset) but then I pulled that out, but forgot to remove that line. And seems like I butchered the lines around there somehow.

I can put in a check for that, yep.

I added some error handling to conftest based on your suggestion, should be more robust against these kind of mistakes now! It also reports details of which test fails if there's a JSON syntax error in an test too.

NickCrews · 2022-09-06T18:45:08Z

tests/testcases.docopt

@@ -950,8 +950,35 @@ local options: --baz
 other options:
 --egg
 --spam
-not-an-option-


Don't delete this line

Ah, so the reason I got rid of this is that with docopt 0.6.2 this is actually a valid way to specify an option. In docopt-ng currently, option descriptions have to have leading whitespace, but not in 0.6.2.

I could put it back with a different name, like

-option-without-whitespace

although the "docopt 0.6.2 compatibility" test below also has options without leading whitespace.

gotcha. I guess it follows the line starting with - or -- rule, so you're right the other tests cover this case.

But, separate from the leading whitespace thing, I'm still hung up on the
-starts-with-single-dash-but-is-long, that isn't intuitive to me if that should be a long or short option.

Testing:

-long-solo -s-d --single--double --double-single -d-s

results in
{"-long-solo": false, "--single--double": false, "--double-single": false}
which I think makes sense, but probably should add a test case to make it official. Can add in follow up PR

Oh, I missed that there was only one dash on that option. The option description parsing code is very lenient, it just assumes the doc is formatted according to the spec, and does almost no validation. The spec does have quite a clear definition of short vs long option in general (not specifically for option descriptions):

Words starting with one or two dashes (with exception of "-", "--" by themselves) are interpreted as short (one-letter) or long options, respectively.

Short options can be "stacked" meaning that -abc is equivalent to -a -b -c.

And the usage pattern / argv parsers (the Pattern classes) do implement this behaviour (slight quirk with the hyphen):

>>> docopt.docopt('usage: prog [-single-dash]', []) {'--': False, '-a': False, '-d': False, '-e': False, '-g': False, '-h': False, '-i': False, '-l': False, '-n': False, '-s': 0}

And the argv parser/matcher doesn't recognise the the single dash option:

>>> docopt.docopt('usage: prog [options]\noptions: -single-dash\n', '-single-dash') usage: prog [options] options: -single-dash An exception has occurred, use %tb to see the full traceback.

I'd suggest following the behaviour implemented in the Pattern code, as it is significantly more detailed/careful than than the option default parsing code.

NickCrews · 2022-09-06T19:23:13Z

tests/test_docopt.py

 def test_issue_126_defaults_not_parsed_correctly_when_tabs():
    section = "Options:\n\t--foo=<arg>  [default: bar]"
-    assert parse_defaults(section) == [Option(None, "--foo", 1, "bar")]
+    assert parse_options(section) == [Option(None, "--foo", 1, "bar")]


How about we just add this as a test case to the cases below? Before, we didn't have this "unit" testing for parsing options, so it made sense to have this as a separate test. But now we do have very similar tests in test_parse_options, so just put this case in there too. Having it separate implies it is somehow special, but I think this is just another instance of those other cases we test.

remove redundant test test_issue_126_defaults_not_parsed_correctly_when_tabs (include its example in main options test)

docopt/__init__.py

NickCrews · 2022-09-06T19:51:40Z

tests/testcases.docopt

@@ -950,8 +950,35 @@ local options: --baz
 other options:


As you say in one of the commit messages:

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.

We need to change the documentation in the README that says

Every line in doc that starts with - or -- (not counting spaces) is treated as an option description, e.g.

Then we should add a test for this:

# lines before the usage section that start with -- # are not treated as options. r"""My CLI program --speed is ignored as an option usage: prog --direction """ # Note that "--speed" is not present $ prog --direction {"--direction": true}

Actually, docopt.org says "Every line that starts with "-" or "--" (not counting spaces) is treated as an option description, e.g.:" so I think we want to stick with that. People are going to be reading that spec, and it's so stable I don't want to mess with it.

That does really throw a wrench in this implementation. What do you think, re-work this implementation to allow for --options to be scattered anywhere?

It's not a problem to allow that. We just need to include sections.before_usage in the string that gets passed to parse_options(). I think you're right, better to stick to the 0.6.2 behaviour.

Include content in before_usage when parsing option descriptions

NickCrews · 2022-09-06T21:01:29Z

docopt/__init__.py

+        # The first line of the body may follow the header without a line break:
+        (?:
+            # Some non-whitespace content
+            [ \t]*\S.*(?:\n|\Z)


could we simplify (?:\n|\Z) to [\n\Z]?

No, we can't, the \Z magic doesn't work in character classes

docopt/__init__.py

codecov · 2022-09-07T01:04:35Z

Codecov Report

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

❗ 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%)`	⬇️

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

This implements Nick's suggestion from jazzband#36. When generating tests from testcases.docopt, we now raise an error if an example doc has no testcases following it to actually exercise the example. Also, tests with invalid JSON testcases are reported using the same error reporting mechanism.

h4l · 2022-09-07T12:23:55Z

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 · 2022-09-07T12:26:27Z

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 · 2022-09-07T12:32:15Z

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 · 2022-09-07T22:25:01Z

tests/test_docopt.py

+        """\
+         \t --foo ARG, -f ARG  With a long
+
+         wrapped description


@h4l what actually terminates an --option item? eg

--foo Intended description. Now I'm back to describing the usage of my program in general. I include [default: 42] on accident (OK this is very contrived)

should --foo have a default of 42? To me it looks like that option description ends on the single line and so it should not have that default.

Should we have the same behavior as when the usage: section terminates? eg the option description starts when we see r"(?: )|$" (as we currently have), and then continues for any number of indented lines?
Oh but then this needs to be modified to "continues for any number of indented lines as long as they don't start with - or --"...yuck

This isn't really solving a likely problem, so we could just leave it unspecified.

Yeah I think you answered it yourself, but the description continues until a line starts with [ \t]-\S. The previous and current code just splits the entire option description text block on lines starting in that way, and everything in between becomes part of the description for the option. So it's not at all nuanced in its handling of unusual stuff.

As for if the default should apply, I've certainly written option descriptions which extend over several lines. But I would keep them indented. Seems reasonable to end a description on a non-indented line, but it'd certainly require some thought.

Like I was mentioning in the other thread, the option description parsing code is really lenient. There was talk of defining a grammar for the docopt language in another issue — even if the implementation continued to use the hand-written parser, it'd be useful to have a grammar for the specification value of what is/isn't valid.

NickCrews · 2022-09-07T22:26:43Z

tests/test_docopt.py

+        pytest.param("", id="empty"),
+        pytest.param("This is a prog\n", id="1line"),
+        pytest.param(
+            "This is a prog\n\n" "Info:\n Blah blah\n\n"


weird little " " hole in the middle of this constant

Ah strange, probably due to Black formatting a wrapped string.

Fix split string literal

NickCrews · 2022-09-07T22:42:41Z

tests/test_docopt.py

+            Option("-e", None, 0, False),
+        ],
+    ),
+]


All of these are the "happy cases" where there truly are options. We should add cases for non-options, such as some text --foo

Good idea, I've added another case to test this.

NickCrews · 2022-09-07T22:48:28Z

@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 · 2022-09-08T07:52:22Z

@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.

This implements Nick's suggestion from jazzband#36. When generating tests from testcases.docopt, we now raise an error if an example doc has no testcases following it to actually exercise the example. Also, tests with invalid JSON testcases are reported using the same error reporting mechanism.

h4l · 2022-09-08T09:42:21Z

@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.

It was throwing an error due to the options: section occurring in usage:, but the test was passing because the message of the exception was not checked.

parse_docstring_sections() divides the docstring into the main non-overlapping sections that are to be further analysed. These are text before the usage, the usage header ("usage:" or similar), the usage body (" proc [options]") and text following the usage. The intention of this partitioning is to facilitate restoring the option description parsing behaviour of the last stable docopt release (0.6.2), while retaining the current improved behaviour of not requiring a blank line after the usage: section; and also removing a small quirk in the 0.6.2 parser, which is that option-defaults can occur before the usage: section. (But this partitioning provides access to the text before the usage section, so this behaviour could be retained if desired.)

parse_options() parses option descriptions in a way that is compatible with docopt 0.6.2, while also compatible with docopt-ng's current behaviour. The differences are: - docopt-ng requires option descriptions to be in an "options:" section, docopt allows them to be anywhere outside the "usage:" section. - docopt-ng requires options descriptions have leading whitespace, docopt allows them to start at column 0. - docopt-ng allows options descriptions to begin on the same line as a section heading, docopt does not. e.g. `options: --foo` is OK with docopt-ng. parse_options() parses options following either docopt or docopt-ng's behaviour. Although it expects the `docstring` argument to be a sub-section of the overall docstring, so the caller is in control of where in the docstring options are parsed from.

This commit uses parse_docstring_sections() and parse_options() 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. This fixes jazzband#33

This implements Nick's suggestion from jazzband#36. When generating tests from testcases.docopt, we now raise an error if an example doc has no testcases following it to actually exercise the example. Also, tests with invalid JSON testcases are reported using the same error reporting mechanism.

This fixes a pre-existing bug in docopt-ng that wasn't previously triggered by a test. Options in the options: section without a description on the same line and with content on the following line (e.g. a line-wrapped description) would be parsed as if the content on the following line was part of the option name & arguments. Option.parse() now ends the option name & arguments section on the end of the line, not just on two consecutive spaces.

It's a bit of an obscure and non-obvious regex feature. The regex uses a negative lookahead assertion now, which is still a little odd, but it's clearer I think.

The usage section heading now needs to have a word break before usage:, so e.g sausage: doesn't match as a usage: section.

lint_docstring() now checks for the usage_body being empty, and fails with a message indicating this.

parse_docstring_sections() is no longer responsible for rejecting docs with empty usage sections. Previously it did so to some extent, but failed to reject whitespace-only usage bodies. lint_docstring() now rejects empty usage bodies, which allows the regex's usage_body group to be simplified. The removed test for empty usage bodies is tested via test_lint_docstring.

Docopt 0.6.2 allows option descriptions anywhere in the doc, not just after the usage: section. With this change, we parse both the section prior to the usage: and the section after for option descriptions.

This adds another case for test_parse_options demonstrating how parse_options() parses docs which contain both options and text that mentions options that should not be interpreted as options. This includes an example (the '-b' option) where we do currently parse an option mentioned in prose as a real option. In principle we could fix this, but it's here for now to document current behaviour. And it's not hard to work around when writing a usage doc - just need to adjust where a line is wrapped.

h4l · 2022-09-08T17:53:56Z

@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.

This implements Nick's suggestion from #36. When generating tests from testcases.docopt, we now raise an error if an example doc has no testcases following it to actually exercise the example. Also, tests with invalid JSON testcases are reported using the same error reporting mechanism.

NickCrews · 2022-09-08T21:16:48Z

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:

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
NONBREAKING now allow for blank lines between options
NONBREAKING now allow options descriptions to be immediately wrapped, don't have to have a two-space separator
BREAKING (?) different errors raised for empty usage section

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

h4l · 2022-09-09T10:23:28Z

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.

This looks good.
Yep.
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 .
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 · 2022-09-09T13:26:25Z

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

NickCrews · 2022-09-09T16:46:09Z

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 · 2022-09-09T17:17:14Z

Great, thanks for handling that!

h4l force-pushed the docopt-0-6-2-compat branch from 0a2429a to b008bf1 Compare August 27, 2022 11:46

NickCrews reviewed Sep 6, 2022

View reviewed changes

docopt/__init__.py Show resolved Hide resolved

NickCrews reviewed Sep 6, 2022

View reviewed changes

docopt/__init__.py Show resolved Hide resolved

NickCrews reviewed Sep 7, 2022

View reviewed changes

This was referenced Sep 7, 2022

Assert error message content in tests #37

Merged

Report exceptions that cause failures in testcases.docopt #35

Merged

NickCrews mentioned this pull request Sep 8, 2022

Docopt 0.6.2 compat squashed #38

Closed

h4l added 14 commits September 8, 2022 17:21

Fix test for missing arg before --

904287a

It was throwing an error due to the options: section occurring in usage:, but the test was passing because the message of the exception was not checked.

refactor: avoid non-greedy matching in parse_docstring_sections

13f2c9d

It's a bit of an obscure and non-obvious regex feature. The regex uses a negative lookahead assertion now, which is still a little odd, but it's clearer I think.

fix: don't match usage-suffix words as usage sections

33c8528

The usage section heading now needs to have a word break before usage:, so e.g sausage: doesn't match as a usage: section.

chore: fix typo

428cda5

refactor: remove unnecessary non-greedy regex modifier

7e3117b

refactor: reject empty usage sections in lint_docstring

efd6017

lint_docstring() now checks for the usage_body being empty, and fails with a message indicating this.

fix: allow option descriptions before the usage: section

046c327

Docopt 0.6.2 allows option descriptions anywhere in the doc, not just after the usage: section. With this change, we parse both the section prior to the usage: and the section after for option descriptions.

h4l force-pushed the docopt-0-6-2-compat branch from 1cdb833 to abf13dd Compare September 8, 2022 17:52

NickCrews merged commit 850293b into jazzband:master Sep 8, 2022

h4l deleted the docopt-0-6-2-compat branch September 9, 2022 10:24

NickCrews mentioned this pull request Sep 9, 2022

Add 0.6.2 compat fix to CHANGELOG #41

Merged

	def parse_test(raw: str):
	raw = re.compile("#.*$", re.M).sub("", raw).strip()
	if raw.startswith('"""'):
	raw = raw[3:]

	for fixture in raw.split('r"""'):
	doc, _, body = fixture.partition('"""')
	cases = []
	for case in body.split("$")[1:]:
	argv, _, expect = case.strip().partition("\n")
	expect = json.loads(expect)
	prog, _, argv = argv.strip().partition(" ")
	cases.append((prog, argv, expect))
	yield doc, cases

Restore compatibility with docopt 0.6.2 docstrings #36

Restore compatibility with docopt 0.6.2 docstrings #36

Conversation

h4l commented Aug 27, 2022

h4l commented Aug 27, 2022

h4l commented Aug 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h4l Sep 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h4l Sep 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickCrews Sep 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickCrews commented Sep 6, 2022

h4l commented Sep 6, 2022

Choose a reason for hiding this comment

NickCrews Sep 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickCrews Sep 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h4l Sep 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Sep 7, 2022 • edited Loading

Codecov Report

h4l commented Sep 7, 2022

h4l commented Sep 7, 2022

h4l commented Sep 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h4l Sep 8, 2022 • edited Loading

Choose a reason for hiding this comment

NickCrews Sep 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickCrews commented Sep 7, 2022

h4l commented Sep 8, 2022 • edited Loading

h4l commented Sep 8, 2022

h4l commented Sep 8, 2022

NickCrews commented Sep 8, 2022

h4l commented Sep 9, 2022

h4l commented Sep 9, 2022

NickCrews commented Sep 9, 2022

h4l commented Sep 9, 2022

h4l Sep 7, 2022 •

edited

Loading

h4l Sep 7, 2022 •

edited

Loading

NickCrews Sep 6, 2022 •

edited

Loading

NickCrews Sep 6, 2022 •

edited

Loading

NickCrews Sep 6, 2022 •

edited

Loading

h4l Sep 7, 2022 •

edited

Loading

codecov bot commented Sep 7, 2022 •

edited

Loading

h4l Sep 8, 2022 •

edited

Loading

NickCrews Sep 7, 2022 •

edited

Loading

h4l commented Sep 8, 2022 •

edited

Loading