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

Append space after completed keywords #69396

Closed
serhiy-storchaka opened this issue Sep 21, 2015 · 11 comments
Closed

Append space after completed keywords #69396

serhiy-storchaka opened this issue Sep 21, 2015 · 11 comments
Assignees
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Sep 21, 2015

BPO 25209
Nosy @Yhg1s, @pitrou, @vstinner, @bitdancer, @vadmium, @serhiy-storchaka
Files
  • rlcompleter_keyword_space.patch
  • rlcompleter_keyword_space_2.patch
  • rlcompleter_keyword_space_3.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 = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2015-09-27.10:47:57.388>
    created_at = <Date 2015-09-21.21:25:13.997>
    labels = ['extension-modules', 'type-feature']
    title = 'Append space after completed keywords'
    updated_at = <Date 2015-09-27.10:47:57.386>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2015-09-27.10:47:57.386>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2015-09-27.10:47:57.388>
    closer = 'serhiy.storchaka'
    components = ['Extension Modules']
    creation = <Date 2015-09-21.21:25:13.997>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['40538', '40549', '40561']
    hgrepos = []
    issue_num = 25209
    keywords = ['patch']
    message_count = 11.0
    messages = ['251262', '251282', '251332', '251347', '251511', '251513', '251514', '251515', '251628', '251698', '251700']
    nosy_count = 7.0
    nosy_names = ['twouters', 'pitrou', 'vstinner', 'r.david.murray', 'python-dev', 'martin.panter', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue25209'
    versions = ['Python 3.6']

    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented Sep 21, 2015

    Open parenthesis ('(') is appended after completed callable globals and attributes. I propose to add a space after completed keyword. In most cases a space is either required after the keyword, or recommended by PEP-8. Attached patch implements this.

    @serhiy-storchaka serhiy-storchaka added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Sep 21, 2015
    @vadmium
    Copy link
    Member

    vadmium commented Sep 22, 2015

    I agree with adding a space in some cases, but not others. E.g. "pass" would only need a space if you wanted to add a comment, "return" often takes no argument, "lambda" may need an immediate colon (lambda: x). See <https://github.com/vadmium/etc/blob/0f8d459/python/pythonstartup.py#L210\> for a whitelist of keywords that I thought should always have spaces, but beware this list may not be up to date (no “await” nor “async” for instance).

    BTW I don’t agree with the default of forcing an open bracket “(” for callables; consider decorators, subclassing, deferred function calls, etc, where that bracket may not be wanted.

    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented Sep 22, 2015

    Good catch Martin. Indeed, "pass", "break" and "continue" don't need a space, and "True", "False" and "None" need a space only in rare cases like "None in collection". Some keywords ("else", "finally", "try") need a colon. But while "return" and "lambda" not always need a space, I think that needing a space is more common case, and in any case the redundant space doesn't make a harm.

    Here is revised patch that doesn't add a space in some cases and instead adds a colon in some cases.

    @vadmium
    Copy link
    Member

    vadmium commented Sep 22, 2015

    “Else” doesn’t always use a colon. Consider “1 if x else 2”. Again, if Python started adding unwanted spaces and colons I imagine users could be annoyed and think Python is being too smart and complicated for its own good. But maybe see what others say.

    @vstinner
    Copy link
    Member

    vstinner commented Sep 24, 2015

    I tested the patch. "wi<tab>" displays "with " (space)., but "fo<tab>" displays "for" (no space). I don't understand why. Well, it's not a big issue, anyway the patch looks good to me. IMHO you should apply it to Python 3.4-3.6 (not only 3.6).

    @vadmium
    Copy link
    Member

    vadmium commented Sep 24, 2015

    Victor, maybe because “for” also stands for “format”?

    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented Sep 24, 2015

    “Else” doesn’t always use a colon. Consider “1 if x else 2”.

    Good catch Martin. Unfortunately the completer doesn't take context into account. "else" with a colon is a common case, but unwanted colon could be annoyed. I agree that it would be safe to not append a colon after "else". At least unless we make the completer context aware. However I don't think that unwanted space would be so annoyed. Here is updated patch.

    I see that your code does an autocompletion for module names in "import", Martin. This was my next idea. Do you want to provide a patch for including this in CPython?

    I tested the patch. "wi<tab>" displays "with " (space)., but "fo<tab>" displays "for" (no space). I don't understand why.

    Because there is "format(".

    IMHO you should apply it to Python 3.4-3.6 (not only 3.6).

    This is definitely a new feature. And there is a risk to break something (if standard completer is used programmatic).

    @vstinner
    Copy link
    Member

    vstinner commented Sep 24, 2015

    This is definitely a new feature. And there is a risk to break something (if standard completer is used programmatic).

    Ok, it's up to you.

    @vadmium
    Copy link
    Member

    vadmium commented Sep 26, 2015

    Yes I am interesting in making a patch for auto-completing module names. I’ll leave it on my to-do list, but don’t expect it overnight. :) My code does some rough parsing of Python syntax, but maybe that is not needed for “import x”, though it might be needed to support module attributes for “from x import y”.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 27, 2015

    New changeset f64ec4aac935 by Serhiy Storchaka in branch 'default':
    Issue bpo-25209: rlcomplete now can add a space or a colon after completed keyword.
    https://hg.python.org/cpython/rev/f64ec4aac935

    @serhiy-storchaka
    Copy link
    Member Author

    serhiy-storchaka commented Sep 27, 2015

    Thank you for your review Martin. Hope to see your patch for completing import statement.

    @serhiy-storchaka serhiy-storchaka self-assigned this Sep 27, 2015
    @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
    extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants