Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a 'print' action for argparse #53645

Closed
travistouchdown mannequin opened this issue Jul 28, 2010 · 15 comments
Closed

Provide a 'print' action for argparse #53645

travistouchdown mannequin opened this issue Jul 28, 2010 · 15 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@travistouchdown
Copy link
Mannequin

travistouchdown mannequin commented Jul 28, 2010

BPO 9399
Nosy @birkenfeld, @rhettinger, @merwok, @berkerpeksag
Files
  • argparse.diff: patch adding a 'write' action to argparse
  • argparse.diff
  • try_write.py
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/rhettinger'
    closed_at = <Date 2019-11-12.05:44:31.327>
    created_at = <Date 2010-07-28.15:10:39.307>
    labels = ['type-feature', 'library']
    title = "Provide a 'print' action for argparse"
    updated_at = <Date 2019-11-12.05:44:31.321>
    user = 'https://bugs.python.org/travistouchdown'

    bugs.python.org fields:

    activity = <Date 2019-11-12.05:44:31.321>
    actor = 'rhettinger'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2019-11-12.05:44:31.327>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2010-07-28.15:10:39.307>
    creator = 'travistouchdown'
    dependencies = []
    files = ['18268', '20140', '35997']
    hgrepos = []
    issue_num = 9399
    keywords = ['patch']
    message_count = 15.0
    messages = ['111824', '111924', '111936', '111949', '112042', '112076', '112310', '122031', '124495', '124509', '127855', '128267', '223198', '223483', '356412']
    nosy_count = 8.0
    nosy_names = ['georg.brandl', 'rhettinger', 'bethard', 'eric.araujo', 'denilsonsa', 'travistouchdown', 'berker.peksag', 'paul.j3']
    pr_nums = []
    priority = 'normal'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue9399'
    versions = ['Python 3.5']

    @travistouchdown
    Copy link
    Mannequin Author

    travistouchdown mannequin commented Jul 28, 2010

    Currently argparse has a 'version' action which can be triggered by user defined options which prints out a custom string.

    parser.add_argument("--version", action="version", version="test 1.2.3")

    Since the 'version' action can be added multiple times, it can be used to output different kinds of information, like the program's license.

    parser.add_argument("--license", action="version", version="This file is licensed under GPL.... [a huge amount of text]")

    The only drawback is that linebreaks are substituted with a normal space. So I propose a 'print' action (perhaps as a replacement for 'version'?) which respects whitespace characters.

    parser.add_argument("--version", action="print", message="test 1.2.3")
    parser.add_argument("--license", action="print", message="This file is licensed under GPL.... [a huge amount of text, now properly formatted!]")
    parser.add_argument("--insult-me", action="print", message="You sick *peep* , *peep* yourself in *peep*")

    Currently, the only solution is to create a custom action which is IMHO a bit overkill for just printing a simple string to stdout.

    @travistouchdown travistouchdown mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jul 28, 2010
    @birkenfeld
    Copy link
    Member

    Sounds like a good addition to me.

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Jul 29, 2010

    Should this print to stdout or stderr? I wonder if the API should allow either, and instead look like:

    parser.add_argument('--license', action='write', message='...', file=sys.stdout)

    Where sys.stdout would be the default for the file= argument. The action would then just literally call file.write(message), so the behavior would be pretty easy to explain.

    Of course, at that point it makes me wonder if maybe it wouldn't just be better to have an easy way to have some arbitrary function called without having to subclass Action, e.g.:

    parser.add_argument('--license', action='call', callable=lambda: sys.stdout.write(message))

    Basically this would be a shorthand for subclassing Action when you don't need any information about the command line.

    @travistouchdown
    Copy link
    Mannequin Author

    travistouchdown mannequin commented Jul 29, 2010

    parser.add_argument('--license', action='write', message='...', file=sys.stdout)
    The ability to redirect the output would be a nice addition.

    parser.add_argument('--license', action='call', callable=lambda: sys.stdout.write(message))
    optparse already has a 'callable' action, I had the impression it was left out of argparse on purpose, even tough I couldn't imagine why...

    Either way is fine for me.

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Jul 30, 2010

    The equivalent to optparse callback is basically to define __call__ in a subclass of Action. Pretty much the same amount of work because they both have complicated parameters.

    The "callable" I (not fully confidently) proposed would just be a shorthand for defining __call__ in a subclass of Action when you don't care about any of the other command line info.

    I guess, without further use cases for "callable" and given that you can subclass Action for other use cases, let's just do the action='write' version. Feel free to supply a patch, or I will when I get some time for argparse again.

    @travistouchdown
    Copy link
    Mannequin Author

    travistouchdown mannequin commented Jul 30, 2010

    Here is a patch that adds a 'write' action to argparse. Usage example:

    >>> parser = argparse.ArgumentParser()
    >>> parser.add_argument("--license", action="write", message="This file\nis licensed under \t GPL")
    >>> parser.parse_args(['--license'])
    This file
    is licensed under        GPL

    A linebreak will be added after the message. The Output can be redirected with the optional 'file=' argument (it defaults to sys.stdout) The parser will then exit.

    This is my first patch ever written, so don't be too harsh if it's utter garbage^^

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Aug 1, 2010

    The patch looks basically right. A few minor issues:

    • "message=None," should probably be "message,", that is, message should not be allowed to default to None - I can't see any use case for this action without a message. I believe this means the body of __call__ can be simplified to::
      self.file.write(self.message)
      self.file.write("\n")
      parser.exit()
    • The other thing the patch needs is to update the test suite to add tests to make sure this behavior works. Take a look at test_argparse.py for how to do that.

    • The last thing is that to have the greatest chance of having someone check this in, you'll want to make your patch against Python trunk as explained here:

    http://www.python.org/dev/faq/#how-do-i-get-a-checkout-of-the-repository-read-only-or-read-write
    http://www.python.org/dev/faq/#how-to-make-a-patch

    Thanks!

    @merwok
    Copy link
    Member

    merwok commented Nov 22, 2010

    sys.std* should not be used as default values in a function definition, because they may be rebound to other objects. The usual idiom is to have None as default value and check it at call time.

    The patch also needs tests and docs.

    (FTR, the example for callable in this report was wrong: First, the message argument was missing in the lambda, second, there was no need for a lambda in the first place :)

    @merwok
    Copy link
    Member

    merwok commented Dec 22, 2010

    Thinking again about that, what’s wrong with argparse replacing \n with spaces and doing its own line wrapping?

    @travistouchdown
    Copy link
    Mannequin Author

    travistouchdown mannequin commented Dec 22, 2010

    I totally forgot about this patch, sorry... Due to university and another python project I am working on I didn't find the time to look at the test suite. I would really appreciate it if someone else could do this ;-)

    Anyway, I added the changes proposed by Steven Bethard and Éric Araujo regarding the default argument values. (patch made against trunk)

    @Éric:
    Well, first there is currently only the 'version' action which does this and imho it just looks strange if you try to print something else than a version info with it.
    Second, there are cases where you want whitespaces to be preserved, like printing out the license of your program (as I mentioned in my first post) Just look at this BSD license:

    Copyright (c) 2010, Dennis Malcorps <dennis.malcorps@gmail.com> Redistribution
    and use in source and binary forms, with or without modification, are
    permitted provided that the following conditions are met: 1. Redistributions
    of source code must retain the above copyright notice, this list of conditions
    and the following disclaimer. 2. Redistributions in binary form must reproduce
    the above copyright notice, this list of conditions and the following
    disclaimer in the documentation and/or other materials provided with the
    distribution. 3. The name of the author may not be used to endorse or promote
    products derived from this software without specific prior written permission.
    THIS SOFTWARE IS PROVIDED BY THE AUTHOR "AS IS" AND ANY EXPRESS OR IMPLIED
    WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
    MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
    EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
    SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
    PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
    BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER
    IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
    ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
    POSSIBILITY OF SUCH DAMAGE.

    This is just plain ugly.

    @merwok
    Copy link
    Member

    merwok commented Feb 4, 2011

    Thanks for the new patch. It does not apply cleanly to the py3k branch, but that’s very easily corrected.

            self.file.write(self.message)
            self.file.write('\n')

    Could be replaced by
    print(self.message, file=self.file)

    print will use the right EOL for the platform and is just coooler.

    By the way, self.file should not be rebound in __call__:
    if self.file is None:
    self.file = _sys.stdout

    file = (self.file if self.file is not None else _sys.stdout)

    Second, there are cases where you want whitespaces to be preserved,
    like printing out the license of your program (as I mentioned in my
    first post) Just look at this BSD license: [...]
    This is just plain ugly.

    Agreed, but I did not suggest that the output be like that. In my previous message, I suggested that argparse could replace \n with spaces and do its own line wrapping, to adapt to terminal length while avoiding chunking ugliness. That’s Steven’s call anyway, I don’t have any strong feeling in favor of preserving whitespace or rewrapping. If argparse wraps lines to the terminal width in other cases (like with epilog text), I think it should do so here too.

    @bethard
    Copy link
    Mannequin

    bethard mannequin commented Feb 10, 2011

    Argparse's wrapping behavior is determined by the formatter_class:

    http://docs.python.org/library/argparse.html#formatter-class

    Is it reasonable to assume that if you're wrapping your own messages you're already specifying formatter_class=argparse.RawTextHelpFormatter? If so, then perhaps the message should be printed via something like:

        formatter = parser._get_formatter()
        formatter.add_text(self.message)
        file.write(formatter.format_help())

    This is what we do in _VersionAction, so I guess it's probably what we should do here.

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 16, 2014

    @paul what is your take on this, other opinions seem positive?

    @paulj3
    Copy link
    Mannequin

    paulj3 mannequin commented Jul 19, 2014

    As Steven notes, the patch lacks tests. It also lacks documentation.

    The 2 things that this class does different from 'version' are

    • write without passing the text through 'textwrap'
    • write to a user defined file

    There is a difference in opinion between Éric and Steven as to whether the class should use write directly or use the HelpFormatter.

    I don't think it needs further action at this time. There doesn't seem to be a lot of interest in it. Also there are a number of ways of accomplishing the task without adding an Action class.

    • the class is a simple adaptation of the 'version' class

    • a user defined class works just as well

    • 'version' with Raw formatter, and shell redirection also works

    • it is also easy to write the text to a file after parse_args.

    I've attached a file that illustrates a number of these alternatives. It includes a 'callable' class.
    -----------------

    The discussion got me thinking about a selective version of the RAW formatting, analogous to the HTML <pre> tag. With this the user could mark a given text (description, epilog, help, version etc) as pre-formatted, without having to change the formatter_class.

        parser.add_argument('--license', action='version',
            version=Pre(formatted_text))

    I probably should submit that in a new issue.

    @rhettinger rhettinger assigned rhettinger and unassigned bethard Aug 30, 2019
    @rhettinger
    Copy link
    Contributor

    Marking this as closed because there has been no activity or interest for over five years.

    If anyone wants to revive this and write a PR, feel free to reopen. The core idea is plausible, but this doesn't seem to be a recurring need.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants