Opening internal pages in a new tab vs. current tab

[merspieler]

A detail, that caught my attention is, that :version opens in a new tab while :messages and :set opens in the current tab.

Would be nice, if this would be consistent across all such commands.

[user202729]

Check out :help :messages. There's -t flag to open in a new tab.

:set doesn't have one but open -t qute://settings/ can be used.

[merspieler]

sure... but in my opinion, the default behaviour should be the same, no matter, if current tab or not...
new users just don't read the help for every command, and different behaviour is counterintuitive

[lufte]

I also find this kind of annoying. Is this OK to fix? Should we default to new tab or same tab?

[The-Compiler]

I'd say:

• Open in the current tab by default (you can always use :back, and it's what you'd expect from e.g. :open without arguments as well)
• Where there aren't --tab / --bg / --window arguments (like with :messages) yet, add them.
• :set is a bit of a special case, as it's kind of overloaded. I think it'd be fine to re-use -t (--temp) for a new tab as well (so :set and :set -t work), but I probably wouldn't add --bg/--window there.

Let me know what you think!

[merspieler]

Sounds reasonable to me.

[lufte]

I'm taking a look at this. Some commands, like :version, get the tabbed-browser from objreg and open new pages using tabbed_browser.load_url. I would like to get the command-dispatcher instead, to be able to use the CommandDispatcher._open method rather than replicating all of that logic (support for -t/-b/-w/-p. checking that they are exclusive, etc) in every command I need to update. Does it make sense to do something like this or do you think this introduces new dependencies to objreg as you are trying to minimize those? Note that these commands are already using objreg anyway.

diff --git a/qutebrowser/misc/utilcmds.py b/qutebrowser/misc/utilcmds.py
index 8c2462b2b..258145635 100644
--- a/qutebrowser/misc/utilcmds.py
+++ b/qutebrowser/misc/utilcmds.py
@@ -260,15 +260,19 @@ def window_only(current_win_id: int) -> None:
 
 @cmdutils.register()
 @cmdutils.argument('win_id', value=cmdutils.Value.win_id)
-def version(win_id: int, paste: bool = False) -> None:
+def version(win_id: int, tab: bool = False, bg: bool = False, window: bool = False,
+            paste: bool = False) -> None:
     """Show version information.
 
     Args:
+        tab: Open in a new tab.
+        bg: Open in a background tab.
+        window: Open in a new window.
         paste: Paste to pastebin.
     """
-    tabbed_browser = objreg.get('tabbed-browser', scope='window',
-                                window=win_id)
-    tabbed_browser.load_url(QUrl('qute://version/'), newtab=True)
+    command_dispatcher = objreg.get('command-dispatcher', scope='window',
+                                    window=win_id, from_command=True)
+    command_dispatcher._open(QUrl('qute://version/'), tab, bg, window)
 
     if paste:
         pastebin_version()

[The-Compiler]

@lufte I'd like to avoid commands calling other commands, except in places where such a command is actually coming from an user (such as :later, :repeat or :run-with-count). Additionally, command-dispatcher should go away at some point, see #1129 - in summary, this adds a lot of new dependencies on code which I don't really consider "the right way" to do things 🙂

I'd rather move the logic from CommandDispatcher._open somewhere where both the commands in CommandDispatcher as well as :version and friends can access it easily. Not sure were it'd match best off-hand and currently on holidays, so I'll get back to you about that later unless you find some place where it'd fit in nicely. The whole "how do we open URLs" API needs a bit of a cleanup in general, see #150 too.

[lufte]

Ah, yes, I remember seeing #150 but I didn't know about #1129. What do you mean by commands calling other commands though? This change only means that new commands would be making use of CommandDispatcher... or is that class considered a command itself? I thought it was some sort of utility class that any command could use.

This particular issue could still be solved by updating the small set of commands that open a new tab by default and make them support -t, which TabbedBrowser.load_url does support; but maybe it's not even worth it. Anyway, I'll let you get back to your holidays.

[amacfie]

I'd say:

• Open in the current tab by default (you can always use :back, and it's what you'd expect from e.g. :open without arguments as well)
• Where there aren't --tab / --bg / --window arguments (like with :messages) yet, add them.
• :set is a bit of a special case, as it's kind of overloaded. I think it'd be fine to re-use -t (--temp) for a new tab as well (so :set and :set -t work), but I probably wouldn't add --bg/--window there.

Let me know what you think!

My preference would be that the default is opening in a new tab. When I run :help I don't think of it as opening a web page but something auxiliary and I don't want it to take over the current tab.