PR: Add indentation guidelines #4572

rlaverde · 2017-06-07T19:22:07Z

I added a floating type panel, and move edgeline and indentguides to use it.

I didn't find a way to abstract all the logic in the PanelManager, and floatings Panels need to have a set_geometry method

This is how it looks like, It uses the comments color (same as edgeline)

goanpeca · 2017-06-07T19:22:57Z

Nice :-), like this a lot!

goanpeca · 2017-06-07T19:24:31Z

spyder/widgets/panels/indentationguides.py

+        self.color = Qt.darkGray
+        self.i_width = 4
+
+        self.setAttribute(Qt.WA_TransparentForMouseEvents)


Why duplicate here?

I forgot to delete it. when moving it to panel class

goanpeca · 2017-06-07T19:24:40Z

spyder/widgets/panels/indentationguides.py

+        self._enabled = True
+
+    def paintEvent(self, event):
+        """Override Qt method"""


Period on docstrings

goanpeca · 2017-06-07T19:24:47Z

spyder/widgets/panels/indentationguides.py

+    # -----------------------------------------------------------------
+
+    def set_enabled(self, state):
+        """Toggle edge line visibility"""


goanpeca · 2017-06-07T19:24:52Z

spyder/widgets/panels/indentationguides.py

+
+    def update_color(self):
+        """
+        Set edgeline color using syntax highlighter color for comments


goanpeca · 2017-06-07T19:32:47Z

spyder/widgets/panels/indentationguides.py

+# (see spyder/__init__.py for details)
+
+"""
+This module contains the indentation guide panel


ccordoba12 · 2017-06-07T19:36:23Z

Screenshot, please? :-)

…t is implemented in Panel class.

rlaverde · 2017-06-07T19:49:10Z

Screenshot, please? :-)

Added 😄

ccordoba12 · 2017-06-07T19:50:35Z

Neat, but the guidelines are discontinuous? Can we do something about it?

goanpeca · 2017-06-07T20:02:53Z

Neat, but the guidelines are discontinuous?

@ccordoba12, please post a screenshot of what you expect? on vscode, or sublime... or ?

goanpeca · 2017-06-07T20:08:49Z

@rlaverde screenshot?

rlaverde · 2017-06-07T20:09:35Z

Neat, but the guidelines are discontinuous? Can we do something about it?

It's drawn using only the current line, I added a little hack to avoid discontinuity (preserve previous indentation), I think this is the best possible without making it "smart" and adding a lot of complexity.

goanpeca · 2017-06-07T20:10:40Z

I think it looks ok, we will probably need information from the fold detector eventually to make it better? Cause I guess this is now python specific?

rlaverde · 2017-06-07T20:19:41Z

we will probably need information from the fold detector eventually to make it better?

I hadn't thought about that 🙈 It was way easy to use folding levels and less error prone. I looks the same as the last screenshot.

goanpeca · 2017-06-07T20:37:42Z

spyder/widgets/panels/indentationguides.py

+        """Calculate and set geometry of indentation guides panel."""
+        offset = self.editor.contentOffset()
+        x = self.editor.blockBoundingGeometry(self.editor.firstVisibleBlock()) \
+            .translated(offset.x(), offset.y()).left() + 5


What is this 5?

A hardcoded offset used in the edgeline panel (I don't have any idea why, but if I delete it and the panel is misplaced) 😕

hmmm ok, we need to dig deeper to know where this 5 comes from :-p

f7c68f3#diff-a032958c9839f940f8038a099a3bf970R1072

So Pïerre is the only one who knows why that 5 is here :-/

I think that is something like the margin of the editor (but I'm not sure)

Ok whatever it is, we need to put in a constant or use it within the methods that return the offsets, so that it is fixed in one place!

Right, but that's not work for this PR, I'd say ;-)

Yes, @rlaverde lets open a techdebt issue to track this problem so we can solve it later on

ccordoba12 · 2017-06-08T01:22:33Z

spyder/widgets/panels/linenumber.py

-        else:
-            self.update(0, qrect.y(),
-                        self.width(),
-                        qrect.height())


Why is this not needed anymore?

It's implemented in the PanelManager class, It's enough to set scrollable=True in a Panel to make it scroll with the editor

ccordoba12 · 2017-06-08T01:24:37Z

This looks terrific!! Great work @rlaverde!

A couple of comments:

Please add a menu entry in the Source menu to easily activate/deactivate these guidelines. This is similar to the Show blank spaces entry.
A test for this would be desirable (even if very simple).

goanpeca · 2017-06-08T03:14:38Z

A test for this would be desirable (even if very simple).

This is a UI exclusive thing, it would be kinda hard to test I think :-|

goanpeca · 2017-06-08T03:15:49Z

Please add a menu entry in the Source menu to easily activate/deactivate these guidelines. This is similar to the Show blank spaces entry.

@ccordoba12 I would say spaces and guidelines fall in the same thing, so maybe we can just merge these options together? I say that in the spirit of keeping the UI and options simple. Thoughts?

ccordoba12 · 2017-06-08T11:56:41Z

This is a UI exclusive thing, it would be kinda hard to test I think :-|

A test could consist in creating a Python file with indentation and test that guidelines are visible. That would be enough for me.

I would say spaces and guidelines fall in the same thing, so maybe we can just merge these options together? I say that in the spirit of keeping the UI and options simple. Thoughts?

I would like to use guidelines only, so I'd prefer to maintain the two options separate.

…d in Mode).

goanpeca · 2017-06-08T14:10:06Z

A test could consist of creating a Python file with indentation and test that guidelines are visible.

Guidelines are painted, not a widget, how do you check that?

rlaverde · 2017-06-08T14:34:24Z

I added a simple test that check that panels are added to the editor and are enabled, I don't know how to test the internal behavior of the Paint method.

goanpeca · 2017-06-08T14:36:03Z

I don't know how to test the internal behavior of the Paint method.

Neither do I.... aside from doing more complex image processing... with a screenshot/pixmap of the editor widget

ccordoba12 · 2017-06-09T14:21:10Z

I don't know how to test the internal behavior of the Paint method.

Fair enough. The test you added is sufficient.

So the only thing missing here is adding an entry to the Source menu. Then I'll merge :-)

…ren't updated at statup.

ccordoba12 · 2017-06-13T18:42:23Z

Is this ready?

rlaverde · 2017-06-13T19:27:57Z

Is this ready?

Yes, the refactoring related to the hardcoded offset will be addressed in other PR

ccordoba12 · 2017-06-13T19:44:59Z

Great, thanks!

rlaverde added 7 commits June 7, 2017 11:33

Add indentation guides panel.

bd805b6

Add indent_guides to editor

2e32ff1

Improve calculation of tab stop width.

10d6bff

Add indent_guides preference, default to False.

c379f45

Add logic for floating panels.

b0f2c45

Change edgeline and indentationguides to use Panel class.

8edda01

Add scrollable=True to linenumberarea, and remove update_ function.

263b62f

rlaverde added this to the v4.0beta1 milestone Jun 7, 2017

rlaverde self-assigned this Jun 7, 2017

rlaverde requested review from ccordoba12 and goanpeca June 7, 2017 19:22

goanpeca reviewed Jun 7, 2017

View reviewed changes

rlaverde added 2 commits June 7, 2017 14:44

Docstring improvements in indentationguide.

46396fe

Remove Qt.WA_TransparentForMouseEvents in floatings Panels, because i…

b3f81e5

…t is implemented in Panel class.

Add a like phack to indent guides to avoid discontinuity.

d33ef15

Use information of folding levels in indentation guides.

5f72b29

goanpeca reviewed Jun 7, 2017

View reviewed changes

ccordoba12 reviewed Jun 8, 2017

View reviewed changes

rlaverde added 3 commits June 8, 2017 08:15

Remove redefinition of _enabled in child Panels class (already define…

ad5ba03

…d in Mode).

Add simple test to editors panels.

e30ab6d

Add scrollable = True to scroollflag and remove signal conection.

3be3415

rlaverde added 3 commits June 9, 2017 10:33

Add action for pindent guides in source menu.

3df7f68

Fix error show blanks spaces only applied to curret editor.

e6a73b4

Fix error blank spaces, and indent guides checkboxs in Source menu we…

d0ea441

…ren't updated at statup.

rlaverde mentioned this pull request Jun 12, 2017

Improve floating panels in the editor #4590

Closed

ccordoba12 changed the title ~~PR: Indentation guides panel~~ PR: Add indentation guidelines Jun 13, 2017

ccordoba12 merged commit c6955dc into spyder-ide:master Jun 13, 2017

rlaverde deleted the indentation-guides-panel branch June 13, 2017 20:49

PR: Add indentation guidelines #4572

PR: Add indentation guidelines #4572

Conversation

rlaverde commented Jun 7, 2017 • edited

goanpeca commented Jun 7, 2017

Choose a reason for hiding this comment

rlaverde Jun 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccordoba12 commented Jun 7, 2017

rlaverde commented Jun 7, 2017

ccordoba12 commented Jun 7, 2017

goanpeca commented Jun 7, 2017

goanpeca commented Jun 7, 2017

rlaverde commented Jun 7, 2017

goanpeca commented Jun 7, 2017 • edited

rlaverde commented Jun 7, 2017

Choose a reason for hiding this comment

rlaverde Jun 7, 2017 • edited

Choose a reason for hiding this comment

goanpeca Jun 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccordoba12 commented Jun 8, 2017

goanpeca commented Jun 8, 2017

goanpeca commented Jun 8, 2017

ccordoba12 commented Jun 8, 2017 • edited

goanpeca commented Jun 8, 2017

rlaverde commented Jun 8, 2017

goanpeca commented Jun 8, 2017 • edited

ccordoba12 commented Jun 9, 2017 • edited

ccordoba12 commented Jun 13, 2017

rlaverde commented Jun 13, 2017

ccordoba12 commented Jun 13, 2017

rlaverde commented Jun 7, 2017 •

edited

rlaverde Jun 7, 2017 •

edited

goanpeca commented Jun 7, 2017 •

edited

rlaverde Jun 7, 2017 •

edited

goanpeca Jun 7, 2017 •

edited

ccordoba12 commented Jun 8, 2017 •

edited

goanpeca commented Jun 8, 2017 •

edited

ccordoba12 commented Jun 9, 2017 •

edited