Faster resize

[Waldner]

So I have a patch floating around that I have been maintaining for some time, it's very simple and does not break existing setups. Instead of manually adapting at every release, I wondered if it could be merged.

Split terminals can be resized (by default) with ctrl + shift + arrow keys. If you have a very big screen, that can be slow (eg from 30 to 400 columns). I propose a faster resize option (by default bound to shift + arrow keys) that makes things faster.

The attached patch implements these new functions. It's against 1.91.

There's a new config option (fast_resize_step) which defines the size to use at each resizing step for fast resizing (50 by default, the bigger the value, the faster the resize). The new ops are bound, by default, to shift + arrow keys, which can of course be changed like other key bindings.

            'resize_up_fast' : '<Shift>Up',
            'resize_down_fast' : '<Shift>Down',
            'resize_left_fast' : '<Shift>Left',
            'resize_right_fast': '<Shift>Right',

[Waldner (waldner)]

Another, simpler, option would be to directly make the step used when using ctrl + shift + arrow key configurable, rather than the fixed value of 10 that is used now. This way there's no need to add four new operation and corresponding keyboard bindings. I'm fine with this solution as well (can produce a patch).

[Waldner (waldner)]

Any feedback on this? Is this totally useless?

[Stephen Boddy (stephen-j-boddy)]

Hi Walder. It's not at all useless, I just have a lot of other stuff going on.

A few comments.

1. Choice of 50px step. Will cause different behaviour for someone who switches to and from a HiDPI display - on HiDPI it will jump about half the lines of text. Even with single DPI the first press could step x lines of text, then the second press step x-1 lines depending on fontsize. I think a better approach would be to have a jump factor z, and multiply the font width/height by z.

2. I'm not on my Linux system right now, so can't confirm, but I have a feeling you're overriding a built-in libvte shortcut for scrolling. I would be very hesitant to set those. I'd much rather leave it for those that need it to find their own shortcut.

3. No Preferences option for changing the step size (or factor as per 1). Means to change it people have to start editing config files.

4. No man page updates documenting the new option. (I will take care of the online manual at readthedocs.)

[Waldner (waldner)]

> 1. Choice of 50px step. Will cause different behaviour for someone who switches to and from a HiDPI
> display - on HiDPI it will jump about half the lines of text. Even with single DPI the first press
> could step x lines of text, then the second press step x-1 lines depending on fontsize. I think a
> better approach would be to have a jump factor z, and multiply the font width/height by z.

Well, that's why it's configurable. Everyone could set it to whatever works best for them on their concrete system. 50 is just the default, because I had to put a number there, but we can make the default 10, for example (I myself use 100). The existing "crtl + shift + arrow" approach does not use any jump factors either, it just resizes by a fixed amount of 10 IIUC. (as said in my second comment, we could just keep that mechanism without adding new stuff and simply make the "10" configurable, I'm fine with that).

> 2. I'm not on my Linux system right now, so can't confirm, but I have a feeling you're overriding
> a built-in libvte shortcut for scrolling. I would be very hesitant to set those. I'd much rather
> leave it for those that need it to find their own shortcut.

Sure, the binding can be changed (and the default too, of course). I chose shift + arrow because it was unused in my setup and was simpler than crtl + shift + arrow. Do you have a suggestion for a usable default shortcut that would not interfere with anything?

> 3. No Preferences option for changing the step size (or factor as per 1). Means to change it
> people have to start editing config files.

Oh! You're right, I though adding it to prefseditor.py would make it appear in the graphical config. I'll try to fix that.

> 4. No man page updates documenting the new option. (I will take care of the online manual at
> readthedocs.)

Ok, that's easy, I'll add it as well.

Thanks for the feedback.

[Stephen Boddy (stephen-j-boddy)]

1. I think you somewhat missed my point. :-) Yes it is configurable. But (taking me as an example) I take my HiDPI laptop on the train everyday, and have a LoDPI external monitor at home. As I switch between the two I don't want to have to change preference settings everytime to get predictable behaviour. To me predicatble is to jump X lines, not X pixels. You are mistaken about it only jumping ten. If the event widget is a terminal the line
  fontheight = widget.vte.get_char_height()
gets the height of a single char/cell/line in pixels, and resizes by that much. So on a HiDPI display where the font is 20px high, the jump is set to 20px. (Obviously width is as appropriate.)

So can I suggest:
           if self.maker.isinstance(widget, 'Terminal'):
               fontheight = widget.vte.get_char_height()
           else:
               fontheight = 10
           if fast:
               fontheight = fontheight * fast_resize_step

2. I'm not sure. The problem is that we have so many, and there so many other various environments with their own shortcuts, that it is often hard to find free ones that don't interfere with existing behaviour, while having them seem logical. From http://terminator-gtk3.readthedocs.io/en/latest/gettingstarted.html#the-scrollbar-and-scrollback-buffer it looks like I was mistakenly thinking of the Ctrl+Shift+Up/Down shortcut. Perhaps Shift+Up/Down is free, but it would need checking for other tools too, like less, vi etc., to make sure we don't block them from working properly. I think the best option is to just leave it blank for the patch, then I'll check when I'm back on Linix what gives me the warm fuzzies in practical use when using a default config file, and I can test some cmd line tools.

3. It requires using glade to edit the preferences.glade file, and adding the appropriate functions in the preferences.py

[Stephen Boddy (stephen-j-boddy)]

Addendum for 3. I'm trying to decide if there's a usecase for having different profiles have different step sizes, rather than a global setting. I'm also trying to figure out which section is the best place to add it the configurable option in the preferences pane (depends on Profile/Global). Let me mull that over before you go spending time editing the glade file.

[Waldner (waldner)]

1. I see now. For me when I wrote the patch the only thing that mattered was "it just has to go fast", but your concerns are valid.

Waiting for your decision to start working. Thanks.

[Waldner (waldner)]

So your suggestion for 1) leads to code like the following:

--------------
       if fast:
            multiplier = self.config['fast_resize_step']
        else:
            multiplier = 1

        if keyname in ['up', 'down'] and isinstance(self, Gtk.VPaned):
            # This is a key we can handle
            position = self.get_position()

            if self.maker.isinstance(widget, 'Terminal'):
                fontheight = widget.vte.get_char_height() * multiplier
            else:
                fontheight = 10 * multiplier
--------------

which to me begs the question of whether we could just make "multiplier" (or whatever we want to call it) a configurable item with a default value of 1 to avoid breaking existing setups, and do away with the 4 new shortcuts.

[Waldner (waldner)]

It all boils down to whether you think user will be fine with a single operation that can be made slower or faster, or they'll want to maintain the original behavior and also have fast mode as an additional operation, so they can decide when to use which.

[Waldner (waldner)]

For what it's worth, I've implemented the above set of changes (using separate keybindings for "fast mode" resize) at https://github.com/waldner/terminator, where I also plan to make some other slight modification to expose more data to plugins.

[Waldner (waldner)]

Sorry, I meant https://github.com/waldner/terminator/commit/ca41cb12b30c21b2bd6068eccee5cdccac1cd353.

[Waldner (waldner)]

I've just published the update version of core Terminator with support for the plugin_ng plugin architecture, along with a sample plugin to demonstrate what can be done with it. They are at https://github.com/waldner/terminator and https://github.com/waldner/terminator-runwith-plugin respectively.

Let me know whether these features could be interesting for being merged.

thanks

[Waldner (waldner)]

Sorry, I'm not very familiar with LP.
I've now created a "fast-resize" branch and requested a merge directly on LP. https://code.launchpad.net/~waldner/terminator/fast-resize/+merge/341534.

[Waldner (waldner)]

Since the two patchsets are virtually non-overlapping, I've also created a merge request for the plugin-ng branch: https://code.launchpad.net/~waldner/terminator/plugin-ng/+merge/341537. Thanks.