Cleaned source of `cmd` module

[zearin]

BPO	15137
Nosy	@terryjreedy, @amauryfa, @merwok, @bitdancer
Files	cmd.py patch-cmd.txt: Patch between my edits and ace45d23628a

^{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 2012-06-24.19:28:35.505>
created_at = <Date 2012-06-22.14:02:27.327>
labels = ['invalid', 'type-feature', 'library']
title = 'Cleaned source of `cmd` module'
updated_at = <Date 2012-06-24.19:36:30.093>
user = 'https://bugs.python.org/zearin'

bugs.python.org fields:

activity = <Date 2012-06-24.19:36:30.093>
actor = 'eric.araujo'
assignee = 'none'
closed = True
closed_date = <Date 2012-06-24.19:28:35.505>
closer = 'eric.araujo'
components = ['Library (Lib)']
creation = <Date 2012-06-22.14:02:27.327>
creator = 'zearin'
dependencies = []
files = ['26082', '26083']
hgrepos = []
issue_num = 15137
keywords = []
message_count = 7.0
messages = ['163409', '163410', '163412', '163413', '163414', '163551', '163825']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'amaury.forgeotdarc', 'eric.araujo', 'r.david.murray', 'zearin']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue15137'
versions = ['Python 3.4']

[zearin]

Cleaned up the source of the Standard Library’s cmd module.

Attempted to focus on readability by changing things like using booleans instead of 0/1, newer syntax for string formatting, lining up variable declarations (judgement call), and adding comments between groups of methods.

Used ace45d23628a to start my work.

[amauryfa]

The output of "hg diff" would be much appreciated, it's the only way for reviewers to realize the changes you made.

[zearin]

Does hg diff produce different output than regular diff?

If so, the patch created by regular diff is attached.

(I basically did my work without using a local repo, and I have very little experience with patching. I basically just looked up how to create a patch with vanilla diff.)

[bitdancer]

I appreciate the work and thought that went into this, but this is not normally the kind of change that we apply. Any change has a chance of introducing bugs (see, for example, bpo-15109, where a seemingly innocuous change from % formatting to .format formatting caused a regression in 2.7.3 compared to 2.7.2).

The other option, applying it just to default, is also not normally done, because then the code base between versions is gratuitously different, making merging more difficult.

So, our general policy is that we do cleanups only when we are changing that area of code in order to fix a bug or add an enhancement. And as you can see from bpo-15109, even then we sometimes get in trouble.

Now, if you are interested in the maintenance of the cmd module, and can suggest one or more fix/enhancement + cleanup patches like Lucasz Lagna did for configparser, that would be wonderful.

[bitdancer]

Also as an FYI, triple double quote marks are our standard for docstrings. I don't believe we normally align assignments, but I don't know if that is specifically called out in PEP-8. I haven't looked at the patch in detail, but in scanning it quickly, while some of your whitespace changes (spaces around operators) agree with our standards, others (extra spaces after a comma in a function call, for example) do not.

Again, I appreciate your putting in this effort, and regret that it isn't something we can use.

[terryjreedy]

Do read PEP-8 Python style guide. http://python.org/dev/peps/pep-0008/

You violated the following:
(Peeves)
More than one space around an assignment (or other) operator to align it with another.

Yes:

x = 1
y = 2
long_variable = 3

No:

x             = 1
y             = 2
long_variable = 3

I used to do that, but it only works with fixed-pitch fonts, which is not really possible for full-unicode fonts. Anyway, that is about half the changes, and they would have to go. Sorry. Some of your other changes make it more compliant. Some I am not sure of others without re-reading.

For the other reasons David gave, I am closing this so you are not mislead into doing more work that will not be accepted. I would note that improving test coverage *is* accepted and good test-coverage is really needed before extensive re-writes. Another document to read is
the developer guide http://docs.python.org/devguide/index.html

Last point. Please use .diff or .patch for diff/patch files as that extension works better for people and, I believe, hg.

Since you are interested in readability, you might consider contributing doc suggestions. You do not have to know .rst formatting. A good suggestion given as plain ascii in a message like this will be copied and formatted by someone who does know .rst. And in simple cases, one can even patch the source .rst withouth knowing much.

[merwok]

Just wanted to offer a comment on this:

Last point. Please use .diff or .patch for diff/patch files as that
extension works better for people
Depending on OS and file association settings, yes.

and, I believe, hg.
Mercurial does not care about file extensions, it works with standard input or any file path or URI.