PR: Add the possibility to rename IPython consoles #4092

tjolive · 2017-02-02T09:00:55Z

Fixes #1962

Tabs and TabBar classes (spyder/widgets/tabs.py) accept now a parameter (rename_tabs) to enable the tab renaming - defaults no disable but IPythonConsole instantiates Tabs with rename_tabs=True. If activated, the TabBar creates a new object EditTabNamePopup which is triggered by right double clicking on a tab.

The EditTabNamePopup instantiates QLineEdit and places it on top of the tab when activated. While activated, if the user clicks outside the QLineEdit of presses the ESC key, the changes are discarded (i.e. QLineEdit gets invisible).

Structure: IPythonConsole > Tabs > TabBar > EditTabNamePopUp

Some details that could be further improved:

The tab names have a margin of 9. This number was reached by trial&error, could not find it anywhere on the QTabBar/QTabWidget config. It works but would like to get this value dynamically
The QLineEdit defaults to a white background so I didn't include any config to change this. Not sure if we would like to see this customizable
The IPythonConsole class (spyder/plugins/ipythonconsole.py) already has a rename_client_tab function which is not used. Not sure if we want to keep it

goanpeca · 2017-02-02T14:33:03Z

@tjolive awesome, thanks for the help!

Could you paste some screenshots of the process or maybe an animation? Maybe use something like http://www.cockos.com/licecap/

tjolive · 2017-02-02T15:52:20Z

here you are:

Realized now that I cannot see any cursor when editing but my windows is behaving strangely...

Coded and tested on:

Windows 7
Spyder 3.2.0.dev0
Python 3.5.1 32bits
Qt 5.7.1
PyQt5 5.7.1

goanpeca

Made some style and code comments ;-)

goanpeca · 2017-02-02T16:06:13Z

spyder/plugins/ipythonconsole.py

@@ -1509,3 +1515,8 @@ def _create_client_for_kernel(self, connection_file, hostname, sshkey,

        # Register client
        self.register_client(client)
+
+    def tab_name_editor(self):
+        """Triggers the tab name editor"""


Lets add a . at the end a docstring is a sentence.

Trigger (without the s).

goanpeca · 2017-02-02T16:06:53Z

spyder/widgets/tabs.py

@@ -48,6 +48,12 @@ def __init__(self, parent, ancestor):
        self.setAcceptDrops(True)
        self.setUsesScrollButtons(True)

+        # Tab name editor
+        self.rename_tabs = rename_tabs
+        if self.rename_tabs is True:


self.rename_tabs is True this is redundant, we can just use

if self.rename_tabs:

Usually I use this as an implicit "assert isinstance(rename_tabs, bool)" but I'll align with your code style

goanpeca · 2017-02-02T16:07:25Z

spyder/widgets/tabs.py

+        self.rename_tabs = rename_tabs
+        if self.rename_tabs is True:
+            # Creates tab name editor
+            self.tab_name_editor = EditTabNamePopup(self)


This variable is only getting created if true, so maybe we should define a

self.tab_name_editor = None above

Good idea, missed that

goanpeca · 2017-02-02T16:07:34Z

spyder/widgets/tabs.py

-
+
+    def mouseDoubleClickEvent(self, event):
+        """Override Qt method to trigge the tab name editor"""


. at the end

trigger (missing r)

goanpeca · 2017-02-02T16:07:55Z

spyder/widgets/tabs.py

+                # Tab is valid, call tab name editor
+                self.tab_name_editor.edit_tab(index)
+        else:
+            # Even is not interessant, raise to parent


Event ? missed a t?

typo, thanks for spotting

goanpeca · 2017-02-02T16:08:18Z

spyder/widgets/tabs.py

+    """Popup on top of the tab to edit its name"""
+
+    def __init__(self, parent):
+        QLineEdit.__init__(self, parent=None)


We can add the same docstring here

goanpeca · 2017-02-02T16:08:37Z

spyder/widgets/tabs.py

+        self.setFrame(False)
+
+        # Align with tab name
+        self.setTextMargins(9, 0, 0, 0)


We will need to check how this looks on other OS

How are these cross-platform tests usually performed?

It looks nice on Linux 😄

I only noticed the QLineEdit a little oversized when the tab isn't active (you need to double-click a little fast in a non active tab)

Can you post some video? On OSX works also (see below comment).

But I can't see the cursor, will fix it.

goanpeca · 2017-02-02T16:09:20Z

spyder/widgets/tabs.py

+        self.setTextMargins(9, 0, 0, 0)
+
+        # Track with tab is being edited
+        self.tab_index = None


It would be good to have a similar structure to how other widgets work

# Instance variables # Widgets # Widget setup # Layouts # Signals and slots

do you have any template for such documentation?

Nope, but most of the widgets follow that convention

class SomeWidget(): # Variables self.some_variable = None # Widgets self.super_widget = SomeSuperWidget() # Widget setup self.super_widget.setSomething(True) # Layouts layout = QVBoxLayout() layout.addWidget(self.super_widget) self.setLayout(layout) # Signals and slots self.super_widget.sig_some_signal.connect(self.some_other_method)

@goanpeca We need to document that somewhere.

I added to the wiki https://github.com/spyder-ide/spyder/wiki/Dev:-Coding-Style#widget-structure

Will align the code with this.

Took the liberty to add to the wiki the docstring format as mentioned here:
https://github.com/spyder-ide/spyder/wiki/Dev:-Coding-Style#docstrings

goanpeca · 2017-02-02T16:09:31Z

spyder/widgets/tabs.py

+        self.tab_index = None
+
+    def eventFilter(self, widget, event):
+        """Catches clicks outside the object and ESC key press"""


. at the end

Catch (infinitive form)

goanpeca · 2017-02-02T16:09:43Z

spyder/widgets/tabs.py

+        return QLineEdit.eventFilter(self, widget, event)
+
+    def edit_tab(self, index):
+        """Activates the edit tab"""


. at the end

Activate (infinitive form)

rlaverde

I tested on Linux and works well. I just left some style comments.

rlaverde · 2017-02-02T21:53:59Z

spyder/plugins/ipythonconsole.py

@@ -756,6 +756,10 @@ def get_plugin_actions(self):
               _("Open a new IPython console connected to an existing kernel"),
               triggered=self.create_client_for_kernel)

+        rename_tab_action = create_action(self, _("&Rename tab"),


I think just "Rename tab"(without the &) is ok, this menu won't be accessible using shortcuts.

rlaverde · 2017-02-02T21:55:25Z

spyder/plugins/ipythonconsole.py

@@ -1509,3 +1515,8 @@ def _create_client_for_kernel(self, connection_file, hostname, sshkey,

        # Register client
        self.register_client(client)
+
+    def tab_name_editor(self):
+        """Triggers the tab name editor"""


Trigger (without the s).

rlaverde · 2017-02-02T21:57:17Z

spyder/widgets/tabs.py

-
+
+    def mouseDoubleClickEvent(self, event):
+        """Override Qt method to trigge the tab name editor"""


trigger (missing r)

rlaverde · 2017-02-02T22:03:21Z

spyder/widgets/tabs.py

+        self.tab_index = None
+
+    def eventFilter(self, widget, event):
+        """Catches clicks outside the object and ESC key press"""


Catch (infinitive form)

rlaverde · 2017-02-02T22:04:25Z

spyder/widgets/tabs.py

+        return QLineEdit.eventFilter(self, widget, event)
+
+    def edit_tab(self, index):
+        """Activates the edit tab"""


Activate (infinitive form)

tjolive · 2017-02-03T08:55:06Z

Tested on OSX Sierra 10.12.2 (16C67, Python 2.7.13 64bits, Qt 5.6.2, PyQt5 5.6 on Darwin

When editing, the text is not as aligned as in windows, maybe because the close icon is rendered on the left. In any way it looks nice and intuitive.

The cursor is also missing in OSX, did you have the same behaviour on linux?

goanpeca · 2017-02-03T14:05:24Z

This is looking great

tjolive · 2017-02-17T19:20:23Z

Hi guys,

new commit addresses all feedback (I hope) and adds a blinking cursor, which I felt was missing.

But now the Travis CI is failing on something that I didn't change and was working on the first commit.

+for f in 'spyder/*/*/*.py'
+[[ spyder/utils/introspection/docstrings.py == *test*/test_* ]]
+[[ spyder/utils/introspection/docstrings.py == spyder/external/*/*.py ]]
+[[ spyder/utils/introspection/docstrings.py == spyder/utils/external/*.py ]]
+[[ spyder/utils/introspection/docstrings.py == spyder/utils/help/*.py ]]
+[[ spyder/utils/introspection/docstrings.py == spyder/utils/ipython/start_kernel.py ]]
+[[ spyder/utils/introspection/docstrings.py == spyder/utils/ipython/spyder_kernel.py ]]
+[[ spyder/utils/introspection/docstrings.py == spyder/utils/site/sitecustomize.py ]]
+[[ spyder/utils/introspection/docstrings.py == spyder/utils/introspection/plugin_client.py ]]
+[[ spyder/utils/introspection/docstrings.py == spyder/widgets/externalshell/systemshell.py ]]
+[[ spyder/utils/introspection/docstrings.py == spyder/widgets/ipythonconsole/__init__.py ]]
+python spyder/utils/introspection/docstrings.py
Traceback (most recent call last):
  File "spyder/utils/introspection/docstrings.py", line 26, in <module>
    from jedi.evaluate.iterable import Array, FakeSequence, AlreadyEvaluated
ImportError: cannot import name 'Array'
The command "./continuous_integration/travis/modules_test.sh" exited with 1.
Done. Your build exited with 1.

Any ideas?

Do you need any more changes on this PR?

Cheers,
Telmo

ccordoba12 · 2017-02-17T19:38:50Z

Please merge with the 3.x branch to fix that error.

goanpeca · 2017-02-21T16:13:19Z

@tjolive I think you made a mistake :-|, cause now the PR is $^%^

You were supposed to merge with 3.x not master :-(

tjolive · 2017-02-21T16:22:35Z

Argh, I see, I tried to do via the github desktop app and it defaulted to master. Reverting and merging with 3.x...

goanpeca · 2017-02-21T16:42:34Z

If you are not sure how to fix this @tjolive, just start from scratch on a new PR and be more careful next time ;-)

tjolive · 2017-02-22T13:42:20Z

Revert was ok. Merged with 3.x but it didn't trigger a re-check...

Will try something else, otherwise I'll issue a new PR

goanpeca · 2017-02-22T13:45:30Z

Even if reverted... your PR is introducing too many unneeded commits, so its not ok at this point

tjolive · 2017-06-30T06:30:21Z

@gaw89: I have this as closed on my end, the coding part is done and tested, now it's up to the owner to integrate this.

…

On 29 June 2017 at 19:34, gaw89 ***@***.***> wrote: @tjolive <https://github.com/tjolive>, are you planning to come back to this PR at some point to see if it can get pushed through? I would be very interested in having this feature available in Spyder. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4092 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AYLgJTYgdqwzAdNg6vKSS4gMfMBgymg5ks5sI-BDgaJpZM4L03CB> .

gaw89 · 2017-06-30T12:32:57Z

@tjolive, I see. I asked you because of the conflicting files listed in the PR status. Is that your responsibility to figure out the conflict?

@ccordoba12, what needs to be done in order to get this PR accepted? If I can help, I'm happy to do so.

tjolive · 2017-06-30T12:43:18Z

@gaw89: i see the conflict now, i don't recall having it when i concluded the coding some months ago. Let me take a look and revert. Anyway, @ccordoba12 <https://github.com/ccordoba12> can we have your view on this one? On 30 Jun 2017 14:33, "gaw89" <notifications@github.com> wrote: @tjolive <https://github.com/tjolive>, I see. I asked you because of the conflicting files listed in the PR status. Is that your responsibility to figure out the conflict? @ccordoba12 <https://github.com/ccordoba12>, what needs to be done in order to get this PR accepted? If I can help, I'm happy to do so. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4092 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AYLgJWagatd4i2flLc8YMmJEI_htm8lIks5sJOr_gaJpZM4L03CB> .

gaw89 · 2017-06-30T12:49:40Z

@tjolive, I see. Perhaps the conflict cropped up since you submitted the PR. If there's anything I can do to help, let me know. This feature would be incredibly useful for me and a great addition to an already great IDE!

ccordoba12 · 2017-06-30T13:21:21Z

@tjolive, don't worry about the conflict. We need to fix a couple of things before merging your work, but we'll keep working on your branch.

ccordoba12 · 2017-06-30T13:22:12Z

@gaw89, this will be part of Spyder 3.2.

gaw89 · 2017-06-30T13:22:43Z

@ccordoba12, awesome! I'll keep an eye out for it. Is there a release date for 3.2?

ccordoba12 · 2017-06-30T13:28:06Z

Hopefully in a couple of weeks.

Conflicts: - spyder/plugins/ipythonconsole.py - spyder/widgets/ipythonconsole/client.py

…iven name

…e change

…dy renamed

…name is possible or not

Comments were already addressed

Fixes #1962

peendebak · 2018-05-22T06:14:01Z

@tjolive @ccordoba12 Is there an option to rename the tabs from the command line itself? E.g. import spyder; spyder.tags.rename('my name');

ccordoba12 · 2018-05-22T07:29:31Z

No, there isn't.

peendebak · 2018-05-22T12:48:39Z

@ccordoba12 Is it technically possible? E.g. can the spyder gui elements be controlled from the ipython console? If so, how, then I can make a PR to create this functionality.

PR1962: rename IPython consoles

2206cd5

goanpeca added this to the v3.2 milestone Feb 2, 2017

goanpeca assigned tjolive Feb 2, 2017

goanpeca requested review from ccordoba12, rlaverde and goanpeca February 2, 2017 16:05

goanpeca previously requested changes Feb 2, 2017

View reviewed changes

goanpeca changed the title ~~PR1962: rename IPython consoles~~ PR: rename IPython consoles Feb 2, 2017

rlaverde reviewed Feb 2, 2017

View reviewed changes

Aligned with comments, added blinking cursor

2595f68

ccordoba12 modified the milestones: v3.2, v3.3 Feb 14, 2017

ccordoba12 changed the base branch from master to 3.x February 17, 2017 19:38

ccordoba12 added 13 commits July 2, 2017 11:59

Update with 3.x

886d1cd

Conflicts: - spyder/plugins/ipythonconsole.py - spyder/widgets/ipythonconsole/client.py

IPython Console: Fix error after merge with 3.x

0b1a7ce

IPython Console: Use a dict for client id

86a126e

IPython console: Fix client id for external kernels

129848a

IPython console: Fix error when an external kernel dies

141014c

Tabs: Move EditTabNamePopup to be the first class in this file

3213ce8

IPython Console: Remove unneeded space in a tab's name when using a g…

3b044a4

…iven name

Tabs: Emit a signal after a name change

16429d4

IPython console: Rename a tab and all its related clients after a nam…

510a926

…e change

IPython console: Fix renaming slave clients when its naster was alrea…

1b7f97c

…dy renamed

Minor fixes

4f137d8

IPython console: Add allow_rename atrr to clients to decide when a re…

ad67f29

…name is possible or not

Tabs: Fix for PyQt4 in EditTabNamePopup

1f5a8fd

ccordoba12 changed the title ~~PR: Rename IPython consoles~~ PR: Add the possibility to rename IPython consoles Jul 2, 2017

IPython console: Fix tests after removing "name" attr in clients

6672619

ccordoba12 approved these changes Jul 2, 2017

View reviewed changes

ccordoba12 merged commit c92bf8f into spyder-ide:3.x Jul 2, 2017

ccordoba12 added a commit that referenced this pull request Jul 2, 2017

Merge from 3.x: PR #4092

fe8d342

Fixes #1962



		def mouseDoubleClickEvent(self, event):
		"""Override Qt method to trigge the tab name editor"""

PR: Add the possibility to rename IPython consoles #4092

PR: Add the possibility to rename IPython consoles #4092

Conversation

tjolive commented Feb 2, 2017 • edited by ccordoba12 Loading

goanpeca commented Feb 2, 2017

tjolive commented Feb 2, 2017

goanpeca left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlaverde Feb 2, 2017 • edited Loading

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

rlaverde Feb 2, 2017 • edited Loading

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

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

rlaverde left a 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

rlaverde Feb 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjolive commented Feb 3, 2017

goanpeca commented Feb 3, 2017

tjolive commented Feb 17, 2017

ccordoba12 commented Feb 17, 2017

goanpeca commented Feb 21, 2017 • edited Loading

tjolive commented Feb 21, 2017

goanpeca commented Feb 21, 2017 • edited Loading

tjolive commented Feb 22, 2017

goanpeca commented Feb 22, 2017 • edited Loading

tjolive commented Jun 30, 2017 via email

gaw89 commented Jun 30, 2017

tjolive commented Jun 30, 2017 via email

gaw89 commented Jun 30, 2017

ccordoba12 commented Jun 30, 2017

ccordoba12 commented Jun 30, 2017

gaw89 commented Jun 30, 2017

ccordoba12 commented Jun 30, 2017

peendebak commented May 22, 2018

ccordoba12 commented May 22, 2018

peendebak commented May 22, 2018

tjolive commented Feb 2, 2017 •

edited by ccordoba12

Loading

rlaverde Feb 2, 2017 •

edited

Loading

rlaverde Feb 2, 2017 •

edited

Loading

rlaverde Feb 2, 2017 •

edited

Loading

goanpeca commented Feb 21, 2017 •

edited

Loading

goanpeca commented Feb 21, 2017 •

edited

Loading

goanpeca commented Feb 22, 2017 •

edited

Loading