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

PR: Improve auto indentation of line following indented block #3826

Merged
merged 14 commits into from
Jan 5, 2017

Conversation

pwoosam
Copy link
Contributor

@pwoosam pwoosam commented Dec 10, 2016

Fixes #3800


Added checkbox to editor's preferences to enable and disable "strict indent".

With strict indenting enabled, automatic indentation has the correct behavior expected in issue #3800.
Disabling strict indenting will have the same automatic indentation behavior as before the update.

@ccordoba12 ccordoba12 added this to the v3.1 milestone Dec 11, 2016
@ccordoba12
Copy link
Member

Very quick feedback: I don't agree with adding a configuration option for this. We just have to come with a more intelligent indentation algorithm.

By the way, this is the same indentation algorithm used in the Jupyter notebook (through Codemirror).

@ccordoba12
Copy link
Member

Let's follow what VSCode and Sublime Text do: if the previous lines is unindented, leave the next line unindented too.

That way we won't need to add an option to control this indentation behavior and will fix #3800 too :-)

@ccordoba12
Copy link
Member

Also, please merge your branch with the latest 3.x one to fix the errors in CircleCI

@pwoosam
Copy link
Contributor Author

pwoosam commented Dec 11, 2016

Yeah, I wasn't too sure whether or not to make this a configuration option or not. I wasn't sure if there was anyone out there that saw it as a feature. :p

And that sounds like a good plan, I'll get on it!

The algorithm has some other issues with multi-line statements too where it does not indent enough. I'll open an issue if there isn't already one.

@ccordoba12
Copy link
Member

Thanks @pwoosam!

If the previous line is unindented, the next one will be aswell.
@pwoosam pwoosam force-pushed the issue3800 branch 2 times, most recently from 042f68b to 0d3bfbe Compare December 11, 2016 03:02
@pwoosam pwoosam force-pushed the issue3800 branch 2 times, most recently from df678b1 to 755cb3d Compare December 11, 2016 05:39
@ccordoba12
Copy link
Member

Your work is breaking a test for indentation. Please review the failure in CircleCI to fix the problem :-)

@pwoosam pwoosam force-pushed the issue3800 branch 2 times, most recently from aeb780e to b902469 Compare December 14, 2016 10:01
@pwoosam
Copy link
Contributor Author

pwoosam commented Dec 14, 2016

@ccordoba12 It took me a while, but I finally got around to fixing the failure. However, I'm not completely confident that this code doesn't break any other auto indent features not covered in the tests.

When I was playing around with it, I also noticed that small mistakes could break other seemingly unrelated such as intelligent backspace.

@ccordoba12
Copy link
Member

It'd be great if you could add a test for your work in widgets/sourcecode/tests/test_autoindent.py. Then we'll be sure of not breaking this feature in the future :-)

Tests added for testing if fix_indent can keep lines unindented.
Before, indent size was hardcoded. Now it is read from preferences.
@pwoosam
Copy link
Contributor Author

pwoosam commented Dec 14, 2016

I actually did not completely fix tabs in 9c99ba4. It's having some issues indenting with tab. If the above line has no whitespace, it won't indent.

I'll try figuring out where I went wrong..

Moved lines that handle tabs down. Now tabs will properly indent lines
that were kept unindented and have lines of whitespace above them.
@pwoosam
Copy link
Contributor Author

pwoosam commented Dec 15, 2016

All I had to do was move a couple of lines down! Such a simple mistake!

I'm much more confident in this fix now :)

@ccordoba12
Copy link
Member

@pwoosam, there's one use case I'd like to see implemented here too:

def f(x):
    return x
<Enter here (in empty/unindented line)>
<Leave next line unindented too, instead of moving cursor at the return indentation level>

@pwoosam
Copy link
Contributor Author

pwoosam commented Dec 17, 2016

@ccordoba12 Would you also like to unindent if you press return after a return statement just like it currently does with continue, break and pass?

@ccordoba12
Copy link
Member

Would you also like to unindent if you press return after a return statement?

Yeah, that'd be great too! Thanks for thinking on it :-)

If enter/return is pressed after a return statement, unindent the next
line. But if the statement contains open brackets, parenthesis, and/or
braces leave a hanging indent.
@pwoosam
Copy link
Contributor Author

pwoosam commented Dec 18, 2016

So currently the way I have this implemented for return statements is that it will automatically unindent the next line just like with break, continue and pass.

def f(x):\n
    return x\n
<new line starts here>

It also does as you asked:

def f(x):\n
    return x\n
\n
<new line starts here>

And it will also allow this:

def f(x):\n
    if x > 0:\n
        return x\n
\n          (it automatically unindented once and the user unindented once more)
<new line starts here>

This would also be useful for break, continue and pass. Here is an example for each:

def f(x):\n
    for i in range(x):\n
        if i % 90:\n
            print(i)\n
        else:\n
            break\n
\n           (it automatically unindented once and the user unindented twice more)
<new line starts here>
def f(x):\n
    for i in range(x):\n
        if i % 90:\n
            print(i)\n
        else:\n
            continue\n
\n          (it automatically unindented once and the user unindented twice more)
<new line starts here>
def f(x):\n
    if x > 0:\n
        pass\n
\n          (it automatically unindented once and the user unindented once more)
<new line starts here>

Currently they do:

def f(x):\n
    for i in range(x):\n
        if i % 90:\n
            print(i)\n
        else:\n
            break\n
\n
        <new line starts here>
def f(x):\n
    for i in range(x):\n
        if i % 90:\n
            print(i)\n
        else:\n
            continue\n
\n
        <new line starts here>
def f(x):\n
    if x > 0:\n
        pass\n
\n
    <new line starts here>

@pwoosam
Copy link
Contributor Author

pwoosam commented Dec 21, 2016

@ccordoba12 Let me know if you would like to see the above implemented for break, continue and pass as well. Otherwise, other than the print statement that I forgot to remove at line 1980, this is ready.

@ccordoba12
Copy link
Member

@pwoosam, I have one final suggestion (and sorry if my previous example with return was not clear enough).

I just want to see that a return in any line that is unindented and blank, leaves the next line unindented too. That's how VSCode works and I find it a good approach :-)

@pwoosam
Copy link
Contributor Author

pwoosam commented Dec 24, 2016

@ccordoba12 Sorry, I'm not too familiar with VSCode and I was confused by the return statement. :p

I believe I've gotten the behavior you wanted now though!

It should keep the same indentation as the line you pressed enter on as long as that line was unindented(relative to the indentation of prevline) and blank.

This works great! It even does everything I wanted above! Simple and awesome :)

@ccordoba12
Copy link
Member

@pwoosam, thanks a lot for the extra effort, it's working great now!

Hope to see more contributions from you in the future, you're doing great so far! ;-)

@ccordoba12 ccordoba12 changed the title PR: Auto indentation of line following indented block PR: Improve auto indentation of line following indented block Jan 5, 2017
@ccordoba12 ccordoba12 merged commit 530f50e into spyder-ide:3.x Jan 5, 2017
ccordoba12 added a commit that referenced this pull request Jan 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants