Code review refactoring

rtv/clipboard.py

@@ -1,42 +1,52 @@
 # -*- coding: utf-8 -*-
-import os
-import platform
+from __future__ import unicode_literals
+
+import sys
 import subprocess
 
 from .exceptions import ProgramError
 
+
 def _subprocess_copy(text, args_list):
     p = subprocess.Popen(args_list, stdin=subprocess.PIPE, close_fds=True)
     p.communicate(input=text.encode('utf-8'))
 
+
 def copy(text):
     """
     Copy text to OS clipboard.
     """
 
-    if os.name == 'mac' or platform.system() == 'Darwin':
-        return copy_osx(text)
-    elif os.name == 'posix' or platform.system() == 'Linux':
-        return copy_linux(text)
+    if sys.platform == 'darwin':
+        copy_osx(text)
     else:
-        raise NotImplementedError
+        # For Linux, BSD, cygwin, etc.
+        copy_linux(text)
+
 
 def copy_osx(text):
     _subprocess_copy(text, ['pbcopy', 'w'])
 
+
 def copy_linux(text):
+
     def get_command_name():
         # Checks for the installation of xsel or xclip
         for cmd in ['xsel', 'xclip']:
             cmd_exists = subprocess.call(
                 ['which', cmd],
-                stdout=subprocess.PIPE, stderr=subprocess.PIPE) is 0
+                stdout=subprocess.PIPE, stderr=subprocess.PIPE) == 0
             if cmd_exists:
                 return cmd
         return None
-    cmd_args = {'xsel' : ['xsel', '-b', '-i'],
-                'xclip' : ['xclip', '-selection', 'c']}
+
+    cmd_args = {
+        'xsel': ['xsel', '-b', '-i'],
+        'xclip': ['xclip', '-selection', 'c']}
     cmd_name = get_command_name()
+
     if cmd_name is None:
-        raise ProgramError("External copy application not installed")
+        raise ProgramError("External copy application not found,\n"
+                           "try installing either `xsel` or `xclip`")
+
     _subprocess_copy(text, cmd_args.get(cmd_name))

rtv/page.py

@@ -19,6 +19,7 @@ from .__version__ import __version__
 
 _logger = logging.getLogger(__name__)
 
+
 def logged_in(f):
     """
     Decorator for Page methods that require the user to be authenticated.
@@ -49,11 +50,11 @@ class Page(object):
         self.content = None
         self.nav = None
         self.controller = None
+        self.copy_to_clipboard = copy
 
         self.active = True
         self._row = 0
         self._subwindows = None
-        self.copy_to_clipboard = copy
 
     def refresh_content(self, order=None, name=None):
         raise NotImplementedError
@@ -322,41 +323,49 @@ class Page(object):
         message = 'New Messages' if inbox > 0 else 'No New Messages'
         self.term.show_notification(message)
 
-    @PageController.register(Command('COPY_SUBMISSION_PERMALINK'))
-    def copy_submission_permalink(self):
+    @PageController.register(Command('COPY_PERMALINK'))
+    def copy_permalink(self):
         """
         Copies submission permalink to OS clipboard
         """
 
         data = self.get_selected_item()
         url = data.get('permalink')
-        if url is not None:
-            try:
-                self.copy_to_clipboard(url)
-                self.term.show_notification(
-                    'Copied {} to clipboard'.format(url), timeout=1)
-            except ProgramError as e:
-               _logger.exception(e)
-               self.term.show_notification(
-                   'Failed to copy {} to clipboard, {}'.format(url, e))
+        if url is None:
+            self.term.flash()
+            return
 
-    @PageController.register(Command('COPY_SUBMISSION_URL'))
-    def copy_post_permalink(self):
+        try:
+            self.copy_to_clipboard(url)
+        except (ProgramError, OSError) as e:
+            _logger.exception(e)
+            self.term.show_notification(
+                'Failed to copy permalink to clipboard\n{0}'.format(e))
+        else:
+            self.term.show_notification(
+                'Copied permalink to clipboard', timeout=1)
+
+    @PageController.register(Command('COPY_URL'))
+    def copy_url(self):
         """
         Copies submission url to OS clipboard
         """
 
         data = self.get_selected_item()
         url = data.get('url_full')
-        if url is not None:
-            try:
-                self.copy_to_clipboard(url)
-                self.term.show_notification(
-                    'Copied {} to clipboard'.format(url), timeout=1)
-            except ProgramError as e:
-               _logger.exception(e)
-               self.term.show_notification(
-                   'Failed to copy {} to clipboard, {}'.format(url, e))
+        if url is None:
+            self.term.flash()
+            return
+
+        try:
+            self.copy_to_clipboard(url)
+        except (ProgramError, OSError) as e:
+            _logger.exception(e)
+            self.term.show_notification(
+                'Failed to copy url to clipboard\n{0}'.format(e))
+        else:
+            self.term.show_notification(
+                'Copied url to clipboard', timeout=1)
 
     def clear_input_queue(self):
         """

rtv/templates/rtv.cfg

@@ -118,8 +118,8 @@ INBOX = i
 REFRESH = r, <KEY_F5>
 PROMPT = /
 SAVE = w
-COPY_SUBMISSION_PERMALINK = y
-COPY_SUBMISSION_URL = Y
+COPY_PERMALINK = y
+COPY_URL = Y
 
 ; Submission page
 SUBMISSION_TOGGLE_COMMENT = 0x20