PR: Create a unique instance of the file switcher attached to the main window #4626

dalthviz · 2017-06-20T21:08:37Z

A plugin could be attached to the file switcher by using the add_to_fileswitcher method of the Main Window. Also, the plugin needs to implement two methods: get_current_tab_manager and set_stack_index method. The set_stack_index method should be placed in the widget that manages the tabs.

ccordoba12 · 2017-06-20T21:13:23Z

spyder/widgets/editor.py

        self.fileswitcher_dlg.sig_goto_file.connect(self.set_stack_index)
-        self.fileswitcher_dlg.sig_close_file.connect(self.close_file)
+        self.fileswitcher_dlg.setup()
        self.fileswitcher_dlg.show()
        self.fileswitcher_dlg.is_visible = True


Do we still need this instance of the file switcher here? I mean, can we remove it given that we have now a single and global instance?

I left this for the windows that the editor could create, but I think it is possible to attach that windows to the file switcher too.

Ok, but how is the file switcher called for those windows?

As it is in this moment for those windows only by clicking in the action of the menu.

@goanpeca, what do you think? should we leave it or remove it?

Hmmm not a strong opinion either way :-|

Let's leave it for now then ;-)

ccordoba12 · 2017-06-20T21:39:57Z

Please merge with 3.x to fix errors in our tests.

ccordoba12 · 2017-06-20T23:00:07Z

spyder/app/mainwindow.py

+        self.open_fileswitcher_dlg()
+        self.fileswitcher_dlg.set_search_text('@')
+
+    def add_to_fileswitcher(self, tabs, data, plugin):


What would be data for the Notebook or the IPython console?

I'd say it's None, so in this case we should change the signature of this function to be

def add_to_fileswitcher(self, plugin, tabs, data=None):

For the Notebook data is self.clients

Ok, then let's change the signature anyway to be

def add_to_fileswitcher(self, plugin, tabs, data)

ccordoba12 · 2017-06-20T23:03:08Z

spyder/widgets/fileswitcher.py


    # Constants that define the mode in which the list widget is working
    # FILE_MODE is for a list of files, SYMBOL_MODE if for a list of symbols
    # in a given file when using the '@' symbol.
    FILE_MODE, SYMBOL_MODE = [1, 2]

-    def __init__(self, parent, tabs, data):
+    def __init__(self, parent, tabs, data, plugin):


Let's change this to be

def __init__(self, parent, plugin, tabs, data):

ccordoba12 · 2017-06-21T00:42:08Z

spyder/app/mainwindow.py

+
+    def add_to_fileswitcher(self, plugin, tabs, data):
+        """Add a plugin to the File Switcher."""
+        if self.fileswitcher_dlg == None:


The convention when comparing with None is:

if self.fileswitcher_dlg is None:

Please change it to be this way.

ccordoba12 · 2017-06-21T00:45:17Z

spyder/widgets/fileswitcher.py

-        return [getattr(td, 'newly_created', False) for td in self.data]
+        return [getattr(td,
+                        'newly_created',
+                        False) for da in self.plugins_data for td in da]


This is too complex to have it as list comprehension. Please simplify it using two list comprehensions instead.

Something like this would work?:

save_status = [] for da in self.plugins_data: save_status += [getattr(td, 'newly_created', False) for td in da] return save_status

Also, I should do this for the widgets, paths and filenames properties, right?

Yes, that works too.

Also, I should do this for the widgets, paths and filenames properties, right?

Yep.

ccordoba12 · 2017-06-21T00:45:50Z

spyder/widgets/fileswitcher.py

-        return [getattr(td, 'filename', None) for td in self.data]
+        return [getattr(td,
+                        'filename',
+                        None) for da in self.plugins_data for td in da]


The same comment applies here: please use two list comprehensions.

ccordoba12 · 2017-06-21T00:46:17Z

spyder/widgets/fileswitcher.py

@@ -335,13 +340,13 @@ def set_search_text(self, _str):
        self.edit.setText(_str)

    def save_initial_state(self):
-        """Saves initial cursors and initial active editor."""
+        """Saves initial cursors and initial active widget."""


Saves -> Save

ccordoba12 · 2017-06-21T00:50:21Z

spyder/app/mainwindow.py

@@ -322,6 +323,9 @@ def signal_handler(signum, frame=None):
        # Tour  # TODO: Should I consider it a plugin?? or?
        self.tour = None
        self.tours_available = None
+
+        # File switcher
+        self.fileswitcher_dlg = None


fileswitcher_dlg -> fileswitcher

ccordoba12 · 2017-06-21T00:50:50Z

spyder/app/mainwindow.py

@@ -2720,6 +2745,36 @@ def show_tour(self, index):
        self.tour.set_tour(index, frames, self)
        self.tour.start_tour()

+    # ---- Global File Switcher
+    def open_fileswitcher_dlg(self):


open_fileswitcher_dlg -> open_fileswitcher

ccordoba12 · 2017-06-21T00:51:09Z

spyder/app/mainwindow.py

+        self.fileswitcher_dlg.show()
+        self.fileswitcher_dlg.is_visible = True
+
+    def open_symbolfinder_dlg(self):


open_symbolfinder_dlg -> open_symbolfinder

ccordoba12 · 2017-06-21T00:51:49Z

spyder/widgets/editor.py

@@ -766,9 +766,9 @@ def open_fileswitcher_dlg(self):
            self.fileswitcher_dlg.hide()
            self.fileswitcher_dlg.is_visible = False
            return
-        self.fileswitcher_dlg = FileSwitcher(self, self.tabs, self.data)
+        self.fileswitcher_dlg = FileSwitcher(self, self, self.tabs, self.data)


fileswitcher_dlg -> fileswitcher

ccordoba12 · 2017-06-21T01:03:41Z

@dalthviz, I also think we need to define here a different icon for the files/tabs of each plugin. So when a plugin registers to the file switcher, it needs to tell it which icon is going to use.

For now let's use these icons:

TextFileIcon for the Editor.
FileIcon for the Notebook.
ipython_console for our consoles (when we implement this, so not for this PR).

ccordoba12 · 2017-06-21T05:17:53Z

spyder/plugins/editor.py

@@ -1183,6 +1164,8 @@ def register_plugin(self):
        if not editorstack.data:
            self.__load_temp_file()
        self.main.add_dockwidget(self)
+        self.main.add_to_fileswitcher(self, editorstack.tabs, editorstack.data,
+                                      icon='TextFileIcon')


This should not be the name of the icon, but the icon returned by our icon manager, i.e.

icon=ima.icon('TextFileIcon'))

Please change this to reflect that.

ccordoba12 · 2017-06-21T06:06:47Z

The last thing to address here is that now we can increase the number of characters displayed in each file path. That number is determined in the shorten_paths function of widgets/fileswitcher.py.

@dalthviz, please play with that function to see if you can improve it to display more characters.

ccordoba12 · 2017-06-21T13:59:02Z

spyder/widgets/fileswitcher.py

+                title = self.widgets[index][1].get_plugin_title().split(' - ')
+                if plugin != title[0]:
+                    plugin = title[0]
+                    text += '<big><b>' + plugin + '</b></big><br>'


Let's add some vertical separation between the plugin title and its list of files/tabs.

ccordoba12 · 2017-06-21T20:23:21Z

@dalthviz, please post a screenshot to see how is this looking :-)

ccordoba12 · 2017-06-21T20:27:17Z

spyder/app/mainwindow.py

+    def open_symbolfinder(self):
+        """Open symbol list management dialog box."""
+        self.open_fileswitcher()
+        self.fileswitcher.set_search_text('@')


When opening the symbol switcher we should give focus to the Editor because it has no meaning for the other plugins, right?

dalthviz · 2017-06-21T21:17:26Z

Preview:

ccordoba12 · 2017-06-21T21:37:43Z

Why don't the icons for the file and symbol switchers appear in the File toolbar in your screenshot?

dalthviz · 2017-06-21T22:49:11Z

I didn't add the actions in the file_toolbar_actions list. The last commit do that 👍

ccordoba12 · 2017-06-21T23:48:31Z

I think this is ready, great work @dalthviz!

Fixes #4575

dalthviz added 2 commits June 14, 2017 15:58

Set fileswitcher in the mainwindow.

142e820

File switcher working.

46f9751

dalthviz added the type:Enhancement label Jun 20, 2017

dalthviz added this to the v3.2 milestone Jun 20, 2017

dalthviz self-assigned this Jun 20, 2017

dalthviz requested review from ccordoba12 and goanpeca June 20, 2017 21:08

Deletes unused params.

af1d76f

ccordoba12 reviewed Jun 20, 2017

View reviewed changes

ccordoba12 changed the title ~~PR: Unique instance of the file switcher~~ PR: Create a unique instance of the file switcher attached to the main window Jun 20, 2017

dalthviz added 2 commits June 20, 2017 16:46

Merge for tests.

d4ce253

Deletes print statement.

a30e6df

ccordoba12 reviewed Jun 20, 2017

View reviewed changes

Changes order of params.

690645d

ccordoba12 reviewed Jun 21, 2017

View reviewed changes

Adds icon handling, changes for code style comments.

d317713

ccordoba12 reviewed Jun 21, 2017

View reviewed changes

Changes for the management of the icons.

4a7c9b0

dalthviz mentioned this pull request Jun 21, 2017

PR: Add notebook tabs to the global file switcher spyder-ide/spyder-notebook#72

Merged

ccordoba12 reviewed Jun 21, 2017

View reviewed changes

Improvements.

7f45c7b

ccordoba12 reviewed Jun 21, 2017

View reviewed changes

Symbolfinder changes plugin to the Editor.

461c6de

Adds actions to the file_toolbar.

9aa3053

ccordoba12 approved these changes Jun 21, 2017

View reviewed changes

ccordoba12 merged commit de6e249 into spyder-ide:3.x Jun 21, 2017

ccordoba12 added a commit that referenced this pull request Jun 21, 2017

Merge from 3.x: PR #4626

feb8058

Fixes #4575

ccordoba12 mentioned this pull request Jun 22, 2017

Focus doesn't go to the selected entry in file switcher when filtering #4636

Closed

jnsebgosselin mentioned this pull request Sep 13, 2017

Fast file switch and fast symbol search do not work properly if the editor is splitted #5057

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Create a unique instance of the file switcher attached to the main window #4626

PR: Create a unique instance of the file switcher attached to the main window #4626

dalthviz commented Jun 20, 2017

ccordoba12 Jun 20, 2017

dalthviz Jun 20, 2017 •

edited

Loading

ccordoba12 Jun 20, 2017

dalthviz Jun 20, 2017

ccordoba12 Jun 20, 2017

goanpeca Jun 20, 2017

ccordoba12 Jun 20, 2017

ccordoba12 commented Jun 20, 2017

ccordoba12 Jun 20, 2017

ccordoba12 Jun 20, 2017

dalthviz Jun 20, 2017

ccordoba12 Jun 20, 2017

ccordoba12 Jun 20, 2017

ccordoba12 Jun 21, 2017

ccordoba12 Jun 21, 2017

dalthviz Jun 21, 2017 •

edited

Loading

ccordoba12 Jun 21, 2017

ccordoba12 Jun 21, 2017

ccordoba12 Jun 21, 2017

ccordoba12 Jun 21, 2017

ccordoba12 Jun 21, 2017

ccordoba12 Jun 21, 2017

ccordoba12 Jun 21, 2017

ccordoba12 commented Jun 21, 2017

ccordoba12 Jun 21, 2017

ccordoba12 commented Jun 21, 2017

ccordoba12 Jun 21, 2017

ccordoba12 commented Jun 21, 2017

ccordoba12 Jun 21, 2017

dalthviz Jun 21, 2017

dalthviz commented Jun 21, 2017

ccordoba12 commented Jun 21, 2017

dalthviz commented Jun 21, 2017

ccordoba12 commented Jun 21, 2017

PR: Create a unique instance of the file switcher attached to the main window #4626

PR: Create a unique instance of the file switcher attached to the main window #4626

Conversation

dalthviz commented Jun 20, 2017

Choose a reason for hiding this comment

dalthviz Jun 20, 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

ccordoba12 commented Jun 20, 2017

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

dalthviz Jun 21, 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

ccordoba12 commented Jun 21, 2017

Choose a reason for hiding this comment

ccordoba12 commented Jun 21, 2017

Choose a reason for hiding this comment

ccordoba12 commented Jun 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dalthviz commented Jun 21, 2017

ccordoba12 commented Jun 21, 2017

dalthviz commented Jun 21, 2017

ccordoba12 commented Jun 21, 2017

dalthviz Jun 20, 2017 •

edited

Loading

dalthviz Jun 21, 2017 •

edited

Loading