tmux/window-copy.c - copy mode scroll behaviour

2016-02-11 Thread Michal Mazurek
Hello,

I know the code is now locked, but I'd like to ask for comments on the
following diff.

I believe that page-up and halfpage-up should have the same effect as
running scroll-up (^Y) the equivalent number of times. There are two
differences:
- with line select active ('V' by default) pgup/pgdown messes up the
selection. (steps to reproduce: select a line with 'V', press pgup,
scroll down with ^E).
- the 'x' position of the cursor doesn't change, even if the current
line length becomes shorter.

The diff below modifies window_copy_pageup() and adds window_copy_pagedown().
These changes remove the differences.

Both nvi and mg exhibit different behaviours from each other, from what
is implemented now, and what I'm proposing. mg always moves the cursor
to position 0. nvi retains the position if the new lines length is equal
or longer, or moves the position to the first printable character
otherwise.

If it looks OK I'd like to keep working on it, make
MODEKEYCOPY_HALFPAGEUP and MODEKEYCOPY_HALFPAGEDOWN use these functions
as well.

Index: window-copy.c
===
RCS file: /cvs/src/usr.bin/tmux/window-copy.c,v
retrieving revision 1.145
diff -u -p -r1.145 window-copy.c
--- window-copy.c   5 Feb 2016 10:20:06 -   1.145
+++ window-copy.c   11 Feb 2016 18:22:59 -
@@ -26,6 +26,7 @@
 
 struct screen *window_copy_init(struct window_pane *);
 void   window_copy_free(struct window_pane *);
+void   window_copy_pagedown(struct window_pane *);
 void   window_copy_resize(struct window_pane *, u_int, u_int);
 void   window_copy_key(struct window_pane *, struct client *, struct session *,
key_code, struct mouse_event *);
@@ -324,15 +325,80 @@ window_copy_pageup(struct window_pane *w
 {
struct window_copy_mode_data*data = wp->modedata;
struct screen   *s = >screen;
-   u_intn;
+   u_intn, ox, oy;
+
+   oy = screen_hsize(data->backing) + data->cy - data->oy;
+   ox = window_copy_find_length(wp, oy);
+
+   if (s->sel.lineflag == LINE_SEL_LEFT_RIGHT && oy == data->sely)
+   window_copy_other_end(wp);
+
+   if (data->cx != ox) {
+   data->lastcx = data->cx;
+   data->lastsx = ox;
+   }
+   data->cx = data->lastcx;
 
n = 1;
if (screen_size_y(s) > 2)
n = screen_size_y(s) - 2;
+
if (data->oy + n > screen_hsize(data->backing))
data->oy = screen_hsize(data->backing);
else
data->oy += n;
+
+   if (!data->screen.sel.flag || !data->rectflag) {
+   u_int py = screen_hsize(data->backing) + data->cy - data->oy;
+   u_int px = window_copy_find_length(wp, py);
+   if ((data->cx >= data->lastsx && data->cx != px) || data->cx > 
px)
+   window_copy_cursor_end_of_line(wp);
+   }
+
+   window_copy_update_selection(wp, 1);
+   window_copy_redraw_screen(wp);
+}
+
+void
+window_copy_pagedown(struct window_pane *wp)
+{
+   struct window_copy_mode_data*data = wp->modedata;
+   struct screen   *s = >screen;
+   u_intn, ox, oy;
+
+   oy = screen_hsize(data->backing) + data->cy - data->oy;
+   ox = window_copy_find_length(wp, oy);
+
+   if (s->sel.lineflag == LINE_SEL_RIGHT_LEFT && oy == data->sely)
+   window_copy_other_end(wp);
+
+   if (data->cx != ox) {
+   data->lastcx = data->cx;
+   data->lastsx = ox;
+   }
+   data->cx = data->lastcx;
+
+   n = 1;
+   if (screen_size_y(s) > 2)
+   n = screen_size_y(s) - 2;
+
+   if (data->oy < n)
+   data->oy = 0;
+   else
+   data->oy -= n;
+
+   if (!data->screen.sel.flag || !data->rectflag) {
+   u_int py = screen_hsize(data->backing) + data->cy - data->oy;
+   u_int px = window_copy_find_length(wp, py);
+   if ((data->cx >= data->lastsx && data->cx != px) || data->cx > 
px)
+   window_copy_cursor_end_of_line(wp);
+   }
+
+   if (data->scroll_exit && data->oy == 0) {
+   window_pane_reset_mode(wp);
+   return;
+   }
+
window_copy_update_selection(wp, 1);
window_copy_redraw_screen(wp);
 }
@@ -479,21 +545,8 @@ window_copy_key(struct window_pane *wp, 
window_copy_pageup(wp);
break;
case MODEKEYCOPY_NEXTPAGE:
-   n = 1;
-   if (screen_size_y(s) > 2)
-   n = screen_size_y(s) - 2;
-   for (; np != 0; np--) {
-   if (data->oy < n)
-   data->oy = 0;
-   else
-   data->oy -= n;
-   }
-   if (data->scroll_exit && 

Re: tmux/window-copy.c - copy mode scroll behaviour

2016-02-11 Thread Michal Mazurek
On 20:24:02, 11.02.16, Nicholas Marriott wrote:
> Hi
> 
> On Thu, Feb 11, 2016 at 08:05:54PM +0100, Michal Mazurek wrote:
> > Hello,
> > 
> > I know the code is now locked, but I'd like to ask for comments on the
> > following diff.
> > 
> > I believe that page-up and halfpage-up should have the same effect as
> > running scroll-up (^Y) the equivalent number of times. There are two
> > differences:
> > - with line select active ('V' by default) pgup/pgdown messes up the
> > selection. (steps to reproduce: select a line with 'V', press pgup,
> > scroll down with ^E).
> > - the 'x' position of the cursor doesn't change, even if the current
> > line length becomes shorter.
> 
> I like the current behaviour, it is nice that page up then page down
> will leave things unchanged no matter what line it ended up on.

The patch preserves this behaviour, similar to pressing the up arrow
and then the down arrow, unless I misunderstood something.

-- 
Michal Mazurek



Re: tmux/window-copy.c - copy mode scroll behaviour

2016-02-11 Thread Nicholas Marriott
Hi

On Thu, Feb 11, 2016 at 08:05:54PM +0100, Michal Mazurek wrote:
> Hello,
> 
> I know the code is now locked, but I'd like to ask for comments on the
> following diff.
> 
> I believe that page-up and halfpage-up should have the same effect as
> running scroll-up (^Y) the equivalent number of times. There are two
> differences:
> - with line select active ('V' by default) pgup/pgdown messes up the
> selection. (steps to reproduce: select a line with 'V', press pgup,
> scroll down with ^E).
> - the 'x' position of the cursor doesn't change, even if the current
> line length becomes shorter.

I like the current behaviour, it is nice that page up then page down
will leave things unchanged no matter what line it ended up on.

> 
> The diff below modifies window_copy_pageup() and adds window_copy_pagedown().
> These changes remove the differences.
> 
> Both nvi and mg exhibit different behaviours from each other, from what
> is implemented now, and what I'm proposing. mg always moves the cursor
> to position 0. nvi retains the position if the new lines length is equal
> or longer, or moves the position to the first printable character
> otherwise.
> 
> If it looks OK I'd like to keep working on it, make
> MODEKEYCOPY_HALFPAGEUP and MODEKEYCOPY_HALFPAGEDOWN use these functions
> as well.
> 
> Index: window-copy.c
> ===
> RCS file: /cvs/src/usr.bin/tmux/window-copy.c,v
> retrieving revision 1.145
> diff -u -p -r1.145 window-copy.c
> --- window-copy.c 5 Feb 2016 10:20:06 -   1.145
> +++ window-copy.c 11 Feb 2016 18:22:59 -
> @@ -26,6 +26,7 @@
>  
>  struct screen *window_copy_init(struct window_pane *);
>  void window_copy_free(struct window_pane *);
> +void window_copy_pagedown(struct window_pane *);
>  void window_copy_resize(struct window_pane *, u_int, u_int);
>  void window_copy_key(struct window_pane *, struct client *, struct session *,
>   key_code, struct mouse_event *);
> @@ -324,15 +325,80 @@ window_copy_pageup(struct window_pane *w
>  {
>   struct window_copy_mode_data*data = wp->modedata;
>   struct screen   *s = >screen;
> - u_intn;
> + u_intn, ox, oy;
> +
> + oy = screen_hsize(data->backing) + data->cy - data->oy;
> + ox = window_copy_find_length(wp, oy);
> +
> + if (s->sel.lineflag == LINE_SEL_LEFT_RIGHT && oy == data->sely)
> + window_copy_other_end(wp);
> +
> + if (data->cx != ox) {
> + data->lastcx = data->cx;
> + data->lastsx = ox;
> + }
> + data->cx = data->lastcx;
>  
>   n = 1;
>   if (screen_size_y(s) > 2)
>   n = screen_size_y(s) - 2;
> +
>   if (data->oy + n > screen_hsize(data->backing))
>   data->oy = screen_hsize(data->backing);
>   else
>   data->oy += n;
> +
> + if (!data->screen.sel.flag || !data->rectflag) {
> + u_int py = screen_hsize(data->backing) + data->cy - data->oy;
> + u_int px = window_copy_find_length(wp, py);
> + if ((data->cx >= data->lastsx && data->cx != px) || data->cx > 
> px)
> + window_copy_cursor_end_of_line(wp);
> + }
> +
> + window_copy_update_selection(wp, 1);
> + window_copy_redraw_screen(wp);
> +}
> +
> +void
> +window_copy_pagedown(struct window_pane *wp)
> +{
> + struct window_copy_mode_data*data = wp->modedata;
> + struct screen   *s = >screen;
> + u_intn, ox, oy;
> +
> + oy = screen_hsize(data->backing) + data->cy - data->oy;
> + ox = window_copy_find_length(wp, oy);
> +
> + if (s->sel.lineflag == LINE_SEL_RIGHT_LEFT && oy == data->sely)
> + window_copy_other_end(wp);
> +
> + if (data->cx != ox) {
> + data->lastcx = data->cx;
> + data->lastsx = ox;
> + }
> + data->cx = data->lastcx;
> +
> + n = 1;
> + if (screen_size_y(s) > 2)
> + n = screen_size_y(s) - 2;
> +
> + if (data->oy < n)
> + data->oy = 0;
> + else
> + data->oy -= n;
> +
> + if (!data->screen.sel.flag || !data->rectflag) {
> + u_int py = screen_hsize(data->backing) + data->cy - data->oy;
> + u_int px = window_copy_find_length(wp, py);
> + if ((data->cx >= data->lastsx && data->cx != px) || data->cx > 
> px)
> + window_copy_cursor_end_of_line(wp);
> + }
> +
> + if (data->scroll_exit && data->oy == 0) {
> + window_pane_reset_mode(wp);
> + return;
> + }
> +
>   window_copy_update_selection(wp, 1);
>   window_copy_redraw_screen(wp);
>  }
> @@ -479,21 +545,8 @@ window_copy_key(struct window_pane *wp, 
>   window_copy_pageup(wp);
>   break;
>   case MODEKEYCOPY_NEXTPAGE:
> - n = 1;
> - if (screen_size_y(s)