Just a quick note to mention that my big branch to adopt click for command line option parsing should now be done.
https://gitlab.com/mailman/mailman/merge_requests/292
This will close #319 and #346 and make adding new mailman subcommands much easier. (We still need the bits to define additional search paths, and probably some better documentation that would be part of a general “Extending Mailman” section.)
Along the way I think I’ve made several other improvements, including (I hope!) reducing or eliminating the occasional hangs we see on CI, speeding up the test suite a bit, and making things more robust.
Please feel free to review it and play with it. It’s finishing CI now but I’ll hold off on merging it for a day or two. I’m especially interested to hear what Jan thinks for the plugin work he’s doing.
The big downside could be that because this is such a big change, existing MPs may have to be rebased.
It’s a big branch with lots of little sweater threads that took longer than I expected, but I should be done now, and I think it will be a good improvement to the code.
Cheers, -Barry
On 07/20/2017 01:56 PM, Barry Warsaw wrote:
Just a quick note to mention that my big branch to adopt click for command line option parsing should now be done.
https://gitlab.com/mailman/mailman/merge_requests/292 ... Please feel free to review it and play with it. It’s finishing CI now but I’ll hold off on merging it for a day or two. I’m especially interested to hear what Jan thinks for the plugin work he’s doing.
The big downside could be that because this is such a big change, existing MPs may have to be rebased.
Thank Barry,
I'll look at it. I'll also rebase !301 and !202 after it lands, and I'll now delete my <https://gitlab.com/msapiro/mailman/tree/withlist> branch as this supercedes that.
-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan
On 07/20/2017 02:12 PM, Mark Sapiro wrote:
On 07/20/2017 01:56 PM, Barry Warsaw wrote:
Just a quick note to mention that my big branch to adopt click for command line option parsing should now be done.
I'll look at it. ...
The first thing I notice right away is the help text doesn't fill. E.g.,
$ ./mailman --help Usage: mailman [OPTIONS] COMMAND [ARGS]...
The GNU Mailman mailing list management system Copyright 1998-2017 by the Free Software Foundation, Inc. http://www.list.org
Options: -C, --config PATH Configuration file to use. If not given, the environment variable MAILMAN_CONFIG_FILE is consulted and used if set. If neither are given, a default configuration file is loaded. --version Show the version and exit. --help Show this message and exit.
Commands: ...
and
$ ./mailman shell --help Usage: mailman shell [OPTIONS] [RUN_ARGS]...
Operate on a mailing list.
For detailed help, see --details
Options: -i, --interactive Leaves you at an interactive prompt after all other processing is complete. This is the default unless the --run option is given. -r, --run TEXT Run a script on a mailing list. The argument is the module path to a callable. This callable will be imported and then called with the mailing list as the first argument. If additional arguments are given at the end of the command line, they are passed as subsequent positional arguments to the callable. For additional help, see --details. --details Print detailed instructions and exit. -l, --listspec TEXT A specification of the mailing list to operate on. This may be the posting address of the list, or its List-ID. The argument can also be a Python regular expression, in which case it is matched against both the posting address and List-ID of all mailing lists. To use a regular expression, LISTSPEC must start with a ^ (and the matching is done with re.match(). LISTSPEC cannot be a regular expression unless --run is given. --help Show this message and exit.
-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan
On 07/20/2017 02:26 PM, Mark Sapiro wrote:
The first thing I notice right away is the help text doesn't fill. E.g.,
$ ./mailman --help Usage: mailman [OPTIONS] COMMAND [ARGS]...
The GNU Mailman mailing list management system Copyright 1998-2017 by the Free Software Foundation, Inc. http://www.list.org
Options: -C, --config PATH Configuration file to use. If not given, the environment variable MAILMAN_CONFIG_FILE is consulted and used if set. If neither are given, a default configuration file is loaded. --version Show the version and exit. --help Show this message and exit.
...
I've looked at the click docs and I see that text from docstrings is wrapped and filled as is the text
"""\ The GNU Mailman mailing list management system Copyright 1998-2017 by the Free Software Foundation, Inc. http://www.list.org """
which wraps and fills as above, nd there is even a control to turn that off paragraph by paragraph, but the 'help=' text for an option doesn't seem to be handled that way.
A possible workaround is to replace things like
help=_("""\ Configuration file to use. If not given, the environment variable MAILMAN_CONFIG_FILE is consulted and used if set. If neither are given, a default configuration file is loaded."""))
with
help=_( 'Configuration file to use. If not given, the environment variable ' 'MAILMAN_CONFIG_FILE is consulted and used if set. If neither are given, ' 'a default configuration file is loaded.'))
This seems to work to produce
Usage: mailman [OPTIONS] COMMAND [ARGS]...
The GNU Mailman mailing list management system Copyright 1998-2017 by the Free Software Foundation, Inc. http://www.list.org
Options: -C, --config PATH Configuration file to use. If not given, the environment variable MAILMAN_CONFIG_FILE is consulted and used if set. If neither are given, a default configuration file is loaded. --version Show the version and exit. --help Show this message and exit.
-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan
On 07/20/2017 03:11 PM, Mark Sapiro wrote:
One more thing. '-h' doesn't invoke help. See <http://click.pocoo.org/5/documentation/#help-parameter-customization> for how to enable this.
-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan
On Jul 20, 2017, at 05:22 PM, Mark Sapiro wrote:
On 07/20/2017 03:11 PM, Mark Sapiro wrote:
One more thing. '-h' doesn't invoke help. See <http://click.pocoo.org/5/documentation/#help-parameter-customization> for how to enable this.
Thanks, fixed! -Barry
On Jul 20, 2017, at 02:26 PM, Mark Sapiro wrote:
The first thing I notice right away is the help text doesn't fill. E.g.,
Now that you remind me, I noticed this too and I was going to file an upstream issue on this, but I forgot.
I did some pdb tracing through the click source and figured out the problem.
https://github.com/pallets/click/issues/834
TL;DR: Your workaround provided the essential clue. The main difference between the original code and your workaround is that the latter omits the embedded newlines. Click apparently dedents the text but doesn't remove the newlines and that confuses click's formatting code.
In the meantime, I have a workaround that I'm in the process of implementing.
Cheers, -Barry
On Jul 20, 2017, at 02:12 PM, Mark Sapiro wrote:
I'll look at it. I'll also rebase !301 and !202 after it lands, and I'll now delete my <https://gitlab.com/msapiro/mailman/tree/withlist> branch as this supercedes that.
Thanks Mark! -Barry
On 07/20/2017 10:56 PM, Barry Warsaw wrote:
Just a quick note to mention that my big branch to adopt click for command line option parsing should now be done.
https://gitlab.com/mailman/mailman/merge_requests/292
This will close #319 and #346 and make adding new
mailmansubcommands much easier. (We still need the bits to define additional search paths, and probably some better documentation that would be part of a general “Extending Mailman” section.)Along the way I think I’ve made several other improvements, including (I hope!) reducing or eliminating the occasional hangs we see on CI, speeding up the test suite a bit, and making things more robust.
Please feel free to review it and play with it. It’s finishing CI now but I’ll hold off on merging it for a day or two. I’m especially interested to hear what Jan thinks for the plugin work he’s doing.
The big downside could be that because this is such a big change, existing MPs may have to be rebased.
It’s a big branch with lots of little sweater threads that took longer than I expected, but I should be done now, and I think it will be a good improvement to the code.
Cheers, -Barry
Nice stuff! Looking at the dynamic loading of subcommands, plugin subcommands can be easily integrated. Pretty much a one line change on top of this and !288:
- add_components('mailman.commands', ICLISubCommand, self._commands)
- add_pluggable_components('commands', ICLISubCommand, self._commands)
Will rebase the plugin branch on top of this once it lands. Cheers,
Jan
/\ # PGP: 362056ADA8F2F4E421565EF87F4A448FE68F329D /__\ # https://neuromancer.sk /\ /\ # Eastern Seaboard Phishing Authority /__\/__\ #
On Jul 20, 2017, at 11:33 PM, Jan Jancar wrote:
Looking at the dynamic loading of subcommands, plugin subcommands can be easily integrated. Pretty much a one line change on top of this and !288:
- add_components('mailman.commands', ICLISubCommand, self._commands)
- add_pluggable_components('commands', ICLISubCommand, self._commands)
Will rebase the plugin branch on top of this once it lands.
Very cool. -Barry
Another thing I noticed is the help for the withlist --run option says in part:
If additional arguments are given at the end of the command line, they are passed as subsequent positional arguments to the callable. For additional help, see --details.
The additional arguments are not actually passed as subsequent positional arguments to the callable. They are passed as a single positional argument which is a tuple of the additional arguments.
--details is correct in its example showing
def change(mlist, args): mlist.display_name = args[0]
but the --run description makes me think it should be
def change(mlist, name): mlist.display_name = name
-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan
On Jul 21, 2017, at 11:46, Mark Sapiro <mark@msapiro.net> wrote:
Another thing I noticed is the help for the withlist --run option says in part:
If additional arguments are given at the end of the command line, they are passed as subsequent positional arguments to the callable. For additional help, see --details.
The additional arguments are not actually passed as subsequent positional arguments to the callable. They are passed as a single positional argument which is a tuple of the additional arguments.
--details is correct in its example showing
def change(mlist, args): mlist.display_name = args[0]
but the --run description makes me think it should be
def change(mlist, name): mlist.display_name = name
Thanks Mark. I’d like to preserve the API of Mailman 2.1, so I’m changing that back to passing them in as positional arguments (i.e. to match the —run description).
Just testing that change locally now.
Cheers, -Barry
On 07/21/2017 08:03 PM, Barry Warsaw wrote:
On Jul 20, 2017, at 03:56 PM, Barry Warsaw wrote:
This will close #319 and #346 and make adding new
mailmansubcommands much easier.This is now merged!
And MRs !202 and !301 have been rebased and CI is running.
-- Mark Sapiro <mark@msapiro.net> The highway is for gamblers, San Francisco Bay Area, California better use your sense - B. Dylan
participants (4)
-
Barry Warsaw -
Barry Warsaw -
Jan Jancar -
Mark Sapiro