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
Show file tree
Hide file tree
Changes from all 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
111 changes: 103 additions & 8 deletions spyder/utils/sourcecode.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@
"""

import re
import os
import sys

from spyder.py3compat import PY2
Copy link
Member

Choose a reason for hiding this comment

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

Please add a blank line before this one, to separate standard library imports from Spyder ones.

if PY2:
from itertools import izip as zip

# Order is important:
EOL_CHARS = (("\r\n", 'nt'), ("\n", 'posix'), ("\r", 'mac'))
Expand Down Expand Up @@ -120,12 +125,102 @@ def get_identifiers(source_code):
valid = re.compile(r'[a-zA-Z_]')
return [token for token in tokens if re.match(valid, token)]

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

if __name__ == '__main__':
code = 'import functools\nfunctools.partial'
assert get_primary_at(code, len(code)) == 'functools.partial'
assert set(get_identifiers(code)) == set(['import', 'functools',
'functools.partial'])
assert split_source(code) == ['import functools', 'functools.partial']
code = code.replace('\n', '\r\n')
assert split_source(code) == ['import functools', 'functools.partial']
Taken from http://stackoverflow.com/q/21498939/438386
"""
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 differentiate_prefix(path_components0, path_components1):
"""
Return the differentiated prefix of the given two iterables.

Taken from http://stackoverflow.com/q/21498939/438386
"""
longest_prefix = []
root_comparison = False
common_elmt = None
for index, (elmt0, elmt1) in enumerate(zip(path_components0, path_components1)):
if elmt0 != elmt1:
if index == 2:
root_comparison = True
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) > 0:
longest_path_prefix = os.path.join(*longest_prefix)
longest_prefix_length = len(longest_path_prefix) + 1
if path_0[longest_prefix_length:] != '' and not root_comparison:
path_0_components = path_components(path_0[longest_prefix_length:])
if path_0_components[0] == ''and path_0_components[1] == ''and len(
path_0[longest_prefix_length:]) > 20:
path_0_components.insert(2, common_elmt)
path_0 = os.path.join(*path_0_components)
else:
path_0 = path_0[longest_prefix_length:]
elif not root_comparison:
path_0 = common_elmt
elif sys.platform.startswith('linux') and path_0 == '':
path_0 = '/'
return path_0

def get_file_title(files_path_list, filename):
"""Get tab title without ambiguation."""
fname = os.path.basename(filename)
same_name_files = get_same_name_files(files_path_list, fname)
if len(same_name_files) > 1:
compare_path = shortest_path(same_name_files)
if compare_path == filename:
same_name_files.remove(path_components(filename))
compare_path = shortest_path(same_name_files)
diff_path = differentiate_prefix(path_components(filename),
path_components(compare_path))
diff_path_length = len(diff_path)
path_component = path_components(diff_path)
if (diff_path_length > 20 and len(path_component) > 2):
if path_component[0] != '/' and path_component[0] != '':
path_component = [path_component[0], '...',
path_component[-1]]
else:
path_component = [path_component[2], '...',
path_component[-1]]
diff_path = os.path.join(*path_component)
fname = fname + " - " + diff_path
return fname

def get_same_name_files(files_path_list, filename):
"""Get a list of the path components of the files with the same name."""
same_name_files = []
for fname in files_path_list:
if filename == os.path.basename(fname):
same_name_files.append(path_components(fname))
return same_name_files

def shortest_path(files_path_list):
"""Shortest path between files in the list."""
if len(files_path_list) > 0:
shortest_path = files_path_list[0]
shortest_path_length = len(files_path_list[0])
for path_elmts in files_path_list:
if len(path_elmts) < shortest_path_length:
shortest_path_length = len(path_elmts)
shortest_path = path_elmts
return os.path.join(*shortest_path)
106 changes: 106 additions & 0 deletions spyder/utils/tests/test_sourcecode.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# -*- coding: utf-8 -*-
#
# Copyright © Spyder Project Contributors
# Licensed under the terms of the MIT License

"""Tests for sourcecode.py"""

import os
import sys

import pytest

from spyder.utils import sourcecode


def test_get_primary_at():
code = 'import functools\nfunctools.partial'
assert sourcecode.get_primary_at(code, len(code)) == 'functools.partial'


def test_get_identifiers():
code = 'import functools\nfunctools.partial'
assert set(sourcecode.get_identifiers(code)) == set(['import', 'functools',
'functools.partial'])


def test_split_source():
code = 'import functools\nfunctools.partial'
assert sourcecode.split_source(code) == ['import functools', 'functools.partial']
code = code.replace('\n', '\r\n')
assert sourcecode.split_source(code) == ['import functools', 'functools.partial']


def test_path_components():
if sys.platform.startswith('linux'):
path_components0 = ['','','documents','test','test.py']
else:
path_components0 = ['c:','','documents','test','test.py']
path0 = os.path.join(*path_components0)
assert sourcecode.path_components(path0) == path_components0


def test_differentiate_prefix():
if sys.platform.startswith('linux'):
path_components0 = ['','','documents','test','test.py']
path_components1 = ['','','documents','projects','test','test.py']
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to test two path_components here? Is not enough with testing only one?

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, one is enough. I will change it. 👍

else:
path_components0 = ['c:','','documents','test','test.py']
path_components1 = ['c:','','documents','projects','test','test.py']
diff_path0 = os.path.join(*['test'])
diff_path1 = os.path.join(*['projects','test'])
assert sourcecode.differentiate_prefix(
path_components0, path_components1) == diff_path0
assert sourcecode.differentiate_prefix(
path_components1, path_components0) == diff_path1

def test_get_same_name_files():
files_path_list = []
if sys.platform.startswith('linux'):
fname0 = os.path.join(*['','','documents','test','test.py'])
files_path_list.append(fname0)
fname1 = os.path.join(*['','','documents','projects','test','test.py'])
files_path_list.append(fname1)
same_name_files = [['','','documents','test','test.py'],
['','','documents','projects','test','test.py']]
else:
fname0 = os.path.join(*['c:','','documents','test','test.py'])
files_path_list.append(fname0)
fname1 = os.path.join(*['c:','','documents','projects','test','test.py'])
files_path_list.append(fname1)
same_name_files = [['c:','','documents','test','test.py'],
['c:','','documents','projects','test','test.py']]
assert sourcecode.get_same_name_files(files_path_list
,'test.py') == same_name_files

def test_shortest_path():
if sys.platform.startswith('linux'):
files_path_list =[['','','documents','test','test.py'],
['','','documents','projects','test','test.py']]
shortest_path = os.path.join(*['','','documents','test','test.py'])
else:
files_path_list =[['c:','','documents','test','test.py'],
['c:','','documents','projects','test','test.py']]
shortest_path = os.path.join(*['c:','','documents','test','test.py'])
assert sourcecode.shortest_path(files_path_list) == shortest_path

def test_get_file_title():
files_path_list = []
if sys.platform.startswith('linux'):
fname0 = os.path.join(*['','','documents','test','test.py'])
files_path_list.append(fname0)
fname1 = os.path.join(*['','','documents','projects','test','test.py'])
files_path_list.append(fname1)
else:
fname0 = os.path.join(*['c:','','documents','test','test.py'])
files_path_list.append(fname0)
fname1 = os.path.join(*['c:','','documents','projects','test','test.py'])
files_path_list.append(fname1)
title0 = 'test.py - ' + os.path.join(*['test'])
title1 = 'test.py - ' + os.path.join(*['projects','test'])
assert sourcecode.get_file_title(files_path_list, fname0) == title0
assert sourcecode.get_file_title(files_path_list, fname1) == title1

if __name__ == '__main__':
pytest.main()

26 changes: 18 additions & 8 deletions spyder/widgets/editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -966,9 +966,12 @@ 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):
"""Return tab title"""
return self.__modified_readonly_title(osp.basename(filename),
def get_tab_text(self, index, is_modified=None, is_readonly=None):
Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @ccordoba12, CircleCi (pydocstyle) is giving me an error of docstring (D400), however the line that is pointing out (968 in widgets/editor.py) can't finish in a period (is the signature of a method) . In this case what can I do to fix the code style issue?

Copy link
Member

@ccordoba12 ccordoba12 Dec 14, 2016

Choose a reason for hiding this comment

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

I fixed this in the latest 3.x branch. Please merge it here again :-)

Copy link
Member

Choose a reason for hiding this comment

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

I finally understood what's this error is all about: you just need to add a dot to the docstring below this line, so that it reads like this

"""Return tab title."""

"""Return tab title."""
files_path_list = [finfo.filename for finfo in self.data]
fname = self.data[index].filename
fname = sourcecode.get_file_title(files_path_list, fname)
return self.__modified_readonly_title(fname,
is_modified, is_readonly)

def get_tab_tip(self, filename, is_modified=None, is_readonly=None):
Expand Down Expand Up @@ -1003,8 +1006,8 @@ def add_to_data(self, finfo, set_current):
self.data.append(finfo)
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))
editor = finfo.editor
self.tabs.insertTab(index, editor, self.get_tab_text(index))
self.set_stack_title(index, False)
if set_current:
self.set_stack_index(index)
Expand All @@ -1020,7 +1023,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, 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 @@ -1054,7 +1058,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, 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 @@ -1198,7 +1202,7 @@ 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 @@ -1599,6 +1603,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 @@ -1614,6 +1623,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
3 changes: 2 additions & 1 deletion spyder/widgets/fileswitcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,8 @@ def paths(self):

@property
def filenames(self):
return [self.tabs.tabText(index) for index in range(self.tabs.count())]
return [os.path.basename(getattr(td, 'filename',
None)) for td in self.data]

@property
def current_path(self):
Expand Down