Created attachment 127625 [details] [review]
Patch to gnome-terminal-2.25.3 - Adds user-definable keys for scroll up/down 1line/1page top/bottom

I know that there are keys available to do various different types of
scrolling, but the predefined key combinations drove me crazy because I was
used to different keys from a different terminal emulator.  I see that the VTE
package is where these key combinations are hardcoded, but I thought the
functionality makes more sense in the gnome-terminal package because scrolling
is part of the GUI functionality and not part of terminal emulation.  

This patch will add user-definable keys to:
scroll up 1 line
scroll down 1 line
scroll up one page
scroll down one page
scroll to top
scroll to bottom

Hope this is useful.

Vte bug 399057 makes these gtkbindings keybindings, meaning you could configure them in a gtkrc file...

How does one create such a binding via a user-friendly GUI?  If it can only be accomplished by text-editing a separate configuration file, I suggest something should be implemented that is more approachable to the end-user.  I'm willing to do this if someone can give me some direction as to the best way this should be done.  I thought that having the key customization clearly visible in the gnome-terminal GUI was the most intuitive place for the end-user.  

Reviewing attachment 127625 [details] [review]

First, please create patches with diff -up (or just use git diff). 

+ #ifndef USE_SCROLLED_WINDOW
+ GtkWidget * terminal_screen_container_get_scrollbar (GtkWidget *container) ;
+ #endif

I don't agree with making this API public. Also, your code below doesn't take the USE_SCROLLED_WINDOW case into account. All you need is the adjustment, not the scrollbar; get it directly from the VteTerminal (vte_terminal_get_adjustment).

+   int current;
+   TerminalScreen *screen = terminal_window_get_active( window ) ;
+   GtkWidget *scrollbar = terminal_screen_container_get_scrollbar( GTK_WIDGET (screen)->parent ) ;
+ 
+   if (screen == NULL)
+     return;
+ 
+   current = gtk_range_get_value( GTK_RANGE( scrollbar ) ) ;
+ 
+   if ( current )
+     gtk_range_set_value( GTK_RANGE ( scrollbar ), current - 1 ) ;
+ 

Major code style issues: space before (, no space after ( or before ) and ;.

+   gtk_widget_show( screen ) ;
+   gtk_widget_show( window ) ;

Why?

Plus, I don't think this is the right approach. If scrolling programmatically is wanted, we should make the internal vte API for this public.

etc etc.

+       <menuitem action="ViewScrollDown1Line" />
+       <menuitem action="ViewScrollUp1Line" />
+       <menuitem action="ViewScrollDown1Page" />
+       <menuitem action="ViewScrollUp1Page" />
+       <menuitem action="ViewScrollToTop" />
+       <menuitem action="ViewScrollToBottom" />

I don't think these actions should be available in the menus at all.

---

However, the question really is, why add configurable keybinding for these and not for any other of the many many key sequences vte handles internally?

(In reply to comment #4)
> Reviewing attachment 127625 [details] [review] [edit]
> 
> First, please create patches with diff -up (or just use git diff). 
> 
> + #ifndef USE_SCROLLED_WINDOW
> + GtkWidget * terminal_screen_container_get_scrollbar (GtkWidget *container) ;
> + #endif
> 
> I don't agree with making this API public. Also, your code below doesn't take
> the USE_SCROLLED_WINDOW case into account. All you need is the adjustment, not
> the scrollbar; get it directly from the VteTerminal
> (vte_terminal_get_adjustment).
> 
> +   int current;
> +   TerminalScreen *screen = terminal_window_get_active( window ) ;
> +   GtkWidget *scrollbar = terminal_screen_container_get_scrollbar( GTK_WIDGET
> (screen)->parent ) ;
> + 
> +   if (screen == NULL)
> +     return;
> + 
> +   current = gtk_range_get_value( GTK_RANGE( scrollbar ) ) ;
> + 
> +   if ( current )
> +     gtk_range_set_value( GTK_RANGE ( scrollbar ), current - 1 ) ;
> + 
> 
> Major code style issues: space before (, no space after ( or before ) and ;.
> 
> +   gtk_widget_show( screen ) ;
> +   gtk_widget_show( window ) ;
> 
> Why?
> 
> Plus, I don't think this is the right approach. If scrolling programmatically
> is wanted, we should make the internal vte API for this public.
> 
> etc etc.
> 
> +       <menuitem action="ViewScrollDown1Line" />
> +       <menuitem action="ViewScrollUp1Line" />
> +       <menuitem action="ViewScrollDown1Page" />
> +       <menuitem action="ViewScrollUp1Page" />
> +       <menuitem action="ViewScrollToTop" />
> +       <menuitem action="ViewScrollToBottom" />
> 
> I don't think these actions should be available in the menus at all.
> 
> ---
> 
> However, the question really is, why add configurable keybinding for these and
> not for any other of the many many key sequences vte handles internally?
> 

I'll make whatever vte API calls that are necessary public if that's what is acceptable to you.

I'll make the configuration options in any place/manner that's agreeable to you.

I only added these particular bindings because they were the only ones that mattered to me, personally.  If you want me to add all of them, I'll do that as well.

Basically - like I said before - if you can give me some direction as to what you would accept as a patch to make this happen, I shall do it.  I'd rather not guess and have you tell me later that it fails because of x,y,z reasons.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-terminal/-/issues/6777.