-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Editor: Move to next line in run_selection() if nothing selected #3146
Conversation
It seems to me from the log that the AppVeyor failure occured while the test environment is set up so it is not caused by anything in the pull request. |
@jitseniesen Awesome :-), thanks for working on this. Indeed, the appveyor tests are broken for some weeks now, @ccordoba12 will fix them soon :-) |
@@ -1773,12 +1773,19 @@ def fix_indentation(self, index=None): | |||
|
|||
#------ Run | |||
def run_selection(self): | |||
"""Run selected text or current line in console""" | |||
"""Run selected text or current line in console | |||
If no text is selected, then advance cursor to next line.""" |
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.
Minor style comment
Can we turn this into
"""
Run selected text or current line in console. If no text is selected, then
advance cursor to next line.
"""
LGTM 👍 Any comments @spyder-ide/core-developers |
Yep, looks sensible to me. |
_looks sensible to me_ WTF? |
As in I agree with the approach, but did not actually test it ;). |
Of course I'm happy to follow the house style (regarding formatting of docstrings), so I changed it. I tried to infer how you wanted it from other docstrings, but obviously failed. |
@jitseniesen thanks for the changes... we do not have an specific style written somewhere, but what I am trying to promote is something in the lines of.
# As in
def test()
"""
Hello world.
"""
# Instead of
def test()
"""Hello world.""" This makes the rule easy.... ALWAYS use that instead of one for one liners or another for whatever reason. What do you think? |
I think it makes sense for a more-than-one-(wo)man project to have style rules and it does not make much sense to argue long about them, so I'm happy to follow those rules. |
Thanks for working on this @jitseniesen! Great addition :-) |
What is preventing merging this PR ? Everything looks fine ? |
If you feel it's ok, please merge it :-) El 30/05/16 a las 09:23, Joseph Martinot-Lagarde escribió:
|
Fixes #3115
@jitseniesen I rewrote the header to the standard we try to use (
Fixes #<issue number>
)