-
Notifications
You must be signed in to change notification settings - Fork 178
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
Allow whitespace before ${cmd_name}. #329
Conversation
On 2017-09-02 04:31:53 -0700, Andrew Shadura wrote:
-- Commit Summary --
* Allow whitespace before ${cmd_name}.
Why do you need this? I just checked the existing osc commands
and they stick to this convention... so I assume you need this
for a plugin? If so, would "fixing" the plugin be an option?
|
I thought the explanation in the pull request would be enough… I don't think it's wrong to put |
Right, I suspect you may have not seen it, as I have added the explanation after I created the pull request, and you seem to be using GitHub's email interface. Let me repost it for you:
|
PEP 257 says:
|
On 2017-09-02 18:27:47 +0000, Andrew Shadura wrote:
Right, I suspect you may have not seen it, as I have added the explanation *after* I created the pull request, and you seem to be using GitHub's email interface. Let me repost it for you:
Yep, I haven't seen your explanation. Thanks!
> It's not immediately obvious to the plug-in writers `${cmd_name}` must be on the very first line of the docstring, and docstrings often begin and end with `"""` on its own line:
If you really need this feature, we should make the patch "consistent",
because with the current version the leading whitespaces are still
present when running "osc cmd --help".
|
Right, I'll have a look and resubmit.
|
if doc and doc.lstrip().startswith(to_strip): | ||
#log.debug("stripping %r from start of %s's help string", | ||
# to_strip, cmdname) | ||
doc = doc[len(to_strip):].lstrip() | ||
doc = (doc.lstrip())[len(to_strip):].lstrip() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't be better to do doc = doc.lstrip()
before if doc ...
?
It is more efficient (running lstrip()
only once) and also covers a situation (unlikely one) that the docstring contains only whitespaces.
This change makes sense to me and it sits in the backlog for too long. |
It's not immediately obvious to the plug-in writers
${cmd_name}
must be on the very first line of the docstring, and docstrings often begin and end with"""
on its own line: