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 7 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
57 changes: 47 additions & 10 deletions spyder/utils/sourcecode.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
"""

import re

import os
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 +123,46 @@ def get_identifiers(source_code):
valid = re.compile(r'[a-zA-Z_]')
return [token for token in tokens if re.match(valid, token)]


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']
def path_components(path):
"""
Return the individual components of a given file path
string (for the local operating system).

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 differ prefix of the given two iterables.
Copy link
Member

Choose a reason for hiding this comment

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

Please change this to

Return the differentiated prefix ...


Taken from http://stackoverflow.com/q/21498939/438386
"""
longest_prefix = []
common_elmt = None
for (elmt0, elmt1) in zip(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]
Copy link
Member

Choose a reason for hiding this comment

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

Add spaces around the minus sign in these two lines, like this

file_name_length = len(path_components0[len(path_components0) - 1])

if(len(longest_prefix)>3):
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 spaces around >

longest_path_prefix = os.path.join(*longest_prefix)
length_to_delete = len(longest_path_prefix)-len(common_elmt)
Copy link
Member

Choose a reason for hiding this comment

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

Spaces around -

return path_0[length_to_delete:]
else:
return path_0
60 changes: 60 additions & 0 deletions spyder/utils/tests/test_sourcecode.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# -*- 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(tmpdir, qtbot):
Copy link
Member

Choose a reason for hiding this comment

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

None of these tests seems to require tmpdir nor qtbot, so please remove them

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


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

Choose a reason for hiding this comment

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

Please align this line like this

.. == set(['import', 'functools',
           'functools.partial'])



def test_split_source(tmpdir, qtbot):
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(tmpdir, qtbot):
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']
path0 = os.path.join(*path_components0)
path1 = os.path.join(*path_components1)
assert sourcecode.path_components(path0) == path_components0
assert sourcecode.path_components(path1) == path_components1


def test_differentiate_prefix(tmpdir, qtbot):
path_components0 = ['documents','test','test.py']
path_components1 = ['documents','projects','test','test.py']
differ_path0 = os.path.join(*['documents','test'])
differ_path1 = os.path.join(*['documents', 'projects', 'test'])
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 call these diff_path0 and diff_path1

assert sourcecode.differentiate_prefix(
path_components0, path_components1) == differ_path0
assert sourcecode.differentiate_prefix(
path_components1, path_components0) == differ_path1


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

43 changes: 37 additions & 6 deletions spyder/widgets/editor.py
Original file line number Diff line number Diff line change
Expand Up @@ -941,11 +941,33 @@ 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, source in enumerate(self.data):
fname_to_compare = osp.basename(source.filename)
if findex is not index and fname == fname_to_compare:
differ_path = sourcecode.differentiate_prefix(
sourcecode.path_components(finfo.filename),
sourcecode.path_components(source.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_component = sourcecode.path_components(differ_path)
last_component_index = len(path_component) - 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's no need of this variable. You can select the last element of this list using:

path_component[-1]

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

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 +1001,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 +1017,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 +1052,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 +1194,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 +1583,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 +1603,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