Empty metavar wreaks havoc on an `assert` statement

[cardoso-neto]

Here I have a very slippery bug.

Using metavar="" on that --branch argument on the subparser makes the argparse assert ' '.join(opt_parts) == opt_usage assertion (see it in context here) false because Tap doesn't send any item on the metavar instead of an item with an empty string.

opt_parts array with metavar="":

[..., "--branch", "[-h]", ...]

Without:

[ ...,  "--branch", "BRANCH", ...]

Code for reproducing:

from tap import Tap
from typing import Literal

class InstallsArgParser(Tap):
    org: str
    app: Literal["cycode"]
    branch: str

    def configure(self) -> None:
        super().configure()
        self.add_argument(
            "--branch",
            metavar="",  # very unusual error
        )
        self.add_argument(
            "--limit",
            nargs="+",
        )

class GeneralArgParser(Tap):
    def configure(self) -> None:
        super().configure()
        self.add_subparsers(help="Choose an action.", dest="command")
        self.add_subparser(
            "installs", InstallsArgParser, help="App installations on repos."
        )

GeneralArgParser().parse_args()

Then run it on the command line with $ python file.py installs.

And before you ask, yes I tried removing the seemingly extraneous lines of code, but then the bug would run away! lol

[swansonk14]

Hi @cardoso-neto,

Thank you for bringing up this very unusual bug! We were certainly confused for a while. We did some debugging, and we believe that the error is actually due to a bug in argparse and thus manifests itself in Tap. For example, consider the following code:

from argparse import ArgumentParser

parser = ArgumentParser()
subparsers = parser.add_subparsers()
subparser = subparsers.add_parser('a')
subparser.add_argument('--b', metavar='', required=True)
subparser.add_argument('--c')
args = parser.parse_args()

If you run this code (python file.py a) and make your terminal window very narrow, then you'll get the same error. The problem seems to derive from this line in argparse, which compares the length of the text of the arguments to the terminal window size and tries to wrap the text if the terminal window is too narrow. The code that does the text wrapping works incorrectly when metavar='' and results in the assertion error that you mentioned. Increasing the width of the terminal window, using either this argparse code or the Tap code that you posted, resolves the error.

Since this error appears to be a problem in argparse, I don't think we'll be able to fix it in Tap unfortunately. We will reach out to the argparse developers about this issue, and please feel free to contact them yourself as well to get this issue fixed!

Thanks again for bringing our attention to this very subtle bug!

Best, Kyle and Jesse

[cardoso-neto]

You guys are definitely more qualified than me to report this error to them, but if you link the issue here, I'll make sure to engage there as well. Thank you for your attention on this issue and for all your work writing this slick library.