PR: Migrate line number area to a panel widget #3463
Conversation
Do you plan to add all of this in this PR? |
|
I mean, I'd prefer if you only focus on moving the LineNumberArea widget out of CodeEditor :-) |
|
I think we can make the Panel be the Last PR, before code folding? (or with code folding, since it will better adapt to that concept) |
I was planing to add that changes too, but as @goanpeca says It will be better to let that for the last PR, after moving the others panels (scrollflagarea, edgeline, and gotoline), so I think this PR is complete. |
|
👍 |
|
I don't understand what's happening with Travis. I'll fix it when I come back to Colombia :-) |
|
@rlaverde, could you rebase or merge this with master? It seems you started this work before 3.0 was released, and that's making Travis to fail :-) |
rlaverde
force-pushed
the
line-number-migration
branch
from
c451777
to
b84de4a
Compare
|
@rlaverde is this working fine and ready to be reviewed? or work is still missing? |
|
@goanpeca yes, I think it's ready (because Panel class will be other PR) |
|
@ccordoba12 or @Nodd or @jitseniesen could you review this one? |
|
Will do tomorrow :-) |
|
@goanpeca In BaseEditMixin there's It's ok to delete that function? and what should I do with that function call in https://github.com/spyder-ide/spyder/blob/master/spyder/widgets/mixins.py#L108? (change it for |
|
@rlaverde since the mixin was made to have a common set of funcs for the editor and consoles then I guess we need to leave it there for backwards compatibility. So you need to leave tha method there, and in the case of the editor we need to overload it to do So no, dont delete those methods from mixin, we just override it on the code editor to get the correct behavior. |
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
Please add our header here too and a docstring describing what is the purpose of this module.
| self.editor.setViewportMargins(self.compute_width(), 0, | ||
| self.editor.get_scrollflagarea_width(), 0) | ||
|
|
||
| def update_linenumberarea(self, qrect, dy): |
There was a problem hiding this comment.
Should we call this one just update?
There was a problem hiding this comment.
I mean, instead of update_linenumberarea which sounds redundant now :-)
There was a problem hiding this comment.
There is a function called update in qwidget class http://doc.qt.io/qt-5/qwidget.html#update, because of that I didn't change this name, should I override this function?
There was a problem hiding this comment.
Nop, then could we call it update_ or perhaps update_widget?
There was a problem hiding this comment.
I changed it for update_
|
I left some minor comments, but this looks good to me :-) Thanks @rlaverde! |
| else: | ||
| self.update(0, qrect.y(), | ||
| self.width(), | ||
| qrect.height()) |
There was a problem hiding this comment.
Please fix the formatting of this call, it's a bit odd :-)
|
Thanks a lot @rlaverde! Merging now :-) |
ccordoba12
changed the title
|
WOOT :-) |
ccordoba12
changed the title
The idea of this PR is to make as a proof of concept for the decoupling of the editors components (line number, scrollbar... ) and make the editor easier to extend through the creation of new panels
I had move the linenumber methods to the LineNumberArea class, there's missing the creation of the Panel class (base class for editor components) and add some management to editors Class to automatize the painting of panels.