[9front] vt: fix scrolling over ssh, add mouse scrolling

[9front] vt: fix scrolling over ssh, add mouse scrolling

[ori]

Testing requested.

We were setting 'resize_flag = 1' in backup(), which was
causing resized() to get called, which was causing us to
call exportsize(), which was eventually singalling sshd,
which forwarded the signal over to the process on the
other end of the ssh pipe.

This was causing a SIGINT to get sent, which was screwing
up the scrolling.

I'm not sure why we were setting resize_flag=1 in the code:
we were always doing something like it since the code first
got into 9front -- eresized(0) initially, moving to setting
the flag when moving to libevent.

Things seem to work without it, so removing it doesn't seem
to have broken anything.

--- a/sys/src/cmd/vt/cons.h
+++ b/sys/src/cmd/vt/cons.h
@@ -54,7 +54,7 @@
 extern int	number(Rune *, int *);
 extern void	shift(int,int,int,int);
 extern void	scroll(int,int,int,int);
-extern int	backup(int);
+extern void	backup(int);
 extern void	sendnchars(int, char *);
 extern Point	pt(int, int);
 extern Point	pos(Point);
--- a/sys/src/cmd/vt/main.c
+++ b/sys/src/cmd/vt/main.c
@@ -133,9 +133,7 @@
 
 Rune	kbdchar;
 
-#define	button1()	((mc->buttons & 07)==1)
-#define	button2()	((mc->buttons & 07)==2)
-#define	button3()	((mc->buttons & 07)==4)
+#define	button(num)	(mc->buttons == (1<<((num)-1)))
 
 Mousectl	*mc;
 Keyboardctl	*kc;
@@ -210,6 +208,7 @@
 {
 	if(hostpid > 0)
 		postnote(PNGROUP, hostpid, "interrupt");
+
 }
 
 void
@@ -834,10 +833,14 @@
 		flushimage(display, 1);
 	switch(alt(a)){
 	case AMOUSE:
-		if(button1() || chording)
+		if(button(1) || chording)
 			selecting();
-		else if(button2() || button3())
+		else if(button(2) || button(3))
 			readmenu();
+		else if(button(4))
+			backup(backc+1);
+		else if(button(5))
+			backup(backc-1);
 		else if(resize_flag == 0)
 			goto Next;
 		break;
@@ -1128,7 +1131,7 @@
 			select(p, q, mode);
 			drawscreen();
 			readmouse(mc);
-		} while(button1());
+		} while(button(1));
 	}
 	if(mc->buttons != chording){
 		switch(mc->buttons & 0x7){
@@ -1167,7 +1170,7 @@
 	Point p;
 
 	p = pos(mc->xy);
-	if(button3()) {
+	if(button(3)) {
 		menu3.item[1] = ttystate[cs->raw].crnl ? "cr" : "crnl";
 		menu3.item[2] = ttystate[cs->raw].nlcr ? "nl" : "nlcr";
 		menu3.item[3] = cs->raw ? "cooked" : "raw";
@@ -1201,13 +1204,11 @@
 
 	switch(menuhit(2, mc, &menu2, nil)) {
 	case Mbackup:		/* back up */
-		if(backup(backc+1))
-			backc++;
+		backup(backc+1);
 		return;
 
 	case Mforward:		/* move forward */
-		if(backc > 0)
-			backup(--backc);
+		backup(backc-1);
 		return;
 
 	case Mreset:		/* reset */
@@ -1237,7 +1238,7 @@
 	}
 }
 
-int
+void
 backup(int count)
 {
 	Rune *cp;
@@ -1244,8 +1245,8 @@
 	int left, n;
 
 	unselect();
-
-	resize_flag = 1;
+	if(count < 0)
+		return;
 	if(count == 0 && !pagemode) {
 		n = ymax;
 		nbacklines = HISTSIZ;	/* make sure we scroll to the very end */
@@ -1270,7 +1271,8 @@
 	if(cp >= &hist[HISTSIZ])
 		cp = hist;
 	backp = cp;
-	return left;
+	if(left)
+		backc = count;
 }
 
 Point

Re: [9front] vt: fix scrolling over ssh, add mouse scrolling

[igor]

Quoth ori@eigenstate.org:
> Testing requested.

Tested with connections to various alien systems including
Darwin, WSL Windows, various Linux distros. The patch doesn't
break anything for my use cases.

Usually I connect like this:

  term% vt -bx ssh tcp!not.a.9front.system.com!2222

However, I have to admit I didn't fully get the issue you
are describing and therefore couldn't repro it. Hence can
only report that the patch doesn't break anything for me ☺

Cheers,
Igor

Re: [9front] vt: fix scrolling over ssh, add mouse scrolling

[ori]

Quoth igor@9lab.org:
> 
> However, I have to admit I didn't fully get the issue you
> are describing

Before this patch, you'd scroll via the 'backup' and 'forward'
entries in the menu. They'd generate interrupts ('^C') on the
remote side when you'd ssh'ed in.

This change fixes that problem, and while I'm there, adds
mouse based scrolling.

Re: [9front] vt: fix scrolling over ssh, add mouse scrolling

[igor]

Quoth ori@eigenstate.org:
> Quoth igor@9lab.org:
> > 
> > However, I have to admit I didn't fully get the issue you
> > are describing
> 
> Before this patch, you'd scroll via the 'backup' and 'forward'
> entries in the menu. They'd generate interrupts ('^C') on the
> remote side when you'd ssh'ed in.
> 
> This change fixes that problem, and while I'm there, adds
> mouse based scrolling.

Thanks for the explanation.

For mouse based scrolling a mouse with scroll wheel is required.
That was the misconception I had (using a three button mouse)
as I did not spot any code that would make the scroll
bar appear so one could emulate scrolling by clicking at various
places within it...anyway, sorry for the confusion.

Retested with scroll wheel mouse. Works as advertised :D

Re: [9front] vt: fix scrolling over ssh, add mouse scrolling

[unobe]

Quoth ori@eigenstate.org:
> Testing requested.
> 
> We were setting 'resize_flag = 1' in backup(), which was
> causing resized() to get called, which was causing us to
> call exportsize(), which was eventually singalling sshd,
> which forwarded the signal over to the process on the
> other end of the ssh pipe.
> 
> This was causing a SIGINT to get sent, which was screwing
> up the scrolling.

Reviewing the logic of what resize_flag does, your patch makes sense:
when can you ever resize the window when scrolling/paging back or
forward?  I don't see how that can be done simultaneously, but maybe
there's something else afoot.

I want to make sure I understand what you're experiencing so I can
test it properly.  Here's what I've pieced togeter without running
anything:

1.  You start vt(1)

2.  You ssh(1) into a foreign host.  Since vt(1) sets the TERM
environment variable, raw mode is on in ssh(1), which sets the vt(1)
console winch to 1 @ /sys/src/cmd/vt/fs.c:/winchon due to the write @
/sys/src/cmd/ssh.c:/^rawon .

3.  You start a long-running process that spews a lot of text.

4.  You then use 'backup'.  That signals to the foreign host to set
the environment variables, which is performed.  No problem there.

5.  However, at the end of exportsize() an interrupt is sent because
ssh(1) had set winch to 1:
	if(cs->winch)
		send_interrupt();
   Thus the problem?

Related to this, I see resize_flag is set to 1 in only one other
scenario, and that deals with the page menu item and when blocked is
true.  Blocked is only set to 1 in /sys/src/cmd/vt/main.c:^newline
from what I see.  That's only done after newline() is called in
/sys/src/cmd/vt/vt.c and only when:

a) pagemode is true; and

b) the number of lines goes beyond the screen size max.

So perhaps the resize_flag set to 1 for 'backup' is to handle a
special case that we're not aware of currently?  Maybe ssh.c is being
too aggressive in setting winchon? Or maybe your fix is the best.