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.

Sign up for GitHub

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: Limit the number of lines in history according to the relevant setting #7132

Merged
merged 8 commits into from
Jun 15, 2018
Merged

PR: Limit the number of lines in history according to the relevant setting #7132

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3c12105
Fixes #7080
impact27 May 15, 2018
922e0f7
pep8
impact27 May 15, 2018
788993f
Correct a typo
impact27 May 15, 2018
b58c529
Don't use CONF because it's not needed
ccordoba12 May 26, 2018
0c4b215
utils/sourcecode: Add a function to normalize eols in a text to a sin…
ccordoba12 Jun 15, 2018
a777927
History: Normalize eols before trying to detect how many lines are on it
ccordoba12 Jun 15, 2018
12e8cd3
History: Really preserve max_entries when cutting history
ccordoba12 Jun 15, 2018
2a39855
Testing: Add max_entries test for history
ccordoba12 Jun 15, 2018
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
7 changes: 7 additions & 0 deletions spyder/plugins/history.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# Standard library imports
import os.path as osp
import sys
import re

# Third party imports
from qtpy import PYQT5
Expand All @@ -29,6 +30,7 @@
from spyder.widgets.tabs import Tabs
from spyder.widgets.sourcecode import codeeditor
from spyder.widgets.findreplace import FindReplace
from spyder.config.main import CONF
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this. It's not needed.



class HistoryConfigPage(PluginConfigPage):
Expand Down Expand Up @@ -230,6 +232,11 @@ def add_history(self, filename):
editor.toggle_wrap_mode( self.get_option('wrap') )

text, _ = encoding.read(filename)
linebreaks = [m.start() for m in re.finditer('\n', text)]
Copy link
Member

Choose a reason for hiding this comment

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

We need to convert all EOL's to \n before doing this.

Copy link
Member

@CAM-Gerlach CAM-Gerlach May 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Can't we just always use LF for our own history files, independent of the platform?

maxNline = CONF.get('historylog', 'max_entries')
Copy link
Member

Choose a reason for hiding this comment

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

Plugins can access config their own options like this

maxNline = self.get_option('max_entries')

if len(linebreak) > maxNline:
Copy link
Member

Choose a reason for hiding this comment

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

This is where all the tests are failing--it should be linebreaks presumably. Easy mistake to make, but one a basic code checker or a simple test (unit test or just actually testing that the functionality worked without error) would have caught.

text = text[linebreaks[-maxNline] +1:]
Copy link
Member

Choose a reason for hiding this comment

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

Whitespace around +

encoding.write(text, filename)
editor.set_text(text)
editor.set_cursor_position('eof')

Expand Down