From 3e5572ea25e124ce58e0f9e8fad7d762db48cf61 Mon Sep 17 00:00:00 2001 From: Michael Lazar Date: Thu, 3 Dec 2015 22:04:59 -0800 Subject: [PATCH] Fixed a few edge cases. --- rtv/oauth.py | 4 +++- rtv/terminal.py | 30 +++++++++++++++++------------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/rtv/oauth.py b/rtv/oauth.py index a0034a9..03de25e 100644 --- a/rtv/oauth.py +++ b/rtv/oauth.py @@ -97,7 +97,6 @@ class OAuthHelper(object): self.term.open_browser(authorize_url) io.start() if self.term.loader.exception: - io.clear_instance() return else: # Open the terminal webbrowser in a background thread and wait @@ -116,6 +115,9 @@ class OAuthHelper(object): elif self.params['error']: self.term.show_notification('Authentication error') return + elif self.params['state'] is None: + # Something went wrong but it's not clear what happened + return elif self.params['state'] != state: self.term.show_notification('UUID mismatch') return diff --git a/rtv/terminal.py b/rtv/terminal.py index 6281bc9..377a9e0 100644 --- a/rtv/terminal.py +++ b/rtv/terminal.py @@ -68,10 +68,11 @@ class Terminal(object): """ if self._display is None: - - # OSX doesn't set DISPLAY so we can't use this to check - # display = bool(os.environ.get("DISPLAY")) - display = True + if sys.platform == 'darwin': + # OSX doesn't always set DISPLAY so we can't use this to check + display = True + else: + display = bool(os.environ.get("DISPLAY")) # Use the convention defined here to parse $BROWSER # https://docs.python.org/2/library/webbrowser.html @@ -290,12 +291,13 @@ class Terminal(object): if self.display: command = "import webbrowser; webbrowser.open_new_tab('%s')" % url args = [sys.executable, '-c', command] - with open(os.devnull, 'ab+', 0) as 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. + null = open(os.devnull, 'ab+', 0) + 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. + try: start = time.time() while time.time() - start < 5: code = p.poll() @@ -303,12 +305,14 @@ class Terminal(object): break # Success elif code is not None: raise exceptions.BrowserError( - 'Process exited with status=%s' % code) + 'Browser 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 + finally: + # Can't check the loader exception because the oauth module + # supersedes this loader and we need to always kill the + # process if escape is pressed p.terminate() else: with self.suspend():