From 33257ac3d1de20e0f9947a69cd2c2073a3ec2672 Mon Sep 17 00:00:00 2001 From: Michael Lazar Date: Thu, 23 Jun 2016 18:30:58 -0700 Subject: [PATCH 1/8] Added logic, still need to test. --- rtv/exceptions.py | 6 ++++- rtv/page.py | 23 ++++++++++------- rtv/submission.py | 24 ++++++++++------- rtv/subreddit.py | 43 +++++++++++++++--------------- rtv/terminal.py | 66 +++++++++++++++++++++++++++++++---------------- 5 files changed, 99 insertions(+), 63 deletions(-) diff --git a/rtv/exceptions.py b/rtv/exceptions.py index 3e8fa6a..1d9f976 100644 --- a/rtv/exceptions.py +++ b/rtv/exceptions.py @@ -35,4 +35,8 @@ class ProgramError(RTVError): class BrowserError(RTVError): - "Could not open a web browser tab" \ No newline at end of file + "Could not open a web browser tab" + + +class TemporaryFileError(RTVError): + "Indicates that an error has occurred and the file should not be deleted" \ No newline at end of file diff --git a/rtv/page.py b/rtv/page.py index a5c8184..fbbbf66 100644 --- a/rtv/page.py +++ b/rtv/page.py @@ -10,6 +10,7 @@ from kitchen.text.display import textual_width from . import docs from .objects import Controller, Color, Command +from .exceptions import TemporaryFileError def logged_in(f): @@ -217,16 +218,20 @@ class Page(object): self.term.flash() return - text = self.term.open_editor(info) - if text == content: - self.term.show_notification('Canceled') - return + with self.term.open_editor(info) as text: + if text == content: + self.term.show_notification('Canceled') + return + + with self.term.loader('Editing', delay=0): + data['object'].edit(text) + time.sleep(2.0) + + if self.term.loader.exception is None: + self.refresh_content() + else: + raise TemporaryFileError() - with self.term.loader('Editing', delay=0): - data['object'].edit(text) - time.sleep(2.0) - if self.term.loader.exception is None: - self.refresh_content() @PageController.register(Command('INBOX')) @logged_in diff --git a/rtv/submission.py b/rtv/submission.py index fd4c954..a70dd3a 100644 --- a/rtv/submission.py +++ b/rtv/submission.py @@ -8,6 +8,7 @@ from . import docs from .content import SubmissionContent from .page import Page, PageController, logged_in from .objects import Navigator, Color, Command +from .exceptions import TemporaryFileError class SubmissionController(PageController): @@ -121,17 +122,20 @@ class SubmissionPage(Page): type=data['type'].lower(), content=content) - comment = self.term.open_editor(comment_info) - if not comment: - self.term.show_notification('Canceled') - return + with self.term.open_editor(comment_info) as comment: + if not comment: + self.term.show_notification('Canceled') + return - with self.term.loader('Posting', delay=0): - reply(comment) - # Give reddit time to process the submission - time.sleep(2.0) - if not self.term.loader.exception: - self.refresh_content() + with self.term.loader('Posting', delay=0): + reply(comment) + # Give reddit time to process the submission + time.sleep(2.0) + + if self.term.loader.exception is None: + self.refresh_content() + else: + raise TemporaryFileError() @SubmissionController.register(Command('DELETE')) @logged_in diff --git a/rtv/subreddit.py b/rtv/subreddit.py index 4c85fe5..21b87b6 100644 --- a/rtv/subreddit.py +++ b/rtv/subreddit.py @@ -10,6 +10,7 @@ from .page import Page, PageController, logged_in from .objects import Navigator, Color, Command from .submission import SubmissionPage from .subscription import SubscriptionPage +from .exceptions import TemporaryFileError class SubredditController(PageController): @@ -118,31 +119,31 @@ class SubredditPage(Page): return submission_info = docs.SUBMISSION_FILE.format(name=name) - text = self.term.open_editor(submission_info) - if not text or '\n' not in text: - self.term.show_notification('Canceled') - return + with self.term.open_editor(submission_info) as text: + if not text or '\n' not in text: + self.term.show_notification('Canceled') + return - title, content = text.split('\n', 1) - with self.term.loader('Posting', delay=0): - submission = self.reddit.submit(name, title, text=content, - raise_captcha_exception=True) - # Give reddit time to process the submission - time.sleep(2.0) - if self.term.loader.exception: - return + title, content = text.split('\n', 1) + with self.term.loader('Posting', delay=0): + submission = self.reddit.submit(name, title, text=content, + raise_captcha_exception=True) + # Give reddit time to process the submission + time.sleep(2.0) + if self.term.loader.exception: + raise TemporaryFileError() - # Open the newly created post - with self.term.loader('Loading submission'): - page = SubmissionPage( - self.reddit, self.term, self.config, self.oauth, - submission=submission) - if self.term.loader.exception: - return + # Open the newly created post + with self.term.loader('Loading submission'): + page = SubmissionPage( + self.reddit, self.term, self.config, self.oauth, + submission=submission) + if self.term.loader.exception: + return - page.loop() + page.loop() - self.refresh_content() + self.refresh_content() @SubredditController.register(Command('SUBREDDIT_OPEN_SUBSCRIPTIONS')) @logged_in diff --git a/rtv/terminal.py b/rtv/terminal.py index 3d2f784..70bc2f0 100644 --- a/rtv/terminal.py +++ b/rtv/terminal.py @@ -6,12 +6,14 @@ import sys import time import codecs import curses +import logging +import tempfile import webbrowser import subprocess import curses.ascii from curses import textpad +from datetime import datetime from contextlib import contextmanager -from tempfile import NamedTemporaryFile import six from kitchen.text.display import textual_width_chop @@ -27,6 +29,9 @@ except ImportError: unescape = html_parser.HTMLParser().unescape +_logger = logging.getLogger(__name__) + + class Terminal(object): MIN_HEIGHT = 10 @@ -377,37 +382,54 @@ class Terminal(object): except OSError: self.show_notification('Could not open pager %s' % pager) + @contextmanager def open_editor(self, data=''): """ Open a temporary file using the system's default editor. The data string will be written to the file before opening. This function will block until the editor has closed. At that point the file - will be read and and lines starting with '#' will be stripped. + will be read and and lines starting with '#' will be stripped. If no + errors occur, the file will be deleted when the context manager closes. """ - with NamedTemporaryFile(prefix='rtv-', suffix='.txt', mode='wb') as fp: + filename = 'rtv_{:%Y%m%d_%H%M%S}.txt'.format(datetime.now()) + filepath = os.path.join(tempfile.gettempdir(), filename) + + with codecs.open(filepath, 'w', 'utf-8') as fp: fp.write(self.clean(data)) - fp.flush() - editor = os.getenv('RTV_EDITOR') or os.getenv('EDITOR') or 'nano' + _logger.info('File created: {}'.format(filepath)) + editor = os.getenv('RTV_EDITOR') or os.getenv('EDITOR') or 'nano' + try: + with self.suspend(): + p = subprocess.Popen([editor, filepath]) + try: + p.wait() + except KeyboardInterrupt: + p.terminate() + except OSError: + self.show_notification('Could not open file with %s' % editor) + + with codecs.open(filepath, 'r', 'utf-8') as fp: + text = ''.join(line for line in fp if not line.startswith('#')) + text = text.rstrip() + + try: + yield text + except exceptions.TemporaryFileError: + # All exceptions will cause the file to *not* be removed, but these + # ones should also be swallowed + _logger.info('Caught TemporaryFileError') + self.show_notification('File saved as: {}'.format(text)) + else: + # If no errors occurred, try to remove the file try: - with self.suspend(): - p = subprocess.Popen([editor, fp.name]) - try: - p.wait() - except KeyboardInterrupt: - p.terminate() - except OSError: - self.show_notification('Could not open file with %s' % editor) - - # Open a second file object to read. This appears to be necessary - # in order to read the changes made by some editors (gedit). w+ - # mode does not work! - with codecs.open(fp.name, 'r', 'utf-8') as fp2: - text = ''.join(line for line in fp2 if not line.startswith('#')) - text = text.rstrip() - return text + os.remove(filepath) + except OSError as e: + _logger.exception(e) + else: + _logger.info('File deleted: {}'.format(filepath)) def text_input(self, window, allow_resize=False): """ @@ -545,4 +567,4 @@ class Terminal(object): break out = '\n'.join(stack) - return out + return out \ No newline at end of file From d81c981dbf982bff6eec0484f63b441a7c634613 Mon Sep 17 00:00:00 2001 From: Michael Lazar Date: Thu, 23 Jun 2016 22:50:12 -0700 Subject: [PATCH 2/8] Updated tests. Updated log format. --- rtv/__main__.py | 5 +++- rtv/terminal.py | 23 +++++++++------ tests/conftest.py | 4 ++- tests/test_submission.py | 6 ++-- tests/test_subreddit.py | 4 +-- tests/test_terminal.py | 60 ++++++++++++++++++++++++++++++++++++---- 6 files changed, 82 insertions(+), 20 deletions(-) diff --git a/rtv/__main__.py b/rtv/__main__.py index 490f427..5c9c4c5 100644 --- a/rtv/__main__.py +++ b/rtv/__main__.py @@ -74,7 +74,10 @@ def main(): # if args[0] != "header:": # _http_logger.info(' '.join(args)) # client.print = print_to_file - logging.basicConfig(level=logging.DEBUG, filename=config['log']) + logging.basicConfig( + level=logging.DEBUG, + filename=config['log'], + format='%(asctime)s:%(levelname)s:%(filename)s:%(lineno)d:%(message)s') else: # Add an empty handler so the logger doesn't complain logging.root.addHandler(logging.NullHandler()) diff --git a/rtv/terminal.py b/rtv/terminal.py index 70bc2f0..ccad825 100644 --- a/rtv/terminal.py +++ b/rtv/terminal.py @@ -385,19 +385,26 @@ class Terminal(object): @contextmanager def open_editor(self, data=''): """ - Open a temporary file using the system's default editor. + Open a file for editing using the system's default editor. - The data string will be written to the file before opening. This - function will block until the editor has closed. At that point the file - will be read and and lines starting with '#' will be stripped. If no - errors occur, the file will be deleted when the context manager closes. + After the file has been altered, the text will be read back and lines + starting with '#' will be stripped. If an error occurs inside of the + context manager, the file will be preserved. Otherwise, the file will + be deleted when the context manager closes. + + Params: + data (str): If provided, text will be written to the file before + opening it with the editor. + + Returns: + text (str): The text that the user entered into the editor. """ filename = 'rtv_{:%Y%m%d_%H%M%S}.txt'.format(datetime.now()) filepath = os.path.join(tempfile.gettempdir(), filename) with codecs.open(filepath, 'w', 'utf-8') as fp: - fp.write(self.clean(data)) + fp.write(data) _logger.info('File created: {}'.format(filepath)) editor = os.getenv('RTV_EDITOR') or os.getenv('EDITOR') or 'nano' @@ -426,8 +433,8 @@ class Terminal(object): # If no errors occurred, try to remove the file try: os.remove(filepath) - except OSError as e: - _logger.exception(e) + except OSError: + _logger.warning('Could not delete: {}'.format(filepath)) else: _logger.info('File deleted: {}'.format(filepath)) diff --git a/tests/conftest.py b/tests/conftest.py index 6632c31..02ee9ee 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,7 +27,9 @@ except ImportError: patch = partial(mock.patch, autospec=True) # Turn on logging, but disable vcr from spamming -logging.basicConfig(level=logging.DEBUG) +logging.basicConfig( + level=logging.DEBUG, + format='%(asctime)s:%(levelname)s:%(filename)s:%(lineno)d:%(message)s') for name in ['vcr.matchers', 'vcr.stubs']: logging.getLogger(name).disabled = True diff --git a/tests/test_submission.py b/tests/test_submission.py index 57d6922..e224bd7 100644 --- a/tests/test_submission.py +++ b/tests/test_submission.py @@ -178,7 +178,7 @@ def test_submission_comment(submission_page, terminal, refresh_token): with mock.patch('praw.objects.Submission.add_comment') as add_comment, \ mock.patch.object(terminal, 'open_editor') as open_editor, \ mock.patch('time.sleep'): - open_editor.return_value = 'comment text' + open_editor.return_value.__enter__.return_value = 'comment text' submission_page.controller.trigger('c') assert open_editor.called @@ -233,7 +233,7 @@ def test_submission_edit(submission_page, terminal, refresh_token): with mock.patch('praw.objects.Submission.edit') as edit, \ mock.patch.object(terminal, 'open_editor') as open_editor, \ mock.patch('time.sleep'): - open_editor.return_value = 'submission text' + open_editor.return_value.__enter__.return_value = 'submission text' submission_page.controller.trigger('e') assert open_editor.called @@ -249,7 +249,7 @@ def test_submission_edit(submission_page, terminal, refresh_token): with mock.patch('praw.objects.Comment.edit') as edit, \ mock.patch.object(terminal, 'open_editor') as open_editor, \ mock.patch('time.sleep'): - open_editor.return_value = 'comment text' + open_editor.return_value.__enter__.return_value = 'comment text' submission_page.controller.trigger('e') assert open_editor.called diff --git a/tests/test_subreddit.py b/tests/test_subreddit.py index 81b7730..dbc9b3e 100644 --- a/tests/test_subreddit.py +++ b/tests/test_subreddit.py @@ -148,7 +148,7 @@ def test_subreddit_post(subreddit_page, terminal, reddit, refresh_token): # Post a submission with a title but with no body subreddit_page.refresh_content(name='python') with mock.patch.object(terminal, 'open_editor'): - terminal.open_editor.return_value = 'title' + terminal.open_editor.return_value.__enter__.return_value = 'title' subreddit_page.controller.trigger('c') text = 'Canceled'.encode('utf-8') terminal.stdscr.subwin.addstr.assert_called_with(1, 1, text) @@ -160,7 +160,7 @@ def test_subreddit_post(subreddit_page, terminal, reddit, refresh_token): mock.patch.object(reddit, 'submit'), \ mock.patch('rtv.page.Page.loop') as loop, \ mock.patch('time.sleep'): - terminal.open_editor.return_value = 'test\ncontent' + terminal.open_editor.return_value.__enter__.return_value = 'test\ncont' reddit.submit.return_value = submission subreddit_page.controller.trigger('c') assert reddit.submit.called diff --git a/tests/test_terminal.py b/tests/test_terminal.py index de6a07f..ef3808b 100644 --- a/tests/test_terminal.py +++ b/tests/test_terminal.py @@ -10,6 +10,7 @@ import pytest from rtv.docs import HELP, COMMENT_EDIT_FILE from rtv.objects import Color +from rtv.exceptions import TemporaryFileError try: from unittest import mock @@ -269,7 +270,10 @@ def test_prompt_y_or_n(terminal, stdscr): assert curses.flash.called -def test_open_editor(terminal): +@pytest.mark.parametrize('ascii', [True, False]) +def test_open_editor(terminal, ascii): + + terminal.ascii = ascii comment = COMMENT_EDIT_FILE.format(content='#| This is a comment! ❤') data = {'filename': None} @@ -284,11 +288,57 @@ def test_open_editor(terminal): with mock.patch('subprocess.Popen', autospec=True) as Popen: Popen.side_effect = side_effect - reply_text = terminal.open_editor(comment) - assert reply_text == 'This is an amended comment! ❤' + with terminal.open_editor(comment) as reply_text: + assert reply_text == 'This is an amended comment! ❤' + assert os.path.isfile(data['filename']) + assert curses.endwin.called + assert curses.doupdate.called assert not os.path.isfile(data['filename']) - assert curses.endwin.called - assert curses.doupdate.called + + +def test_open_editor_error(terminal): + + with mock.patch('subprocess.Popen', autospec=True) as Popen, \ + mock.patch.object(terminal, 'show_notification'): + + # Invalid editor + Popen.side_effect = OSError + with terminal.open_editor('hello') as text: + assert text == 'hello' + assert 'Could not open' in terminal.show_notification.call_args[0][0] + + data = {'filename': None} + + def side_effect(args): + data['filename'] = args[1] + return mock.Mock() + + # Temporary File Errors don't delete the file + Popen.side_effect = side_effect + with terminal.open_editor('test'): + assert os.path.isfile(data['filename']) + raise TemporaryFileError() + assert os.path.isfile(data['filename']) + os.remove(data['filename']) + + # Other Exceptions don't delete the file *and* are propagated + Popen.side_effect = side_effect + with pytest.raises(ValueError): + with terminal.open_editor('test'): + assert os.path.isfile(data['filename']) + raise ValueError() + assert os.path.isfile(data['filename']) + os.remove(data['filename']) + + # Gracefully handle the case when we can't remove the file + with mock.patch.object(os, 'remove'): + os.remove.side_effect = OSError + with terminal.open_editor(): + pass + assert os.remove.called + + assert os.path.isfile(data['filename']) + os.remove(data['filename']) def test_open_browser(terminal): From a2a00019436cf9518c23e986c27d4573a96023f9 Mon Sep 17 00:00:00 2001 From: Michael Lazar Date: Thu, 23 Jun 2016 22:52:54 -0700 Subject: [PATCH 3/8] Added log message on session start. --- rtv/__main__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rtv/__main__.py b/rtv/__main__.py index 5c9c4c5..2f4aacc 100644 --- a/rtv/__main__.py +++ b/rtv/__main__.py @@ -78,6 +78,7 @@ def main(): level=logging.DEBUG, filename=config['log'], format='%(asctime)s:%(levelname)s:%(filename)s:%(lineno)d:%(message)s') + _logger.info('Starting new session, RTV v{}'.format(__version__)) else: # Add an empty handler so the logger doesn't complain logging.root.addHandler(logging.NullHandler()) From a5a42fc5dab8ad48e7118828fe2c4a458e0633a1 Mon Sep 17 00:00:00 2001 From: Michael Lazar Date: Thu, 23 Jun 2016 23:06:41 -0700 Subject: [PATCH 4/8] Fixed some error messages --- rtv/subreddit.py | 5 ++++- rtv/terminal.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/rtv/subreddit.py b/rtv/subreddit.py index 21b87b6..d931cea 100644 --- a/rtv/subreddit.py +++ b/rtv/subreddit.py @@ -120,9 +120,12 @@ class SubredditPage(Page): submission_info = docs.SUBMISSION_FILE.format(name=name) with self.term.open_editor(submission_info) as text: - if not text or '\n' not in text: + if not text: self.term.show_notification('Canceled') return + elif '\n' not in text: + self.term.show_notification('Missing body') + return title, content = text.split('\n', 1) with self.term.loader('Posting', delay=0): diff --git a/rtv/terminal.py b/rtv/terminal.py index ccad825..26c9144 100644 --- a/rtv/terminal.py +++ b/rtv/terminal.py @@ -428,7 +428,7 @@ class Terminal(object): # All exceptions will cause the file to *not* be removed, but these # ones should also be swallowed _logger.info('Caught TemporaryFileError') - self.show_notification('File saved as: {}'.format(text)) + self.show_notification('Post saved as: {}'.format(filepath)) else: # If no errors occurred, try to remove the file try: From 8529b4a313e5e2cb5d388fef17ab939c326066bd Mon Sep 17 00:00:00 2001 From: Michael Lazar Date: Thu, 23 Jun 2016 23:08:30 -0700 Subject: [PATCH 5/8] Fixed another test. --- tests/test_subreddit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_subreddit.py b/tests/test_subreddit.py index dbc9b3e..5416536 100644 --- a/tests/test_subreddit.py +++ b/tests/test_subreddit.py @@ -150,7 +150,7 @@ def test_subreddit_post(subreddit_page, terminal, reddit, refresh_token): with mock.patch.object(terminal, 'open_editor'): terminal.open_editor.return_value.__enter__.return_value = 'title' subreddit_page.controller.trigger('c') - text = 'Canceled'.encode('utf-8') + text = 'Missing body'.encode('utf-8') terminal.stdscr.subwin.addstr.assert_called_with(1, 1, text) # Post a fake submission From a2f51dfc2e634b612aefc7f71808d45560777e95 Mon Sep 17 00:00:00 2001 From: Michael Lazar Date: Thu, 23 Jun 2016 23:14:58 -0700 Subject: [PATCH 6/8] pylint. --- rtv/__init__.py | 2 +- rtv/__main__.py | 2 +- rtv/terminal.py | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rtv/__init__.py b/rtv/__init__.py index fd24762..7a6addc 100644 --- a/rtv/__init__.py +++ b/rtv/__init__.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -""" +r""" ________ __________________________ ___ __ \__________ /_____ /__(_)_ /_ __ /_/ / _ \ __ /_ __ /__ /_ __/ diff --git a/rtv/__main__.py b/rtv/__main__.py index 2f4aacc..03e5fc3 100644 --- a/rtv/__main__.py +++ b/rtv/__main__.py @@ -78,7 +78,7 @@ def main(): level=logging.DEBUG, filename=config['log'], format='%(asctime)s:%(levelname)s:%(filename)s:%(lineno)d:%(message)s') - _logger.info('Starting new session, RTV v{}'.format(__version__)) + _logger.info('Starting new session, RTV v%s', __version__) else: # Add an empty handler so the logger doesn't complain logging.root.addHandler(logging.NullHandler()) diff --git a/rtv/terminal.py b/rtv/terminal.py index 26c9144..9921443 100644 --- a/rtv/terminal.py +++ b/rtv/terminal.py @@ -405,7 +405,7 @@ class Terminal(object): with codecs.open(filepath, 'w', 'utf-8') as fp: fp.write(data) - _logger.info('File created: {}'.format(filepath)) + _logger.info('File created: %s', filepath) editor = os.getenv('RTV_EDITOR') or os.getenv('EDITOR') or 'nano' try: @@ -428,15 +428,15 @@ class Terminal(object): # All exceptions will cause the file to *not* be removed, but these # ones should also be swallowed _logger.info('Caught TemporaryFileError') - self.show_notification('Post saved as: {}'.format(filepath)) + self.show_notification('Post saved as: %s', filepath) else: # If no errors occurred, try to remove the file try: os.remove(filepath) except OSError: - _logger.warning('Could not delete: {}'.format(filepath)) + _logger.warning('Could not delete: %', filepath) else: - _logger.info('File deleted: {}'.format(filepath)) + _logger.info('File deleted: %', filepath) def text_input(self, window, allow_resize=False): """ From c5db35fe82137b738abf3facba12018188155d06 Mon Sep 17 00:00:00 2001 From: Michael Lazar Date: Thu, 23 Jun 2016 23:23:02 -0700 Subject: [PATCH 7/8] Delete file before opening newly created post. --- rtv/subreddit.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rtv/subreddit.py b/rtv/subreddit.py index d931cea..a03cb1a 100644 --- a/rtv/subreddit.py +++ b/rtv/subreddit.py @@ -136,6 +136,7 @@ class SubredditPage(Page): if self.term.loader.exception: raise TemporaryFileError() + if not self.term.loader.exception: # Open the newly created post with self.term.loader('Loading submission'): page = SubmissionPage( From 5f64a3e005e58d5c490743ab5720e90f8e40068e Mon Sep 17 00:00:00 2001 From: Michael Lazar Date: Thu, 23 Jun 2016 23:25:56 -0700 Subject: [PATCH 8/8] typo in log messages. --- rtv/terminal.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rtv/terminal.py b/rtv/terminal.py index 9921443..d7fa38e 100644 --- a/rtv/terminal.py +++ b/rtv/terminal.py @@ -434,9 +434,9 @@ class Terminal(object): try: os.remove(filepath) except OSError: - _logger.warning('Could not delete: %', filepath) + _logger.warning('Could not delete: %s', filepath) else: - _logger.info('File deleted: %', filepath) + _logger.info('File deleted: %s', filepath) def text_input(self, window, allow_resize=False): """