
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:
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:26 PM, Mark Sapiro wrote:
...
I've looked at the click docs and I see that text from docstrings is wrapped and filled as is the text
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
with
This seems to work to produce
-- 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 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 07/20/2017 10:56 PM, Barry Warsaw wrote:
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 /__\/__\ #

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:
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/20/2017 01:56 PM, Barry Warsaw wrote:
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:26 PM, Mark Sapiro wrote:
...
I've looked at the click docs and I see that text from docstrings is wrapped and filled as is the text
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
with
This seems to work to produce
-- 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 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 07/20/2017 10:56 PM, Barry Warsaw wrote:
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 /__\/__\ #

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:
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
participants (4)
-
Barry Warsaw
-
Barry Warsaw
-
Jan Jancar
-
Mark Sapiro