Adds completion for tab-focus. #1317

toofar · 2016-02-18T06:07:42Z

This is so you can jump between tabs based on tab title or URL instead
of just index just like the buffer command in vim. Adds a b alias
for tab-focus to be more like vim too.

The method of updating the completion model (rebuild the whole thing
every time a tab is created, removed or updated) is inefficient but is
still very fast for even unreasonable numbers of open tabs.

TODO: I did a quick test and it doesn't seem to work with multiple windows.
Probably because of how I am getting the win_id. Can anyone tell me how
to get the current win_id?

The-Compiler · 2016-02-18T06:10:35Z

qutebrowser/browser/commands.py

+            try:
+                index = int(index)
+            except ValueError:
+                raise cmdexc.CommandError("tab-focus takes an int or 'last' not: {}".format(index))


What's the difference between checking this here and the way it was before (via the argument annotation, index: {'type': (int, 'last')}=None)?

(I didn't actually check, but they should do the exact same thing, no?)

Oh I suppose it would do the same thing. Since typing until there is only one option left then pressing enter passes a string through I was going to add a title to index lookup there for one less keypress but that seems like way too much effort. I'll change it back.

The-Compiler · 2016-02-18T06:30:24Z

Handling multiple windows obviously is the biggest problem here, yeah. The concept of a "current window" doesn't really exist when the completions are being rebuilt, and currently there's no mechanism for the completion to let the model know the current window when the completion is being invoked.

Those are good ideas to keep in mind for #74 somewhen in the future though.

I'd suggest a slightly different approach: Don't add a completion for :tab-focus for now, but add a different :buffer command to switch to any tab (in any window).

Then you can iterate over all windows (e.g. via for win_id in objreg.window_registry:) and build a completion for all of them, using a completion category for each window.

The-Compiler · 2016-02-18T06:32:39Z

#32 is the related issue, by the way - a :buffer command is one of the most requested features and I never got to it, so thanks for tackling this, I'm sure many will appreciate it!

vyp · 2016-02-18T08:09:51Z

a :buffer command is one of the most requested features and I never got to it, so thanks for tackling this, I'm sure many will appreciate it!

aye aye! ✨ 😃 👍

toofar · 2016-02-19T07:39:53Z

Alrighty. I changed the completion model to iterate over all windows, put tab-focus back how it was, add a new buffer command which can switch focus between windows and can work based on a substring so that you can be lazy and don't have to actually select anything in the completion widget.

The-Compiler · 2016-02-21T20:04:53Z

So I played with this a bit - seems quite nice, but it seems the completion doesn't update correctly when I do :open -w example.com (or open new tabs in the new window).

I'll also add some line comments for some style things - thanks for this again, looking forward to it!

The-Compiler · 2016-02-21T20:05:55Z

qutebrowser/browser/commands.py

+            index: The win_id/index of the tab to focus. Or a substring
+                   in which case the closest match will be focused.
+        """
+        if not index or not len(index):


Empty strings are false in Python, so checking if not index: here is enough.

On the other hand, why not just make index mandatory by removing the =None default?

toofar · 2016-02-22T02:20:26Z

but it seems the completion doesn't update correctly on new windows

Yeah ... coulda sworn that worked during testing. Looks like because there is no notification of new windows (or new tabbed_browsers) the completion model never gets a chance to hook up to the new new_tab signal.

toofar · 2016-02-22T03:24:19Z

I cleaned up commands:buffer() and added a signal
Application.new_window() which is emited from the MainWindow.__init__()
because it looks like windows are made all over the place and the main
application is the only non-per-window object (?). Not entirely sure how elegent that is but it works better now!

The-Compiler · 2016-02-22T10:18:16Z

I think that's the best solution - I'll leave some more line comments.

The-Compiler · 2016-02-22T10:18:49Z

qutebrowser/browser/commands.py

+            idx = int(index)
+            win_id = 0
+            active_win = objreg.get('app').activeWindow()
+            if not active_win == None:


This should be if active_win is not None:

The-Compiler · 2016-02-22T10:29:48Z

Can you please also try writing some end-to-end tests for this? You can find some inspiration in tests/integration/features/tabs.feature. You can run those locally by installing tox and optionally Xvfb (to avoid windows popping up), and then run tox -e py35 -- tests/integration/features/test_tabs.py. (or py34 if you're on a system with Python 3.4)

toofar · 2016-03-07T21:38:39Z

Sorry for taking so long getting back to you. Writing tests is not very motivating. Hope this addresses your concerns.

The-Compiler · 2016-03-08T06:00:57Z

qutebrowser/browser/commands.py

+                # Not sure how you enter a command without and active window...
+                raise cmdexc.CommandError(
+                    "No window specified and couldn't find active window!")
+            win_id = active_win.win_id


Can you add an else: raise AssertionError("Unexpected index length {}".format(len(index_parts))) or so here, so there's a defined behaviour if that should ever happen (e.g. by someone changing the completion model in the future) rather than undefined behaviour?

Well there is a index_parts = index.split('/', 1) at the top of the method so there really isn't going to b more than 2 parts.

Fair enough, I missed that 😄

AeliusSaionji · 2016-03-23T17:05:47Z

fwiw, I've been @toofar 's tab-complete branch with no issues. If I try to access :buffers before the browser finishes starting up, the list will be empty, but otherwise all is good.

The-Compiler · 2016-03-23T17:06:54Z

Thanks for letting me know! Now that the crowdfunding is up I can get back to coding and reviewing PRs, so I'll probably merge this in the coming few days.

Using -m py.test seems to cause some issues, see e.g. #1317 (comment)

The-Compiler · 2016-03-29T19:03:57Z

qutebrowser/completion/models/miscmodels.py

+        QTimer.singleShot(0, self.rebuild_cat)
+
+    @pyqtSlot(str)
+    def rebuild_cat(self, arg=None): # pylint: disable=unused-argument


What's the reason for the argument here? It seems to work fine when I use @pyqtSlot() and drop the , arg=None. Note slots can have less arguments than the signals they're connected to.

The-Compiler · 2016-03-29T19:09:12Z

When I close a window, I still see the window category when typing :buffer until I close the next tab - I think that's just due to objreg.window_registry still containing the windows for a little bit until everything is cleaned up? Not sure if you can do much about this, though...

Could you also fix the line comments and the few nitpicks found by the linters?

************* Module qutebrowser.completion.models.miscmodels
C:187, 4: Missing method docstring (missing-docstring)

./qutebrowser/completion/models/miscmodels.py:197:37: E261 at least two spaces before inline comment

Can you also run scripts/dev/src2asciidoc.py and commit the result?

After that, I think this is ready to merge - thanks a lot!

toofar · 2016-03-29T23:26:10Z

When I close a window, I still see the window category when typing :buffer until I close the next tab - I think that's just due to objreg.window_registry still containing the windows for a little bit until everything is cleaned up? Not sure if you can do much about this, though...

Since I already added a tabbed_browser.new_window signal I suppose I could add a window_close signal too.

The-Compiler · 2016-03-30T04:40:48Z

Thanks for the quick reaction!

Since I already added a tabbed_browser.new_window signal I suppose I could add a window_close signal too.

Sounds good - can you try if that helps? I'm not entirely sure (due to how window_registry works), but I think it should.

What do you think about keybindings for :set-cmd-text -s :buffer? Since we normally conform to dwb keybindings, that'd be gt, but that's already bound to :tab-focus (by @thiagowfx in #443)... I'm somewhat inclined to just change that to conform with dwb like everything else.

`buffer` takes either a tab index or a string and focuses the specified tab. The index can be of the form [0-9]+ which will switch to the relevant tab in the current window or [0-9]+/[0-9]+ (that is win_id/index) which will focus the specified window before switching tabs. If a string is passed the list of open tabs across all windows is sorted based on title and url (just like in the completion widget) and the top result is selected.

The buffer_troubling_args tests may look a little un-intuitive but that is because they are testing the edge cases for the current behaviour. If these edge cases are encountered during normal usage you are doing something wrong.

toofar · 2016-03-31T01:01:26Z

Turns out the model was getting rebuilt after a window closed (due to the tab_closed signal) but the categories were being created before tabbed_browser.shutting_down was checked. So that was an easy fix. (Conecting to the MainWindow.destroyed signal and setting a timer also worked.)

For the keybindings I am just using b. I'm not really a fan of most of the dwb keybindings, I would prefer them to more ... logical; to have more of grammer than almost arbitrary mnemonics.

The-Compiler · 2016-03-31T05:11:42Z

Merged - thanks for the great work, and sorry for the delays!

Using -m py.test seems to cause some issues, see e.g. qutebrowser#1317 (comment)

See #1317, #32

The-Compiler · 2020-05-16T13:22:26Z

It's been a couple of years - but since a long time, we can get the current win_id for the completion, so I added :tab-focus completion based on the :buffer completion now!

rien333 · 2020-05-16T19:10:01Z

Thanks for still being committed to PRs as old as this one! I understood that the PR backlog was a bit of hurdle, so good luck with the other ones you plan to do!

The-Compiler reviewed Feb 18, 2016
View reviewed changes

toofar force-pushed the tab-complete branch from f53b6e2 to 4d69219 Compare February 19, 2016 07:36

The-Compiler reviewed Feb 21, 2016
View reviewed changes

toofar force-pushed the tab-complete branch from 4d69219 to 800654f Compare February 22, 2016 03:22

The-Compiler reviewed Feb 22, 2016
View reviewed changes

toofar force-pushed the tab-complete branch from 800654f to b8eae17 Compare March 7, 2016 21:31

The-Compiler reviewed Mar 8, 2016
View reviewed changes

toofar force-pushed the tab-complete branch 3 times, most recently from 95b6229 to 7f86c61 Compare March 22, 2016 21:46

The-Compiler added a commit that referenced this pull request Mar 26, 2016

tox: Use -m pytest instead of -m py.test

3ccbf3c

Using -m py.test seems to cause some issues, see e.g. #1317 (comment)

toofar force-pushed the tab-complete branch from 7f86c61 to 5bc8133 Compare March 26, 2016 22:12

The-Compiler added this to the v0.6.0 milestone Mar 29, 2016

The-Compiler self-assigned this Mar 29, 2016

The-Compiler reviewed Mar 29, 2016
View reviewed changes

toofar force-pushed the tab-complete branch from 5bc8133 to 3ce56c9 Compare March 29, 2016 23:48

toofar added 3 commits March 31, 2016 14:00

Adds tests for buffer command.

97ab6db

The buffer_troubling_args tests may look a little un-intuitive but that is because they are testing the edge cases for the current behaviour. If these edge cases are encountered during normal usage you are doing something wrong.

Add docs for :buffer

3846ce1

toofar force-pushed the tab-complete branch from 3ce56c9 to 3846ce1 Compare March 31, 2016 01:01

The-Compiler merged commit 3846ce1 into qutebrowser:master Mar 31, 2016

The-Compiler mentioned this pull request Mar 31, 2016

More completions #32

Open

26 tasks

toofar deleted the tab-complete branch March 31, 2016 22:56

ninewise pushed a commit to ninewise/qutebrowser that referenced this pull request Apr 5, 2016

tox: Use -m pytest instead of -m py.test

b59202a

Using -m py.test seems to cause some issues, see e.g. qutebrowser#1317 (comment)

The-Compiler mentioned this pull request Aug 18, 2017

Rename :buffer to :tab-select #2910

Closed

qutebrowser-bot added the status: stale label Dec 20, 2017

The-Compiler added a commit that referenced this pull request May 16, 2020

Add completion for :tab-focus

305e7c9

See #1317, #32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds completion for tab-focus. #1317

Adds completion for tab-focus. #1317

toofar commented Feb 18, 2016

The-Compiler Feb 18, 2016

toofar Feb 18, 2016

The-Compiler commented Feb 18, 2016

The-Compiler commented Feb 18, 2016

vyp commented Feb 18, 2016

toofar commented Feb 19, 2016

The-Compiler commented Feb 21, 2016

The-Compiler Feb 21, 2016

toofar commented Feb 22, 2016

toofar commented Feb 22, 2016

The-Compiler commented Feb 22, 2016

The-Compiler Feb 22, 2016

The-Compiler commented Feb 22, 2016

toofar commented Mar 7, 2016

The-Compiler Mar 8, 2016

toofar Mar 9, 2016

The-Compiler Mar 9, 2016

AeliusSaionji commented Mar 23, 2016

The-Compiler commented Mar 23, 2016

The-Compiler Mar 29, 2016

The-Compiler commented Mar 29, 2016

toofar commented Mar 29, 2016

The-Compiler commented Mar 30, 2016

toofar commented Mar 31, 2016

The-Compiler commented Mar 31, 2016

The-Compiler commented May 16, 2020

rien333 commented May 16, 2020

Adds completion for tab-focus. #1317

Adds completion for tab-focus. #1317

Conversation

toofar commented Feb 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

The-Compiler commented Feb 18, 2016

The-Compiler commented Feb 18, 2016

vyp commented Feb 18, 2016

toofar commented Feb 19, 2016

The-Compiler commented Feb 21, 2016

Choose a reason for hiding this comment

toofar commented Feb 22, 2016

toofar commented Feb 22, 2016

The-Compiler commented Feb 22, 2016

Choose a reason for hiding this comment

The-Compiler commented Feb 22, 2016

toofar commented Mar 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AeliusSaionji commented Mar 23, 2016

The-Compiler commented Mar 23, 2016

Choose a reason for hiding this comment

The-Compiler commented Mar 29, 2016

toofar commented Mar 29, 2016

The-Compiler commented Mar 30, 2016

toofar commented Mar 31, 2016

The-Compiler commented Mar 31, 2016

The-Compiler commented May 16, 2020

rien333 commented May 16, 2020