diff --git a/rtv/exceptions.py b/rtv/exceptions.py index 34a6d4b..ab6a8c6 100644 --- a/rtv/exceptions.py +++ b/rtv/exceptions.py @@ -28,3 +28,7 @@ class SubscriptionError(RTVError): class ProgramError(RTVError): "Problem executing an external program" + + +class BrowserError(RTVError): + "Could not open a web browser tab" \ No newline at end of file diff --git a/rtv/objects.py b/rtv/objects.py index c4b419f..1792fd1 100644 --- a/rtv/objects.py +++ b/rtv/objects.py @@ -112,6 +112,7 @@ class LoadScreen(object): """ HANDLED_EXCEPTIONS = [ + (exceptions.BrowserError, 'Could not open browser'), (exceptions.SubscriptionError, 'No Subscriptions'), (exceptions.AccountError, 'Unable to Access Account'), (exceptions.SubredditError, 'Invalid Subreddit'), diff --git a/rtv/terminal.py b/rtv/terminal.py index 5b92fb0..6281bc9 100644 --- a/rtv/terminal.py +++ b/rtv/terminal.py @@ -16,8 +16,8 @@ from tempfile import NamedTemporaryFile import six from kitchen.text.display import textual_width_chop +from . import exceptions from .objects import LoadScreen, Color -from .exceptions import EscapeInterrupt, ProgramError class Terminal(object): @@ -68,7 +68,11 @@ class Terminal(object): """ if self._display is None: - display = bool(os.environ.get("DISPLAY")) + + # OSX doesn't set DISPLAY so we can't use this to check + # display = bool(os.environ.get("DISPLAY")) + display = True + # Use the convention defined here to parse $BROWSER # https://docs.python.org/2/library/webbrowser.html console_browsers = ['www-browser', 'links', 'links2', 'elinks', @@ -287,7 +291,25 @@ class Terminal(object): command = "import webbrowser; webbrowser.open_new_tab('%s')" % url args = [sys.executable, '-c', command] with open(os.devnull, 'ab+', 0) as null: - subprocess.check_call(args, stdout=null, stderr=null) + p = subprocess.Popen(args, stdout=null, stderr=null) + with self.loader(message='Opening page in a new window'): + # Give the browser 5 seconds to open a new tab. Because the + # display is set, calling webbrowser should be non-blocking. + # If it blocks or returns an error, something went wrong. + start = time.time() + while time.time() - start < 5: + code = p.poll() + if code == 0: + break # Success + elif code is not None: + raise exceptions.BrowserError( + 'Process exited with status=%s' % code) + time.sleep(0.01) + else: + raise exceptions.BrowserError('Timeout opening browser') + if self.loader.exception is not None: + # Cleanup the process + p.terminate() else: with self.suspend(): webbrowser.open_new_tab(url) @@ -310,7 +332,8 @@ class Terminal(object): with self.suspend(): subprocess.Popen([editor, fp.name]).wait() except OSError: - raise ProgramError('Could not open file with %s' % editor) + raise exceptions.ProgramError( + '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+ @@ -343,9 +366,9 @@ class Terminal(object): def validate(ch): "Filters characters for special key sequences" if ch == self.ESCAPE: - raise EscapeInterrupt() + raise exceptions.EscapeInterrupt() if (not allow_resize) and (ch == curses.KEY_RESIZE): - raise EscapeInterrupt() + raise exceptions.EscapeInterrupt() # Fix backspace for iterm if ch == curses.ascii.DEL: ch = curses.KEY_BACKSPACE @@ -358,7 +381,7 @@ class Terminal(object): out = textbox.edit(validate=validate) if isinstance(out, six.binary_type): out = out.decode('utf-8') - except EscapeInterrupt: + except exceptions.EscapeInterrupt: out = None curses.curs_set(0) @@ -442,4 +465,4 @@ class Terminal(object): break out = '\n'.join(stack) - return out \ No newline at end of file + return out diff --git a/tests/test_terminal.py b/tests/test_terminal.py index ce4e7db..1b76e9a 100644 --- a/tests/test_terminal.py +++ b/tests/test_terminal.py @@ -28,9 +28,10 @@ def test_terminal_properties(terminal, config): assert len(terminal.guilded) == 2 assert isinstance(terminal.guilded[0], six.text_type) + # DISPLAY isn't reliable on all systems so we no longer rely on it. terminal._display = None with mock.patch.dict('os.environ', {'DISPLAY': ''}): - assert terminal.display is False + assert terminal.display is True terminal._display = None with mock.patch.dict('os.environ', {'DISPLAY': ':0', 'BROWSER': 'w3m'}): @@ -279,9 +280,10 @@ def test_open_browser(terminal): url = 'http://www.test.com' terminal._display = True - with mock.patch('subprocess.check_call', autospec=True) as check_call: + with mock.patch('subprocess.Popen', autospec=True) as Popen: + Popen.return_value.poll.return_value = 0 terminal.open_browser(url) - assert check_call.called + assert Popen.called assert not curses.endwin.called assert not curses.doupdate.called