Moving a2x from optparse to argparse

[aerostitch]

Tested the following elements, no issue after the change:

• display help
• display version
• dry run on the conversion of an asciidoc file
• conversion of an asciidoc file
• refusing to run if a 2nd file is provided

Note the change from eval(str(...)) to the use of the builtin vars(...) that provides the same result but safer.
Along with #33 it closes: #4

[aerostitch]

Help output:

$ python3 a2x.py -h
usage: usage: a2x.py [OPTIONS] ASCIIDOC_FILE

A toolchain manager for AsciiDoc (converts Asciidoc text files to other file
formats)

positional arguments:
  asciidoc_file         AsciiDoc source file

optional arguments:
  -h, --help            show this help message and exit
  --version             show program's version number and exit
  -a ATTRIBUTE, --attribute ATTRIBUTE
                        set asciidoc attribute value
  --asciidoc-opts ASCIIDOC_OPTS
                        asciidoc options
  --copy                DEPRECATED: does nothing
  --conf-file CONF_FILE
                        configuration file
  -D PATH, --destination-dir PATH
                        output directory (defaults to ASCIIDOC_FILE directory)
  -d DOCTYPE, --doctype DOCTYPE
                        article, manpage, book
  -b BACKEND, --backend BACKEND
                        name of backend plugin
  --epubcheck           check EPUB output with epubcheck
  -f FORMAT, --format FORMAT
                        chunked, epub, htmlhelp, manpage, pdf, text, xhtml,
                        dvi, ps, tex, docbook
  --icons               use admonition, callout and navigation icons
  --icons-dir PATH      admonition and navigation icon directory
  -k, --keep-artifacts  do not delete temporary build files
  --lynx                use lynx to generate text files
  -L, --no-xmllint      do not check asciidoc output with xmllint
  -n, --dry-run         just print the commands that would have been executed
  -r PATH, --resource PATH
                        resource file or directory containing resource files
  -m FILE, --resource-manifest FILE
                        read resources from FILE
  --resource-dir PATH   DEPRECATED: use --resource
  -s, --skip-asciidoc   DEPRECATED: redundant
  --stylesheet STYLESHEET
                        HTML CSS stylesheet file name
  --safe                DEPRECATED: does nothing
  --dblatex-opts DBLATEX_OPTS
                        dblatex options
  --backend-opts BACKEND_OPTS
                        backend plugin options
  --fop                 use FOP to generate PDF files
  --fop-opts FOP_OPTS   options for FOP pdf generation
  --xsltproc-opts XSLTPROC_OPTS
                        xsltproc options for XSL stylesheets
  --xsl-file XSL_FILE   custom XSL stylesheet
  -v, --verbose         increase verbosity

[elextr]

Looks ok to me, @MasterOdin ?

[MasterOdin]

LGTM

[eli-schwartz]

While trying to package asciidoc 9.x for my distro, I discovered this change resulted in a regression. Previously working software which depends on asciidoc to build documentation, stopped working with the following message:

/usr/bin/a2x --no-xmllint -d manpage -f manpage --xsltproc-opts '-param man.endnotes.list.enabled 0 -param man.endnotes.are.numbered 0' -D doc --asciidoc-opts '-f /home/eschwartz/git/archlinux/pacman/doc/asciidoc.conf -a pacman_version="5.2.1" -a pacman_date=2020-07-05 -a srcext=.src.tar.gz -a pkgext=.pkg.tar.gz -a pkgdatadir=/usr/share/pacman -a localstatedir=/var -a sysconfdir=/etc -a datarootdir=/usr/share -a rootdir=/' ../doc/alpm-hooks.5.asciidoc
usage: usage: a2x [OPTIONS] ASCIIDOC_FILE
a2x: error: argument --asciidoc-opts: expected one argument

Note the option

--asciidoc-opts '-f /home/eschwartz/git/archlinux/pacman/doc/asciidoc.conf -a pacman_version="5.2.1" -a pacman_date=2020-07-05 -a srcext=.src.tar.gz -a pkgext=.pkg.tar.gz -a pkgdatadir=/usr/share/pacman -a localstatedir=/var -a sysconfdir=/etc -a datarootdir=/usr/share -a rootdir=/'

This is caused by python bug https://bugs.python.org/issue9334

argparse is NOT a drop-in replacement for optparse, and quite frankly on a personal level I'm now disappointed in argparse and no longer wish to use it for things.

[elextr]

Try --asciidoc-opts='--blah blah'

[eli-schwartz]

This is a demonstration of the bug, not an assertion that I only have the bug in one place. I don't anticipate having fun testing around a hundred packages which have build-depends on asciidoc to check which ones might trigger a bug in a2x/argparse, then adding workarounds there, before I can package asciidoc.
I mean, yes, I could do it... but all I wanted was to have a nice happy migration from python2 to python3, and argparse is raining on that parade.

Though at least in the case of this one package, I will now have a) ugly code doing even more nested array joining and string .format()'ing in order to pass this argument, b) comments to warn people away from ever trying to simplify this.

Furthermore this is broken argument parsing behavior, but apparently no python developers care sufficiently about having correct behavior to fix this over the course of a decade, leading me to the inevitable conclusion that argparse itself will never be fixed. This is a general issue in that people using asciidoc might not expect the equals sign to be needed there, as it never was before and it breaks from common option parsing conventions. They will get strange errors from argparse but think those errors come from a2x.

[elextr]

This shows the = (and always showed it IIRC) which is about the best we can do since as you point out Python isn't going to change it, and optparse is deprecated (albeit still available for now).

Note that I am carefully not buying into the argument about which is correct, argparse or optparse, so I am not using terms like 'bug' or 'broken' for either behaviour :)

[eli-schwartz]

Note I would also need to protect:

--xsltproc-opts '-param man.endnotes.list.enabled 0 -param man.endnotes.are.numbered 0'

because the only reason that isn't an argparse error too, is because currently a2x doesn't accept a -p argument. If it ever added such an argument, then code invoking --xsltproc-opts '-param' would abruptly stop working, again depending on your version of asciidoc.

[eli-schwartz]

This shows the = (and always showed it IIRC)

I didn't notice that, but on the other hand the --help output shows it without the "=". Because argparse controls that. Anyway it's generally expected UX for tools in general that this should not make a difference and both forms work. And... both forms do work, superficially (until argparse misinterprets them). Nowhere does it say "you really need the equals sign, even though you might think it works without it".

which is about the best we can do since as you point out Python isn't going to change it, and optparse is deprecated (albeit still available for now).

FWIW, optparse is "documentation-only" deprecation and I believe it's not intended to ever remove it.

[elextr]

carefully not buying into the argument

Who am I kidding, I love a good syntax lawyer argument, so ... :)

Anyway it's generally expected UX for tools in general that this should not make a difference and both forms work.

Posix does not address long options so they are non-standard, so we need to turn to the glibc extension that AFAICT was the first to allow long arguments, so having priority for defining "correct" behaviour of long options.

Glibc specified a requirement for the value for a long argument to be attached by =, from the GNU getopt() docs: "To specify an argument for a long option, write ‘--name=value’. This syntax enables a long option to accept an argument that is itself optional.".

So AFAICT it was Python optparse() that added the non-standard behaviour for long options on its own, possibly accidentally because its how POSIX short options are defined to operate with arguments, and the same code was also used for long arguments without anybody noticing the implications. But it opens up the ambiguities that are happening here, and that the Glibc specification avoids.

Since optparse() has been around for a long time it might therefore be common for Python applications to accept such argument lists , but I would disagree that it is "generally expected UX behaviour" with non-python applications, and the non-standard behaviour has ambiguity that leads to these sorts of arguments [pun intended].

Although argparse is more standard in that it doesn't accept disconnected argument values starting with - it still perpetuates the non-standard behaviour otherwise, so its broken too, and inconsistent. At least the argparse() maintainers offered to accept a patch for optionally making it consistent.

And argparse() help is also broken, as you pointed out it does not show the = thus condoning and perpetuating non-standard behaviour, but its something we don't control AFAIK.

Agree that I wouldn't expect optparse to be removed before Python 4, but the change will occur at some time, and in between optparse is unlikely to get much love.

So all-in-all, for me, the whole Python argument parsing ecosystem is pretty much broken and non-standard and encourages users to use ambiguous commands without the = thus laying nasty traps for them in the future.

So finally getting back to Asciidoc, I would take the view that the Asciidoc documentation (RTFineM remember :) always showed the correct standard (for non-python apps) usage, and the fact that the non-standard usage happened to work was an artefact (or even a bug) not a deliberate feature. As such it can cease working at any time. It is unfortunate that the incorrect usage has propagated far and wide, but that is going to have to be fixed at some point.

I understand that there is effort in changing lots of uses and that takes time, so I might have suggested that the change be reverted and a deprecation period given to allow time for the erroneous commands in scripts to be fixed.

But since it seems nobody reads the asciidoc documentation anyway, I'm not sure how the deprecation would be communicated, especially for existing scripts buried inside other applications build systems, so nobody will notice such a deprecation and all it will do is delay the inevitable.

[end lawyering, I rest my case m'lud]

[MasterOdin]

Given the intention to make the 9.0 release a painless upgrade from 8.6.10, I agree on reverting this PR. I'll do this later on today in a 9.0.2 release as I agree this is a "bug" in the context of the above statement.

The rest of the conversation on usage syntax and such can be used as context for whether we do actually want to move to argparse from optparse until we're actually forced to so as to retain this behavior, and any such change would be for 10.0, to be included with other BC breaking changes.

[eli-schwartz]

Ping?

[MasterOdin]

Sorry about that @eli-schwartz, it ended up totally slipping my mind. I've now properly cut 9.0.2.