PR: Comment lines taking into account code indentation and pep8

[dalthviz]

Fixes #2845
Fixes #1785

Edit:
Takes into account some ideas from @bcolsen and @Ericvulpi, thanks for your help in this one :)

[dalthviz]

A preview:

[goanpeca]

Lets add a space between the # and the code so it is valid pep8 and ensure that we check if there is a space or not also when uncommenting.

So that

# def hello():
    # pass

#def hello():
    #pass

Both work when removing the comment

[ccordoba12]

@bcolsen, what do you think about this one?

@rlaverde, would this help code folding to work correctly when big chunks are commented?

[rlaverde]

would this help code folding to work correctly when big chunks are commented?

Hmm, this was the original solution, but we can't expect that lines will be always comment out with spyder shortcut, because of that I make code folding ignore commented lines in #4728, they could have an arbitrary indentation and that will break codefolding

Although I really like this change, IMO commented out lines at the first of the line is visually annoying. also other editors comment out lines according identation

[bcolsen]

I really like this style because it keeps the flow of the code even with comments in the middle.

The code is good at undoing the comments even crazy stuff like this:

# if __name__=="__main__":
#     app = QApplication(sys.argv)
    #btn = Test()
#     btn.show()
#     app.exec_()

However, it is not backwards compatible with the previous block comments, and can break code that has been manually commented out at the beginning of the line. These can be hard to spot and make whitespace management a difficult chore.

But I think this problem can be fixed.

The code fails on this:

#if __name__=="__main__":
#    app = QApplication(sys.argv)
#    btn = Test()
#    btn.show()
#    app.exec_()

It deletes the spaces in front of the indented section leaving 3 spaces instead of four.

Should be possible to tell whether a space needs to be deleted or not by:

1. Count the number of spaces
2. Mod the count by number of spaces set for indents in the file
3. If mod equals 1 remove the extra space
4. Else don't remove any spaces

This wouldn't work perfectly for long line continuations, but those won't break your programs like other indents do.

[ccordoba12]

@dalthviz, please try to fix the use case shown by @bcolsen.

[pep8speaks]

Hello @dalthviz! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on October 05, 2017 at 21:23 Hours UTC

[ccordoba12]

@dalthviz, great work so far. Please squash all commits referring to Testing and also make sure that this PR also fixes issue #1785.

[ccordoba12]

You can take a look at PR #4720 to see how to do it.

[dalthviz]

@ccordoba12 @bcolsen this is a preview:

@bcolsen could you test it please? thanks :)

[bcolsen]

Wow that's great it's much more robust pycharm and I like the look of it. I gave it quite a bit of abuse and it held up nicely. I like the unblocking of block comments that's slick.

Now onto the bugs ;)

Issue 1

If I highlight the beginning of a line and comment out the section I start loosing selection off the end of the selection. It looks like the number of characters decreased by twice the number of lines that are highlighted in the selection.

Highlight:

Comment:

Uncomment and Comment again:

Issue 2

You may have noticed in the screen shots above that the comments are not in line with the text this seems to happen when a line is blank. If you have a space in the line it skips it but not if its blank.

Issue 3

if you uncomment a block comment with at commented line light highlighted it completely uncomments the line instead of only removing the block comments.

Issue 4

The new unblocking of the block comments seems incompatible with the old blocks. I'm not sure how the blocks are detected so I don't know if it's possible. It would be nice, but not the end of the world either way.

[dalthviz]

@bcolsen thanks again for the good feedback 👍, could you test the latest changes? I think this is almost finished :)

[bcolsen]

Issue 1 and 4 are solved now. 👍

I'm back on my laptop so I made some gifs of the other two.

Issue 2

The blank lines getting skipped now but they are still counting as smallest indent so the comments are still on the far left.

Issue 3

The comment in block comments are still getting uncommented when I use ctrl-1. If you press undo after you remove the block comments the original comment is restored and undo again to bring back the block comment.

[rlaverde]

Maybe to resolve these issues we could consider two shortcuts for comment/uncomment code, instead of only one. (i.e Ctrl+1 will always comment code, and Ctrl+Shift+1 will uncomment code )

[bcolsen]

I was actually thinking the same thing

Crtl-shift-1 is a little awkward but it makes sense

[dalthviz]

@bcolsen I made some more changes can you test again? :). About the reasons of the issues still there: in the case of the unblockcomment I was not returning in the method and in the case of the indentation level of the comment it was a missing validation.

About the suggestion of @rlaverde (by the way, thanks for the feedback 👍), thinking beyond the issues that @bcolsen helped to find, you guys think that could be helpful to have a different shortcut to manage the comment/uncomment? What do you think @ccordoba12?

[bcolsen]

@dalthviz Everything looks great to me here! I couldn't find anything that would I would normally do or end up with that can break. 👍

As for the ctrl-shift-1 for removing comments:

Keep the current commenting tool the same

I think ctrl-1 is very easy as it is now and should remain the same, removing comments if only all the selected lines are commented. The ctrl-1 code already comments mixed commented and uncommented code. I can't think of a time I would want to double or triple comment something that is already commented out.

Add an option for just uncommenting

I have had times when I wanted to uncomment a whole section that had mixed commented and uncommented code. A ctrl-shift-1 uncomment could be very handy in these situations.

For example, pressing ctrl-shift-1 twice would completely uncomment this code with only one selection. With the current commenting code this would require three selections and three key presses

# if language.lower() in value:
#     self.supported_language = True
#     # sh_class, comment_string, CFMatch = self.LANGUAGES[key]
#     # self.comment_string = comment_string
    if key in CELL_LANGUAGES:
        self.supported_cell_language = True
        self.cell_separators = CELL_LANGUAGES[key]
#     if CFMatch is None:
#         self.classfunc_match = None
#     else:
#         self.classfunc_match = CFMatch()

This way we can keep the commenting tool "smart"(or contextual) like it is now about when to comment or uncomment, as well as give the option to take an arbitrary block of code and uncomment all of it.

[dalthviz]

Thanks a lot @bcolsen for all your help with this 👍

About the new shortcut for only uncommenting, from that point of view could be interesting, however I think that it could be left for another issue @ccordoba12 @rlaverde what do you guys think?

[ccordoba12]

@bcolsen, thanks a lot for your careful and detailed review! It helped us a lot to improve this!

About adding an option for just uncommenting, it looks like a reasonable idea. Please open a new issue about it, so we can continue our discussion there.

[ccordoba12]

Please change this to

Handle the removal of prefix comment for a single line

[ccordoba12]

@dalthviz, I left a couple of minor comments, then I'll merge ;-)

[ccordoba12]

Great work @dalthviz!