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.

Sign up for GitHub

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

Jump to bottom

mention of list2cmdline() in docs of subprocess.Popen #56036

Closed
elibendersky mannequin opened this issue Apr 11, 2011 · 13 comments
Closed

mention of list2cmdline() in docs of subprocess.Popen #56036

elibendersky mannequin opened this issue Apr 11, 2011 · 13 comments
Labels
docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@elibendersky
Copy link
Mannequin

elibendersky mannequin commented Apr 11, 2011

BPO 11827
Nosy @ezio-melotti, @merwok, @bitdancer
Files
  • issue11827.1.patch
    		•

    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 = None
closed_at = <Date 2011-04-30.05:56:06.677>
created_at = <Date 2011-04-11.08:38:15.897>
labels = ['type-bug', 'docs']
title = 'mention of list2cmdline() in docs of subprocess.Popen'
updated_at = <Date 2011-07-29.13:54:53.230>
user = 'https://bugs.python.org/elibendersky'

    bugs.python.org fields:

    activity = <Date 2011-07-29.13:54:53.230>
actor = 'eric.araujo'
assignee = 'docs@python'
closed = True
closed_date = <Date 2011-04-30.05:56:06.677>
closer = 'eli.bendersky'
components = ['Documentation']
creation = <Date 2011-04-11.08:38:15.897>
creator = 'eli.bendersky'
dependencies = []
files = ['21634']
hgrepos = []
issue_num = 11827
keywords = ['patch']
message_count = 13.0
messages = ['133507', '133510', '133511', '133514', '133563', '133566', '133568', '133599', '133673', '133780', '133782', '133783', '141382']
nosy_count = 7.0
nosy_names = ['ezio.melotti', 'eric.araujo', 'r.david.murray', 'eli.bendersky', 'docs@python', 'rosslagerwall', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue11827'
versions = ['Python 3.1', 'Python 2.7', 'Python 3.2', 'Python 3.3']
    @elibendersky
    Copy link
    Mannequin Author

    elibendersky mannequin commented Apr 11, 2011

    The documentation of subprocess.Popen mentions a function named list2cmdline():

    ----
    On Windows: the Popen class uses CreateProcess() to execute the child program, which operates on strings. If args is a sequence, it will be converted to a string using the list2cmdline() method. Please note that not all MS Windows applications interpret the command line the same way: list2cmdline() is designed for applications using the same rules as the MS C runtime.
    ----

    I find this rather opaque and useless. list2cmdline() in subprocess is publicly accessible (doesn't begin with underscores) but it isn't documented.

    The solution can be one of the following:

    1. Document list2cmdline in the docs of subprocess, making the reference to is useful
    2. Don't mention list2cmdline and instead mention what it does with the command-line list

    @elibendersky elibendersky mannequin assigned docspython Apr 11, 2011
    @elibendersky elibendersky mannequin added docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error labels Apr 11, 2011
    @bitdancer
    Copy link
    Member

    I vote for (2) (I presume 'it' in that sentence is 'subprocess'). list2cmdline shouldn't be a "real" public method, at least not without the issues surrounding being given careful design attention.

    @elibendersky
    Copy link
    Mannequin Author

    elibendersky mannequin commented Apr 11, 2011

    I also prefer (2) since I see no reason for the user to use list2cmdline() directly, let alone from subprocess (had there been rationale for such a public function it should probably be in another module).

    As for 'it', I guess you can say it means 'subprocess' or 'list2cmdline', doesn't matter which.

    @bitdancer
    Copy link
    Member

    Ah, right. I guess I was advocating that the docs be written from the perspective that list2cmdline doesn't exist as an identifiable entity. From the POV of the updated docs, it is just subprocess's behavior, and list2cmdline is an implementation detail. Which is what you were saying.

    @rosslagerwall
    Copy link
    Mannequin

    rosslagerwall mannequin commented Apr 12, 2011

    bpo-10838 has a bit of discussion about list2cmdline and being part of the public api (generally agreeing that it should be made private, I think).

    @elibendersky
    Copy link
    Mannequin Author

    elibendersky mannequin commented Apr 12, 2011

    Thanks for the pointer, Ross.

    So I propose to remove the mention of list2cmdline from the documentation of subprocess, explaining instead what it does to the path (since I think this should be publicly known, otherwise it's just black magic).

    I will prepare a patch for this and post it for review here before committing.

    Do you folks think we should also prepend an underscore to list2cmdline, thus settling the doubt about its "public-ness" once and for all? Other private functions in subprocess do have underscores prepended.

    @ezio-melotti
    Copy link
    Member

    If a _ is added to list2cmdline the old name should be kept and deprecated.
    The function is also on 2.x and it's not deprecated there (and it's probably too late to deprecate it now), so we might have to wait a few versions before it will be possible to remove list2cmdline from 3.x.

    @elibendersky
    Copy link
    Mannequin Author

    elibendersky mannequin commented Apr 12, 2011

    Attaching a proposed patch for 3.2, focusing only on the documentation for the time being (I realize that deprecation is a loaded issue and should be probably handled in a centralized manner).

    The patch removes mention of list2cmdline, instead explaining its intention (using the docstring of list2cmdline quite literally). I realize this isn't an ideal approach, but IMHO it's much better than what we have now. Suggestions are most welcome.

    @merwok
    Copy link
    Member

    merwok commented Apr 13, 2011

    LGTM.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 15, 2011

    New changeset e7a55e236b8b by Eli Bendersky in branch '3.1':
    Issue bpo-11827: remove mention of list2cmdline in the doc of subprocess
    http://hg.python.org/cpython/rev/e7a55e236b8b

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 15, 2011

    New changeset f38489a3394f by Eli Bendersky in branch '2.7':
    Issue bpo-11827: remove mention of list2cmdline in the doc of subprocess
    http://hg.python.org/cpython/rev/f38489a3394f

    @elibendersky
    Copy link
    Mannequin Author

    elibendersky mannequin commented Apr 15, 2011

    Patch committed to 3.3, 3.2, 3.1, 2.7

    In case no objections arise, I will close this issue in a couple of days

    @elibendersky elibendersky mannequin closed this as completed Apr 30, 2011
    @merwok
    Copy link
    Member

    merwok commented Jul 29, 2011

    The module docstring (which duplicates the reST docs) still mentions list2cmdline. I’d vote for removal there too.

    @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
    Assignees
    No one assigned
    Labels
    docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @bitdancer @merwok @ezio-melotti