Merge pull request #231 from michael-lazar/exception_file_save

Exception file save

rtv/__init__.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-"""
+r"""
 ________     __________________________
 ___  __ \__________  /_____  /__(_)_  /_
 __  /_/ /  _ \  __  /_  __  /__  /_  __/

rtv/__main__.py

@@ -74,7 +74,11 @@ 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')
+        _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())

rtv/exceptions.py

@@ -36,3 +36,7 @@ class ProgramError(RTVError):
 
 class BrowserError(RTVError):
     "Could not open a web browser tab"
+
+
+class TemporaryFileError(RTVError):
+    "Indicates that an error has occurred and the file should not be deleted"

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)
+        with self.term.open_editor(info) as text:
-        if text == content:
+            if text == content:
-            self.term.show_notification('Canceled')
+                self.term.show_notification('Canceled')
-            return
+                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

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)
+        with self.term.open_editor(comment_info) as comment:
-        if not comment:
+            if not comment:
-            self.term.show_notification('Canceled')
+                self.term.show_notification('Canceled')
-            return
+                return
 
-        with self.term.loader('Posting', delay=0):
+            with self.term.loader('Posting', delay=0):
-            reply(comment)
+                reply(comment)
-            # Give reddit time to process the submission
+                # Give reddit time to process the submission
-            time.sleep(2.0)
+                time.sleep(2.0)
-        if not self.term.loader.exception:
+
-            self.refresh_content()
+            if self.term.loader.exception is None:
+                self.refresh_content()
+            else:
+                raise TemporaryFileError()
 
     @SubmissionController.register(Command('DELETE'))
     @logged_in

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,35 @@ class SubredditPage(Page):
             return
 
         submission_info = docs.SUBMISSION_FILE.format(name=name)
-        text = self.term.open_editor(submission_info)
+        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')
+                self.term.show_notification('Canceled')
-            return
+                return
+            elif '\n' not in text:
+                self.term.show_notification('Missing body')
+                return
 
-        title, content = text.split('\n', 1)
+            title, content = text.split('\n', 1)
-        with self.term.loader('Posting', delay=0):
+            with self.term.loader('Posting', delay=0):
-            submission = self.reddit.submit(name, title, text=content,
+                submission = self.reddit.submit(name, title, text=content,
-                                            raise_captcha_exception=True)
+                                                raise_captcha_exception=True)
-            # Give reddit time to process the submission
+                # Give reddit time to process the submission
-            time.sleep(2.0)
+                time.sleep(2.0)
-        if self.term.loader.exception:
+            if self.term.loader.exception:
-            return
+                raise TemporaryFileError()
 
-        # Open the newly created post
+        if not self.term.loader.exception:
-        with self.term.loader('Loading submission'):
+            # Open the newly created post
-            page = SubmissionPage(
+            with self.term.loader('Loading submission'):
-                self.reddit, self.term, self.config, self.oauth,
+                page = SubmissionPage(
-                submission=submission)
+                    self.reddit, self.term, self.config, self.oauth,
-        if self.term.loader.exception:
+                    submission=submission)
-            return
+            if self.term.loader.exception:
+                return
 
-        page.loop()
+            page.loop()
 
-        self.refresh_content()
+            self.refresh_content()
 
     @SubredditController.register(Command('SUBREDDIT_OPEN_SUBSCRIPTIONS'))
     @logged_in

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,61 @@ 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.
+        Open a file for editing using the system's default editor.
 
-        The data string will be written to the file before opening. This
+        After the file has been altered, the text will be read back and lines
-        function will block until the editor has closed. At that point the file
+        starting with '#' will be stripped. If an error occurs inside of the
-        will be read and and lines starting with '#' will be stripped.
+        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.
         """
 
-        with NamedTemporaryFile(prefix='rtv-', suffix='.txt', mode='wb') as fp:
+        filename = 'rtv_{:%Y%m%d_%H%M%S}.txt'.format(datetime.now())
-            fp.write(self.clean(data))
+        filepath = os.path.join(tempfile.gettempdir(), filename)
-            fp.flush()
-            editor = os.getenv('RTV_EDITOR') or os.getenv('EDITOR') or 'nano'
 
+        with codecs.open(filepath, 'w', 'utf-8') as fp:
+            fp.write(data)
+        _logger.info('File created: %s', 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('Post saved as: %s', filepath)
+        else:
+            # If no errors occurred, try to remove the file
             try:
-                with self.suspend():
+                os.remove(filepath)
-                    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)
+                _logger.warning('Could not delete: %s', filepath)
-
+            else:
-            # Open a second file object to read. This appears to be necessary
+                _logger.info('File deleted: %s', filepath)
-            # 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
 
     def text_input(self, window, allow_resize=False):
         """

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
 

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

tests/test_subreddit.py

@@ -148,9 +148,9 @@ 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')
+        text = 'Missing body'.encode('utf-8')
         terminal.stdscr.subwin.addstr.assert_called_with(1, 1, text)
 
     # Post a fake submission
@@ -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

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)
+        with terminal.open_editor(comment) as reply_text:
-        assert reply_text == 'This is an amended comment! ❤'
+            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):