From 45c9768c165c1c63f66e268c3977440e1b66a7e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=BCrmann?= Date: Mon, 5 Oct 2015 21:09:26 +0200 Subject: [PATCH 1/3] Added --target command line argument #922 This argument is used to override the new-instance-open-target config option. --- qutebrowser/app.py | 14 ++++++--- qutebrowser/mainwindow/mainwindow.py | 5 +-- qutebrowser/misc/ipc.py | 21 ++++++++++--- qutebrowser/qutebrowser.py | 4 +++ tests/unit/misc/test_ipc.py | 47 +++++++++++++++++----------- 5 files changed, 60 insertions(+), 31 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 11c5b05ed..a50e8ec8a 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -103,8 +103,9 @@ def run(args): if server is None: sys.exit(usertypes.Exit.ok) else: - server.got_args.connect(lambda args, cwd: - process_pos_args(args, cwd=cwd, via_ipc=True)) + server.got_args.connect(lambda args, target_arg, cwd: + process_pos_args(args, cwd=cwd, via_ipc=True, + target_arg=target_arg)) init(args, crash_handler) ret = qt_mainloop() @@ -229,7 +230,7 @@ def _load_session(name): session_manager.delete('_restart') -def process_pos_args(args, via_ipc=False, cwd=None): +def process_pos_args(args, via_ipc=False, cwd=None, target_arg=None): """Process positional commandline args. URLs to open have no prefix, commands to execute begin with a colon. @@ -255,7 +256,11 @@ def process_pos_args(args, via_ipc=False, cwd=None): log.init.debug("Empty argument") win_id = mainwindow.get_window(via_ipc, force_window=True) else: - win_id = mainwindow.get_window(via_ipc) + if via_ipc and target_arg and target_arg != 'auto': + open_target = target_arg + else: + open_target = config.get('general', 'new-instance-open-target') + win_id = mainwindow.get_window(via_ipc, open_target=open_target) tabbed_browser = objreg.get('tabbed-browser', scope='window', window=win_id) log.init.debug("Startup URL {}".format(cmd)) @@ -265,7 +270,6 @@ def process_pos_args(args, via_ipc=False, cwd=None): message.error('current', "Error in startup argument '{}': " "{}".format(cmd, e)) else: - open_target = config.get('general', 'new-instance-open-target') background = open_target in ('tab-bg', 'tab-bg-silent') tabbed_browser.tabopen(url, background=background) diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index 8b13f4e24..35b3aad2d 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -41,7 +41,7 @@ from qutebrowser.misc import crashsignal win_id_gen = itertools.count(0) -def get_window(via_ipc, force_window=False, force_tab=False): +def get_window(via_ipc, force_window=False, force_tab=False, open_target=None): """Helper function for app.py to get a window id. Args: @@ -55,7 +55,8 @@ def get_window(via_ipc, force_window=False, force_tab=False): # Initial main window return 0 window_to_raise = None - open_target = config.get('general', 'new-instance-open-target') + if not open_target: + open_target = config.get('general', 'new-instance-open-target') if (open_target == 'window' or force_window) and not force_tab: window = MainWindow() window.show() diff --git a/qutebrowser/misc/ipc.py b/qutebrowser/misc/ipc.py index 9271c4593..4c4bda1fc 100644 --- a/qutebrowser/misc/ipc.py +++ b/qutebrowser/misc/ipc.py @@ -152,7 +152,7 @@ class IPCServer(QObject): got_invalid_data: Emitted when there was invalid incoming data. """ - got_args = pyqtSignal(list, str) + got_args = pyqtSignal(list, str, str) got_raw = pyqtSignal(bytes) got_invalid_data = pyqtSignal() @@ -287,6 +287,7 @@ class IPCServer(QObject): @pyqtSlot() def on_ready_read(self): """Read json data from the client.""" + # pylint: disable=too-many-return-statements if self._socket is None: # This happens when doing a connection while another one is already # active for some reason. @@ -321,6 +322,13 @@ class IPCServer(QObject): self._handle_invalid_data() return + try: + target_arg = json_data['target_arg'] + except KeyError: + log.ipc.error("target arg missing: {}".format(decoded.strip())) + self._handle_invalid_data() + return + try: protocol_version = int(json_data['protocol_version']) except (KeyError, ValueError): @@ -336,7 +344,7 @@ class IPCServer(QObject): return cwd = json_data.get('cwd', None) - self.got_args.emit(args, cwd) + self.got_args.emit(args, target_arg, cwd) @pyqtSlot() def on_timeout(self): @@ -418,8 +426,8 @@ def _has_legacy_server(name): return False -def send_to_running_instance(socketname, command, *, legacy_name=None, - socket=None): +def send_to_running_instance(socketname, command, target_arg, *, + legacy_name=None, socket=None): """Try to send a commandline to a running instance. Blocks for CONNECT_TIMEOUT ms. @@ -448,7 +456,8 @@ def send_to_running_instance(socketname, command, *, legacy_name=None, connected = socket.waitForConnected(CONNECT_TIMEOUT) if connected: log.ipc.info("Opening in existing instance") - json_data = {'args': command, 'version': qutebrowser.__version__, + json_data = {'args': command, 'target_arg': target_arg, + 'version': qutebrowser.__version__, 'protocol_version': PROTOCOL_VERSION} try: cwd = os.getcwd() @@ -500,6 +509,7 @@ def send_or_listen(args): try: try: sent = send_to_running_instance(socketname, args.command, + args.target, legacy_name=legacy_socketname) if sent: return None @@ -513,6 +523,7 @@ def send_or_listen(args): log.init.debug("Got AddressInUseError, trying again.") time.sleep(0.5) sent = send_to_running_instance(socketname, args.command, + args.target, legacy_name=legacy_socketname) if sent: return None diff --git a/qutebrowser/qutebrowser.py b/qutebrowser/qutebrowser.py index caebd4967..ab4581575 100644 --- a/qutebrowser/qutebrowser.py +++ b/qutebrowser/qutebrowser.py @@ -66,6 +66,10 @@ def get_argparser(): "session even if one would be restored.", action='store_true') parser.add_argument('--json-args', help=argparse.SUPPRESS) + parser.add_argument('--target', choices=['auto', 'tab', 'tab-bg', + 'tab-silent', 'tab-bg-silent', 'window'], + help="How the urls should be opened if there is " + "already a qutebrowser instance running.") debug = parser.add_argument_group('debug arguments') debug.add_argument('-l', '--loglevel', dest='loglevel', diff --git a/tests/unit/misc/test_ipc.py b/tests/unit/misc/test_ipc.py index 277ad4407..474acbca9 100644 --- a/tests/unit/misc/test_ipc.py +++ b/tests/unit/misc/test_ipc.py @@ -442,8 +442,8 @@ class TestHandleConnection: assert msg in all_msgs def test_read_line_immediately(self, qtbot, ipc_server, caplog): - data = '{{"args": ["foo"], "protocol_version": {}}}\n'.format( - ipc.PROTOCOL_VERSION) + data = '{{"args": ["foo"], "target_arg": "tab", ' \ + '"protocol_version": {}}}\n'.format(ipc.PROTOCOL_VERSION) socket = FakeSocket(data=data.encode('utf-8')) ipc_server._server = FakeServer(socket) @@ -454,6 +454,7 @@ class TestHandleConnection: assert len(spy) == 1 assert spy[0][0] == ['foo'] + assert spy[0][1] == 'tab' all_msgs = [r.message for r in caplog.records()] assert "We can read a line immediately." in all_msgs @@ -490,12 +491,14 @@ NEW_VERSION = str(ipc.PROTOCOL_VERSION + 1).encode('utf-8') (b'\n', 'invalid json'), (b'{"is this invalid json?": true\n', 'invalid json'), (b'{"valid json without args": true}\n', 'no args'), - (b'{"args": [], "protocol_version": ' + OLD_VERSION + b'}\n', - 'incompatible version'), - (b'{"args": [], "protocol_version": ' + NEW_VERSION + b'}\n', - 'incompatible version'), - (b'{"args": [], "protocol_version": "foo"}\n', 'invalid version'), - (b'{"args": []}\n', 'invalid version'), + (b'{"args": []}\n', 'target arg missing'), + (b'{"args": [], "target_arg": null, "protocol_version": ' + OLD_VERSION + + b'}\n', 'incompatible version'), + (b'{"args": [], "target_arg": null, "protocol_version": ' + NEW_VERSION + + b'}\n', 'incompatible version'), + (b'{"args": [], "target_arg": null, "protocol_version": "foo"}\n', + 'invalid version'), + (b'{"args": [], "target_arg": null}\n', 'invalid version'), ]) def test_invalid_data(qtbot, ipc_server, connected_socket, caplog, data, msg): got_args_spy = QSignalSpy(ipc_server.got_args) @@ -515,8 +518,10 @@ def test_multiline(qtbot, ipc_server, connected_socket): spy = QSignalSpy(ipc_server.got_args) error_spy = QSignalSpy(ipc_server.got_invalid_data) - data = ('{{"args": ["one"], "protocol_version": {version}}}\n' - '{{"args": ["two"], "protocol_version": {version}}}\n'.format( + data = ('{{"args": ["one"], "target_arg": "tab",' + ' "protocol_version": {version}}}\n' + '{{"args": ["two"], "target_arg": null,' + ' "protocol_version": {version}}}\n'.format( version=ipc.PROTOCOL_VERSION)) with qtbot.waitSignals([ipc_server.got_args, ipc_server.got_args], @@ -526,13 +531,15 @@ def test_multiline(qtbot, ipc_server, connected_socket): assert len(spy) == 2 assert not error_spy assert spy[0][0] == ['one'] + assert spy[0][1] == 'tab' assert spy[1][0] == ['two'] + assert spy[1][1] == '' class TestSendToRunningInstance: def test_no_server(self, caplog): - sent = ipc.send_to_running_instance('qute-test', []) + sent = ipc.send_to_running_instance('qute-test', [], None) assert not sent msg = caplog.records()[-1].message assert msg == "No existing instance present (error 2)" @@ -550,7 +557,7 @@ class TestSendToRunningInstance: if not has_cwd: m = mocker.patch('qutebrowser.misc.ipc.os') m.getcwd.side_effect = OSError - sent = ipc.send_to_running_instance('qute-test', ['foo']) + sent = ipc.send_to_running_instance('qute-test', ['foo'], None) assert sent @@ -558,11 +565,12 @@ class TestSendToRunningInstance: expected_cwd = str(tmpdir) if has_cwd else '' assert len(spy) == 1 - assert spy[0] == [['foo'], expected_cwd] + assert spy[0] == [['foo'], '', expected_cwd] assert len(raw_spy) == 1 assert len(raw_spy[0]) == 1 - raw_expected = {'args': ['foo'], 'version': qutebrowser.__version__, + raw_expected = {'args': ['foo'], 'target_arg': None, + 'version': qutebrowser.__version__, 'protocol_version': ipc.PROTOCOL_VERSION} if has_cwd: raw_expected['cwd'] = str(tmpdir) @@ -572,20 +580,20 @@ class TestSendToRunningInstance: def test_socket_error(self): socket = FakeSocket(error=QLocalSocket.ConnectionError) with pytest.raises(ipc.Error) as excinfo: - ipc.send_to_running_instance('qute-test', [], socket=socket) + ipc.send_to_running_instance('qute-test', [], None, socket=socket) msg = "Error while writing to running instance: Error string (error 7)" assert str(excinfo.value) == msg def test_not_disconnected_immediately(self): socket = FakeSocket() - ipc.send_to_running_instance('qute-test', [], socket=socket) + ipc.send_to_running_instance('qute-test', [], None, socket=socket) def test_socket_error_no_server(self): socket = FakeSocket(error=QLocalSocket.ConnectionError, connect_successful=False) with pytest.raises(ipc.Error) as excinfo: - ipc.send_to_running_instance('qute-test', [], socket=socket) + ipc.send_to_running_instance('qute-test', [], None, socket=socket) msg = ("Error while connecting to running instance: Error string " "(error 7)") @@ -628,12 +636,13 @@ def test_ipcserver_socket_none(ipc_server, caplog, method, args): class TestSendOrListen: - Args = collections.namedtuple('Args', 'no_err_windows, basedir, command') + Args = collections.namedtuple('Args', 'no_err_windows, basedir, command, ' + 'target') @pytest.fixture def args(self): return self.Args(no_err_windows=True, basedir='/basedir/for/testing', - command=['test']) + command=['test'], target=None) @pytest.fixture(autouse=True) def cleanup(self): From afc166a13e254da93a6578a5d50df7df6576008f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=BCrmann?= Date: Tue, 6 Oct 2015 22:59:49 +0200 Subject: [PATCH 2/3] Coding style fixes #1002 --- qutebrowser/app.py | 10 +++++++++- qutebrowser/mainwindow/mainwindow.py | 7 +++++-- qutebrowser/misc/ipc.py | 23 ++++++++--------------- qutebrowser/qutebrowser.py | 6 +++--- tests/unit/misc/test_ipc.py | 8 ++++---- 5 files changed, 29 insertions(+), 25 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index a50e8ec8a..44d3caaf7 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -239,6 +239,14 @@ def process_pos_args(args, via_ipc=False, cwd=None, target_arg=None): args: A list of arguments to process. via_ipc: Whether the arguments were transmitted over IPC. cwd: The cwd to use for fuzzy_url. + target_arg: Command line argument received by a running instance via + ipc. If the --target argument was not specified, target_arg + will be an empty string instead of None. This behavior is + caused by the PyQt signal + ``got_args = pyqtSignal(list, str, str)`` + used in the misc.ipc.IPCServer class. PyQt converts the None + value into a null QString and then back to an empty python + string """ if via_ipc and not args: win_id = mainwindow.get_window(via_ipc, force_window=True) @@ -260,7 +268,7 @@ def process_pos_args(args, via_ipc=False, cwd=None, target_arg=None): open_target = target_arg else: open_target = config.get('general', 'new-instance-open-target') - win_id = mainwindow.get_window(via_ipc, open_target=open_target) + win_id = mainwindow.get_window(via_ipc, force_target=open_target) tabbed_browser = objreg.get('tabbed-browser', scope='window', window=win_id) log.init.debug("Startup URL {}".format(cmd)) diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index 35b3aad2d..257d6e211 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -41,13 +41,14 @@ from qutebrowser.misc import crashsignal win_id_gen = itertools.count(0) -def get_window(via_ipc, force_window=False, force_tab=False, open_target=None): +def get_window(via_ipc, force_window=False, force_tab=False, force_target=None): """Helper function for app.py to get a window id. Args: via_ipc: Whether the request was made via IPC. force_window: Whether to force opening in a window. force_tab: Whether to force opening in a tab. + force_target: Override the new-instance-open-target config """ if force_window and force_tab: raise ValueError("force_window and force_tab are mutually exclusive!") @@ -55,7 +56,9 @@ def get_window(via_ipc, force_window=False, force_tab=False, open_target=None): # Initial main window return 0 window_to_raise = None - if not open_target: + if force_target is not None: + open_target = force_target + else: open_target = config.get('general', 'new-instance-open-target') if (open_target == 'window' or force_window) and not force_tab: window = MainWindow() diff --git a/qutebrowser/misc/ipc.py b/qutebrowser/misc/ipc.py index 4c4bda1fc..a0a68c3ef 100644 --- a/qutebrowser/misc/ipc.py +++ b/qutebrowser/misc/ipc.py @@ -287,7 +287,6 @@ class IPCServer(QObject): @pyqtSlot() def on_ready_read(self): """Read json data from the client.""" - # pylint: disable=too-many-return-statements if self._socket is None: # This happens when doing a connection while another one is already # active for some reason. @@ -315,19 +314,12 @@ class IPCServer(QObject): self._handle_invalid_data() return - try: - args = json_data['args'] - except KeyError: - log.ipc.error("no args: {}".format(decoded.strip())) - self._handle_invalid_data() - return - - try: - target_arg = json_data['target_arg'] - except KeyError: - log.ipc.error("target arg missing: {}".format(decoded.strip())) - self._handle_invalid_data() - return + for name in ('args', 'target_arg'): + if name not in json_data: + log.ipc.error("Missing {}: {}".format(name, + decoded.strip())) + self._handle_invalid_data() + return try: protocol_version = int(json_data['protocol_version']) @@ -344,7 +336,7 @@ class IPCServer(QObject): return cwd = json_data.get('cwd', None) - self.got_args.emit(args, target_arg, cwd) + self.got_args.emit(json_data['args'], json_data['target_arg'], cwd) @pyqtSlot() def on_timeout(self): @@ -435,6 +427,7 @@ def send_to_running_instance(socketname, command, target_arg, *, Args: socketname: The name which should be used for the socket. command: The command to send to the running instance. + target_arg: --target command line argument socket: The socket to read data from, or None. legacy_name: The legacy name to first try to connect to. diff --git a/qutebrowser/qutebrowser.py b/qutebrowser/qutebrowser.py index ab4581575..04872ab5d 100644 --- a/qutebrowser/qutebrowser.py +++ b/qutebrowser/qutebrowser.py @@ -65,11 +65,11 @@ def get_argparser(): parser.add_argument('-R', '--override-restore', help="Don't restore a " "session even if one would be restored.", action='store_true') - parser.add_argument('--json-args', help=argparse.SUPPRESS) parser.add_argument('--target', choices=['auto', 'tab', 'tab-bg', 'tab-silent', 'tab-bg-silent', 'window'], - help="How the urls should be opened if there is " - "already a qutebrowser instance running.") + help="How URLs should be opened if there is already a " + "qutebrowser instance running.") + parser.add_argument('--json-args', help=argparse.SUPPRESS) debug = parser.add_argument_group('debug arguments') debug.add_argument('-l', '--loglevel', dest='loglevel', diff --git a/tests/unit/misc/test_ipc.py b/tests/unit/misc/test_ipc.py index 474acbca9..8d5d9231b 100644 --- a/tests/unit/misc/test_ipc.py +++ b/tests/unit/misc/test_ipc.py @@ -442,8 +442,8 @@ class TestHandleConnection: assert msg in all_msgs def test_read_line_immediately(self, qtbot, ipc_server, caplog): - data = '{{"args": ["foo"], "target_arg": "tab", ' \ - '"protocol_version": {}}}\n'.format(ipc.PROTOCOL_VERSION) + data = ('{{"args": ["foo"], "target_arg": "tab", ' + '"protocol_version": {}}}\n'.format(ipc.PROTOCOL_VERSION)) socket = FakeSocket(data=data.encode('utf-8')) ipc_server._server = FakeServer(socket) @@ -490,8 +490,8 @@ NEW_VERSION = str(ipc.PROTOCOL_VERSION + 1).encode('utf-8') (b'\x80\n', 'invalid utf-8'), (b'\n', 'invalid json'), (b'{"is this invalid json?": true\n', 'invalid json'), - (b'{"valid json without args": true}\n', 'no args'), - (b'{"args": []}\n', 'target arg missing'), + (b'{"valid json without args": true}\n', 'Missing args'), + (b'{"args": []}\n', 'Missing target_arg'), (b'{"args": [], "target_arg": null, "protocol_version": ' + OLD_VERSION + b'}\n', 'incompatible version'), (b'{"args": [], "target_arg": null, "protocol_version": ' + NEW_VERSION + From 1e8170d98b59cb89a6ce29143dbe84aa3c7a54bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=BCrmann?= Date: Tue, 6 Oct 2015 23:18:04 +0200 Subject: [PATCH 3/3] Fixed lines which were too long --- qutebrowser/app.py | 6 +++--- qutebrowser/mainwindow/mainwindow.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 44d3caaf7..abdba799e 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -244,9 +244,9 @@ def process_pos_args(args, via_ipc=False, cwd=None, target_arg=None): will be an empty string instead of None. This behavior is caused by the PyQt signal ``got_args = pyqtSignal(list, str, str)`` - used in the misc.ipc.IPCServer class. PyQt converts the None - value into a null QString and then back to an empty python - string + used in the misc.ipc.IPCServer class. PyQt converts the + None value into a null QString and then back to an empty + python string """ if via_ipc and not args: win_id = mainwindow.get_window(via_ipc, force_window=True) diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index 257d6e211..ac602194a 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -41,7 +41,8 @@ from qutebrowser.misc import crashsignal win_id_gen = itertools.count(0) -def get_window(via_ipc, force_window=False, force_tab=False, force_target=None): +def get_window(via_ipc, force_window=False, force_tab=False, + force_target=None): """Helper function for app.py to get a window id. Args: