classification
Title: Append space after completed keywords
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: haypo, martin.panter, pitrou, python-dev, r.david.murray, serhiy.storchaka, twouters
Priority: normal Keywords: patch

Created on 2015-09-21 21:25 by serhiy.storchaka, last changed 2015-09-27 10:47 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
rlcompleter_keyword_space.patch serhiy.storchaka, 2015-09-21 21:25 review
rlcompleter_keyword_space_2.patch serhiy.storchaka, 2015-09-22 17:21 review
rlcompleter_keyword_space_3.patch serhiy.storchaka, 2015-09-24 12:18 review
Messages (11)
msg251262 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-21 21:25
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 PEP8. Attached patch implements this.
msg251282 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-09-22 01:41
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.
msg251332 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-22 17:21
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.
msg251347 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-09-22 21:44
“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.
msg251511 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-09-24 11:46
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).
msg251513 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-09-24 11:55
Victor, maybe because “for” also stands for “format”?
msg251514 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-24 12:18
> “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).
msg251515 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2015-09-24 12:21
> 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.
msg251628 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-09-26 02:24
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”.
msg251698 - (view) Author: Roundup Robot (python-dev) Date: 2015-09-27 10:44
New changeset f64ec4aac935 by Serhiy Storchaka in branch 'default':
Issue #25209: rlcomplete now can add a space or a colon after completed keyword.
https://hg.python.org/cpython/rev/f64ec4aac935
msg251700 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-27 10:47
Thank you for your review Martin. Hope to see your patch for completing import statement.
History
Date User Action Args
2015-09-27 10:47:57serhiy.storchakasetstatus: open -> closed
messages: + msg251700

assignee: serhiy.storchaka
resolution: fixed
stage: patch review -> resolved
2015-09-27 10:44:55python-devsetnosy: + python-dev
messages: + msg251698
2015-09-26 02:24:00martin.pantersetmessages: + msg251628
2015-09-24 12:21:40hayposetmessages: + msg251515
2015-09-24 12:18:36serhiy.storchakasetfiles: + rlcompleter_keyword_space_3.patch

messages: + msg251514
2015-09-24 11:55:10martin.pantersetmessages: + msg251513
2015-09-24 11:46:27hayposetnosy: + haypo
messages: + msg251511
2015-09-22 21:44:08martin.pantersetmessages: + msg251347
2015-09-22 17:21:21serhiy.storchakasetfiles: + rlcompleter_keyword_space_2.patch

messages: + msg251332
2015-09-22 01:41:47martin.pantersetnosy: + martin.panter
messages: + msg251282
2015-09-21 21:25:14serhiy.storchakacreate