Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR: Add disambiguation functionality for Editor tabs #3715

Merged
merged 25 commits into from
Dec 17, 2016
Merged
Changes from 3 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e99a311
Added disambiguation functionality for Editor tabs
dalthviz Nov 20, 2016
69ced28
Enhanced logic to generate titles for tabs. Added validation and logi…
dalthviz Nov 24, 2016
5f62542
Merge remote-tracking branch 'upstream/3.x' into fixes_issue_3160
dalthviz Nov 24, 2016
c65c066
Changes following the review. Added test_sourcecode.py
dalthviz Nov 29, 2016
f302a7b
Changes in the test for sourcecode.py to include an operative system …
dalthviz Dec 1, 2016
42a4052
Changes in the test for sourcecode.py to include an operative system …
dalthviz Dec 1, 2016
c9d4d38
Changes in the way to import izip
dalthviz Dec 1, 2016
4295fa8
Added blank line to separate standard library imports from Spyder ones
dalthviz Dec 1, 2016
ff2e144
Changes following the review
dalthviz Dec 2, 2016
da30594
Changed name of variables to diff..., deleted variable for last index.
dalthviz Dec 2, 2016
81f98ba
Changes tabs titles in the file switcher to only show the file name
dalthviz Dec 5, 2016
f4d596f
Changes for not take into account the last common directory from the …
dalthviz Dec 6, 2016
606e4d0
Improved validation for taking into account when the common prefix pa…
dalthviz Dec 7, 2016
2cc4574
Fixed problem when comparing with root directory.
dalthviz Dec 10, 2016
3aea355
Merge for test.
dalthviz Dec 10, 2016
75ddace
Fixed code style problems.
dalthviz Dec 11, 2016
7cbbdd1
Added case for comparison between file in root directory and one in a…
dalthviz Dec 11, 2016
2d9acbe
Added support for linux.
dalthviz Dec 11, 2016
1473091
Merge for tests with CircleCI.
dalthviz Dec 11, 2016
858414e
Improvements in the logic. Pass all the logic to utils/sourcecode.py.…
dalthviz Dec 14, 2016
b8b8b7a
Change of data structure from data of the editor to a list. Changes i…
dalthviz Dec 14, 2016
a4d1c73
Merge for tests.
dalthviz Dec 14, 2016
4e0bdb0
Added validation to detect common parent directory when applying path…
dalthviz Dec 14, 2016
92a7b7a
Merge for tests.
dalthviz Dec 14, 2016
0163546
Fixes pydocstyle D400.
dalthviz Dec 17, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 83 additions & 6 deletions spyder/widgets/editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
import os
import os.path as osp
import sys
try:
# Python 2
from itertools import izip
except ImportError:
# Python 3
izip = zip
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this import to py3compat


# Third party imports
from qtpy import is_pyqt46
Expand Down Expand Up @@ -941,11 +947,73 @@ def __modified_readonly_title(self, title, is_modified, is_readonly):
title = "(%s)" % title
return title

def get_tab_text(self, filename, is_modified=None, is_readonly=None):
def get_tab_text(self, index, filename, is_modified=None, is_readonly=None):
"""Return tab title"""
return self.__modified_readonly_title(osp.basename(filename),
fname = self.get_file_title(index)
return self.__modified_readonly_title(fname,
is_modified, is_readonly)

def get_file_title(self, index):
"""Get tab title without ambiguation."""
finfo = self.data[index]
fname = osp.basename(finfo.filename)
for findex, file in enumerate(self.data):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file is a reserved word in Python 2, so you need to use a different name here.

fname_to_compare = osp.basename(file.filename)
if findex is not index and fname == fname_to_compare:
differ_path = self.differ_prefix(
self.path_components(finfo.filename),
self.path_components(file.filename))
differ_path_length = len(differ_path)
if (differ_path_length > 20):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This length is too long. Let's change it to 4.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the length changes to 4 this will be the way the titles will be shown:

image

Is this format ok?

path_components = self.path_components(differ_path)
last_component_index = len(path_components) -1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a missing space here after the minus sign:

len(path_components) - 1

path_components = [path_components[0], '...',
path_components[last_component_index]]
differ_path = os.path.join(*path_components)
fname = fname + " - " + differ_path
break
return fname

def path_components(self, path):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this function to utils/sourcecode.py and add a test for it. For that you need to move the tests at the end of that file to the tests folder in utils, in a file called test_sourcecode.py.

"""Return the individual components of the given file path
string (for the local operating system). Taked from
http://stackoverflow.com/questions/21498939/how-to-circumvent
-the-fallacy-of-pythons-os-path-commonprefix."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This docstring should be formatted like this

"""
Return the individual components of a given file path
string (for the local operating system).

Taken from http://stackoverflow.com/q/21498939/438386
"""

That link is the short link to the SO question you referenced. You can get those links by pressing the share links that are placed below every SO question or answer.

components = []
# The loop guarantees that the returned components can be
# os.path.joined with the path separator and point to the same
# location:
while True:
(new_path, tail) = os.path.split(path) # Works on any platform
components.append(tail)
if new_path == path: # Root (including drive, on Windows) reached
break
path = new_path
components.append(new_path)
components.reverse() # First component first
return components

def differ_prefix(self, path_components0, path_components1):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also move this function to utils/sourcecode.py and a test for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

differ is a verb. Did you mean different here? Or maybe differentiate?

"""Return the differ prefix of the given two iterables. Based
on longest_prefix in http://stackoverflow.com/questions/21498939/how-to-
circumvent-the-fallacy-of-pythons-os-path-commonprefix."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reformat this docstring as I said above for path_components.

longest_prefix = []
common_elmt = None
for (elmt0, elmt1) in izip(path_components0, path_components1):
if elmt0 != elmt1:
break
else:
common_elmt = elmt0
longest_prefix.append(elmt0)
file_name_length = len(path_components0[len(path_components0)-1])
path_0 = os.path.join(*path_components0)[:-file_name_length-1]
if(len(longest_prefix)>3):
longest_path_prefix = os.path.join(*longest_prefix)
length_to_delete = len(longest_path_prefix)-len(common_elmt)
return path_0[length_to_delete:]
else:
return path_0

def get_tab_tip(self, filename, is_modified=None, is_readonly=None):
"""Return tab menu title"""
if self.fullpath_sorting_enabled:
Expand Down Expand Up @@ -979,7 +1047,7 @@ def add_to_data(self, finfo, set_current):
self.data.sort(key=self.__get_sorting_func())
index = self.data.index(finfo)
fname, editor = finfo.filename, finfo.editor
self.tabs.insertTab(index, editor, self.get_tab_text(fname))
self.tabs.insertTab(index, editor, self.get_tab_text(index, fname))
self.set_stack_title(index, False)
if set_current:
self.set_stack_index(index)
Expand All @@ -995,7 +1063,8 @@ def __repopulate_stack(self):
is_modified = True
else:
is_modified = None
tab_text = self.get_tab_text(finfo.filename, is_modified)
index = self.data.index(finfo)
tab_text = self.get_tab_text(index, finfo.filename, is_modified)
tab_tip = self.get_tab_tip(finfo.filename)
index = self.tabs.addTab(finfo.editor, tab_text)
self.tabs.setTabToolTip(index, tab_tip)
Expand Down Expand Up @@ -1029,7 +1098,7 @@ def set_stack_title(self, index, is_modified):
fname = finfo.filename
is_modified = (is_modified or finfo.newly_created) and not finfo.default
is_readonly = finfo.editor.isReadOnly()
tab_text = self.get_tab_text(fname, is_modified, is_readonly)
tab_text = self.get_tab_text(index, fname, is_modified, is_readonly)
tab_tip = self.get_tab_tip(fname, is_modified, is_readonly)
self.tabs.setTabText(index, tab_text)
self.tabs.setTabToolTip(index, tab_tip)
Expand Down Expand Up @@ -1171,7 +1240,9 @@ def close_file(self, index=None, force=False):
if self.get_stack_count() == 0 and self.create_new_file_if_empty:
self.sig_new_file[()].emit()
return False


self.__modify_stack_title()

return is_ok

def close_all_files(self):
Expand Down Expand Up @@ -1558,6 +1629,11 @@ def __check_file_status(self, index):
# Finally, resetting temporary flag:
self.__file_status_flag = False

def __modify_stack_title(self):
for index, finfo in enumerate(self.data):
state = finfo.editor.document().isModified()
self.set_stack_title(index, state)
Copy link
Member

@ccordoba12 ccordoba12 Dec 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you have to add this function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this function in order to refresh the names in the tabs after a file was closed or opened, this includes the name of the file that is currently on focus in the editor.

Copy link
Member

@ccordoba12 ccordoba12 Dec 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this piece of code could also solve issue #2907.

Could you verify if that's the case or not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this function solves that issue 😄


def refresh(self, index=None):
"""Refresh tabwidget"""
if index is None:
Expand All @@ -1573,6 +1649,7 @@ def refresh(self, index=None):
self.__refresh_statusbar(index)
self.__refresh_readonly(index)
self.__check_file_status(index)
self.__modify_stack_title()
self.update_plugin_title.emit()
else:
editor = None
Expand Down