menu_context itches

[Oswald Buddenhagen <ossi@…>]

when the cursor is outside of the area menu_context is supposed to keep it in (this happens quite often when exiting from some sub-view), one is often faced with unexpected scrolls. "correct position by one line" seems to be applied blindly regardless of the preceding operation. e.g., moving the cursor in the direction of the "allowed region" should not scroll. anyway, the simplest and in many/all cases correct fix would be just re-adjusting the position when returning to a menu.

[Alain Bench <veronatif@…>]

Hello Oswald,

 On Monday, August 1, 2005 at 5:33:44 PM +0200, Oswald Buddenhagen wrote:

> when the cursor is outside of the area menu_context is supposed to
> keep it in (this happens quite often when exiting from some sub-view),
> one is often faced with unexpected scrolls. "correct position by one
> line" seems to be applied blindly regardless of the preceding
> operation. e.g., moving the cursor in the direction of the "allowed
> region" should not scroll. anyway, the simplest and in many/all cases
> correct fix would be just re-adjusting the position when returning to
> a menu.

    Could you please explain more precisely what happens, and what
should happen? An exact step-by-step example in 1-34c1 screen positions
would be the best, together with what settings are used. What is a
sub-view?


Bye!	Alain.
-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[Oswald Buddenhagen <ossi@…>]

On Sat, Sep 10, 2005 at 08:25:01PM +0200, Alain Bench wrote:
>      Could you please explain more precisely [...]
>
bleh. no way in hell i'm gonna spend more time one this.
anyway, tamo's only_fix patch seems to work reasonably well here - at
least i don't get that "wtf!? ..." feeling wrt that feature any more. :)

[ab]

Hello Oswald,
 On Sat, Sep 10, 2005 at 08:25:01PM +0200, Alain Bench wrote:

    Seemingly fixed by Tamo in CVS.

[Oswald Buddenhagen <ossi@…>]

ok guys, i've got bad news: with this setup, one can still get
nonsensical scrolling:

menu_move_off=yes
menu_context=15
menu_scroll=yes  

the mailbox is pretty full. when opened, the cursor is on the last
message - outside the menu_context region ...

[Alain Bench <veronatif@…>]

On Monday, October 24, 2005 at 12:45:02 +0200, Oswald Buddenhagen wr=
ote:

> one can still get nonsensical scrolling:
>| menu_move_off=3Dyes
>| menu_context=3D15
>| menu_scroll=3Dyes
> the mailbox is pretty full. when opened, the cursor is on the last
> message - outside the menu_context region ...

    Unreproduceble. Mutt stock 1.5.11, 34 lines screen, 795 all read
entries in index: On folder open, I see 19 entries #777-795, cursor o=
n
#795, and well 15 empty lines below cursor. That's nominal with those
settings. Accuracy in reports may be usefull.


Bye!=09Alain.
--=20
He even put in one of the stinkin smilies (you know: yellow, round,
happy... my personal preference is the one with a bleeding bullet hol=
e
in is happy yellow forehead)...
=09Greg K. in =AB=A0Scarface IV -- I Hate Them All=A0=BB

[Oswald Buddenhagen <ossi@…>]

the initial cursor position results from the number of messages and the
terminal size, so doing some changes here might help with reproducing
it.
things are really weird; there seems to be a relation to whether the
index view height is even or odd, and the relation to menu_context
matters as well (if m_c * 2 < i_v_h, the cursor is just centered in
a single jump, otherwise you get the weird motions - e.g., cursor is
below screen middle, you move up - and the index scrolls up as well).

[ab]

ok guys, i've got bad news: with this setup, one can still get
  On Monday, October 24, 2005 at 12:45:02 +0200, Oswald Buddenhagen wr=
 the initial cursor position results from the number of messages and the

    Thanks Oswald: Reopened. This break-dance effect seems
to happen when menu height is even and $menu_context * 2 >=
menu height. Example: Mutt stock 1.5.11, 34 lines menu, 795
entries, menu_scroll=yes, menu_context=17 (or more). On
folder open, I see 17 entries #779-795, cursor on #795.
Typing once <last-entry> gives 778-795c795.
Typing again <last-entry> gives back 779-795c795.
And further <last-entry> toggles between those 2 positions.
The same break-dance happens with other movements like
<next-page>, <next-entry>, 795 <jump>, and so on.

    No problem with odd menu height. No problem with
$menu_context strictly smaller than half height.

[Thomas Roessler <roessler@…>]

On 2005-10-29 13:55:24 +0200, Alain Bench wrote:

>     Thanks Oswald: Reopened. This break-dance effect seems
> to happen when menu height is even and $menu_context * 2 >=3D
> menu height. Example: Mutt stock 1.5.11, 34 lines menu, 795
> entries, menu_scroll=3Dyes, menu_context=3D17 (or more). On
> folder open, I see 17 entries #779-795, cursor on #795.
> Typing once <last-entry> gives 778-795c795.
> Typing again <last-entry> gives back 779-795c795.
> And further <last-entry> toggles between those 2 positions.
> The same break-dance happens with other movements like
> <next-page>, <next-entry>, 795 <jump>, and so on.

The following patch should fix the problem; I'm putting it into CVS:

diff -u -r3.21 menu.c
--- menu.c	17 Sep 2005 20:46:10 -0000	3.21
+++ menu.c	31 Oct 2005 10:29:02 -0000
@@ -387,7 +387,7 @@
   else if (menu->current >=3D menu->top + menu->pagelen - c) /* indicator =
below bottom threshold */
   {
     if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0))
-      menu->top =3D menu->current - menu->pagelen + c + 1;
+      menu->top =3D menu->current - menu->pagelen + c;
     else
       menu->top +=3D (menu->pagelen - c) * ((menu->current - menu->top) / =
(menu->pagelen - c)) - c;
   }


--=20
Thomas Roessler =B7 Personal soap box at <http://log.does-not-exist.org/>.

[roessler]

Fixed in CVS.

[Alain Bench <veronatif@…>]

Hello Thomas!

 On Monday, October 31, 2005 at 11:35:01 +0100, Thomas Roessler wrote:

> On 2005-10-29 13:55:24 +0200, Alain Bench wrote:
>> This break-dance effect seems to happen when menu height is even and
>> $menu_context * 2 >= menu height.
> The following patch should fix the problem; I'm putting it into CVS:

    Your patch fixes the break-dance in said conditions, but
unfortunately creates other problems in other conditions. Especially
cursor altitude is no more stable in odd menu height, in all conditions
$menu_context keeps one less margin below indicator, and scrolling down
with $menu_context=0 overwrites pager status line.

    What about adding:

| + ((MenuContext >= menu->pagelen / 2 && (menu->pagelen & 1) == 0 ) ? 0 : 1)

    Or same but clearer the patch-1.5.11.ab.menu_context_itches_2019.2a
(against straight 1.5.11) I upload to Gnats. With big $menu_context this
should always give stable altitude cursor: Centered on an odd height
menu, and just below center on an even one.


Bye!	Alain.
-- 
Followups to bug reports are public, and should go to both
bug-any@bugs.mutt.org, and the reporter or correspondent.
Do not CC mutt-dev mailing list, the BTS does it already.
It is important to keep the "mutt/nnnn:" tag in subject.

[ab]

On 2005-10-29 13:55:24 +0200, Alain Bench wrote:

    Commited fix had bad effects

[ab]

Upload patch-1.5.11.ab.menu_context_itches_2019.2a to
replace previous fix.

[Thomas Roessler <roessler@…>]

On 2005-10-31 19:55:01 +0100, Alain Bench wrote:

>      Your patch fixes the break-dance in said conditions, but
>  unfortunately creates other problems in other conditions. Especially
>  cursor altitude is no more stable in odd menu height, in all conditions
>  $menu_context keeps one less margin below indicator, and scrolling down
>  with $menu_context=3D0 overwrites pager status line.

ooops.

>      Or same but clearer the
>  patch-1.5.11.ab.menu_context_itches_2019.2a (against straight
>  1.5.11) I upload to Gnats. With big $menu_context this should
>  always give stable altitude cursor: Centered on an odd height
>  menu, and just below center on an even one.

I had missed your patch.

I don't like the explicit special-casing, though...  So, why don't
we change the flow of control a little bit, as in the patch below?

(Let me know if I'm missing another corner case with that version. ;-)

--=20
Thomas Roessler =B7 Personal soap box at <http://log.does-not-exist.org/>.





diff -u -r3.22 menu.c
--- menu.c	31 Oct 2005 10:29:45 -0000	3.22
+++ menu.c	1 Nov 2005 08:29:12 -0000
@@ -384,19 +384,32 @@
       set_option (OPTNEEDREDRAW);
     }
   }
-  else if (menu->current >=3D menu->top + menu->pagelen - c) /* indicator =
below bottom threshold */
-  {
-    if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0))
-      menu->top =3D menu->current - menu->pagelen + c;
-    else
-      menu->top +=3D (menu->pagelen - c) * ((menu->current - menu->top) / =
(menu->pagelen - c)) - c;
-  }
-  else if (menu->current < menu->top + c) /* indicator above top threshold=
 */
+  else=20
   {
-    if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0))
-      menu->top =3D menu->current - c;
-    else
-      menu->top -=3D (menu->pagelen - c) * ((menu->top + menu->pagelen - 1=
 - menu->current) / (menu->pagelen - c)) - c;
+    /*=20
+     * If c =3D menu->pagelen / 2 and menu->pagelen is even, then (obvious=
ly):
+     *=20
+     *   menu->top + menu->pagelen - c =3D=3D menu->top + c
+     *
+     * In that case, having an "else if" below leads to what has become kn=
own as the
+     * indicator break dance effect.  Instead of special-casing, we just f=
orget the=20
+     * "else".
+     */
+   =20
+    if (menu->current < menu->top + c) /* indicator above top threshold */
+    {
+      if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0))
+	menu->top =3D menu->current - c;
+      else
+	menu->top -=3D (menu->pagelen - c) * ((menu->top + menu->pagelen - 1 - me=
nu->current) / (menu->pagelen - c)) - c;
+    }
+    if (menu->current >=3D menu->top + menu->pagelen - c) /* indicator bel=
ow bottom threshold */
+    {
+      if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0))
+	menu->top =3D menu->current - menu->pagelen + c + 1;
+      else
+	menu->top +=3D (menu->pagelen - c) * ((menu->current - menu->top) / (menu=
->pagelen - c)) - c;
+    }
   }
=20
   if (!option (OPTMENUMOVEOFF)) /* make entries stick to bottom */

[Alain Bench <veronatif@…>]

On Tuesday, November 1, 2005 at 9:33:57 +0100, Thomas Roessler wrote:

> On 2005-10-31 19:55:01 +0100, Alain Bench wrote:
>> patch-1.5.11.ab.menu_context_itches_2019.2a
> I had missed your patch.

    Well no: I made it only after your .1 ;-).


> I don't like the explicit special-casing, though... So, why don't we
> change the flow of control a little bit, as in the patch below?

    Ah yes: Nice and elegant! I'm shaking the .3 index big times right
now, seemingly without any remaining misbehaviour falling down.

    Now middle of an even height menu is just above middle (as in my
2b variant). That's probably better (on very small screens) than 2a, if
we assume people mostly scroll down, and lookahead down is preferred.
For consistency we should change <current-middle> to do the same (with
small $menu_context, <current-middle> sets cursor just below middle).


Bye!	Alain.
-- 
Mutt muttrc tip to send mails in best adapted first necessary and sufficient
charset (version for Western Latin-1/Latin-9/CP-850/CP-1252 terminal users):
set send_charset="us-ascii:iso-8859-1:iso-8859-15:windows-1252:utf-8"

[ab]

Hello Thomas!
 On 2005-10-31 19:55:01 +0100, Alain Bench wrote:
  On Tuesday, November 1, 2005 at 9:33:57 +0100, Thomas Roessler wrote:

    Fixed better in CVS: Thanks Thomas.

[ab]

Deleted my previous suboptimal patch.

[Oswald Buddenhagen <ossi@…>]

> Fixed better in CVS
> 
ehm ... no, not really.
now, 1) if the index has fewer lines than menu_context and the number of
lines is odd/even (pick one :), menu_context seems to have no effect at
all (the cursor should be just kept in the middle, like it is in the
other case). 2) when the index is long enough, the cursor can still have
an initial position outside menu_context; this also happens after
resizing the terminal. this leads either to scrolling in the wrong
direction or to a jump recenter, depending on the terminal size (i
think).

[roessler]

> Fixed better in CVS
Oswald accurately observes that the fix in CVS leaves cases
open.

[Thomas Roessler <roessler@…>]

On 2005-11-03 18:45:03 +0100, Oswald Buddenhagen wrote:

>  now, 1) if the index has fewer lines than menu_context and the
>  number of lines is odd/even (pick one :), menu_context seems to
>  have no effect at all (the cursor should be just kept in the
>  middle, like it is in the other case). 2) when the index is long
>  enough, the cursor can still have an initial position outside
>  menu_context; this also happens after resizing the terminal.
>  this leads either to scrolling in the wrong direction or to a
>  jump recenter, depending on the terminal size (i think).

Mhhh...  Basically, if menu_context grows too large, we want
menu_scroll behavior regardless of whether or not that option is
set.

I seem to be unable to reproduce the problems that we've seen with
the attached patch against current CVS, but I might still overlook
some corner case.

--=20
Thomas Roessler =B7 Personal soap box at <http://log.does-not-exist.org/>.



diff -u -r3.23 menu.c
--- menu.c	1 Nov 2005 08:42:00 -0000	3.23
+++ menu.c	17 Nov 2005 17:19:29 -0000
@@ -379,36 +379,27 @@
=20
   if (!option (OPTMENUMOVEOFF) && menu->max <=3D menu->pagelen) /* less en=
tries than lines */
   {
-    if (menu->top !=3D 0) {
+    if (menu->top !=3D 0)=20
+    {
       menu->top =3D 0;
       set_option (OPTNEEDREDRAW);
     }
   }
   else=20
   {
-    /*=20
-     * If c =3D menu->pagelen / 2 and menu->pagelen is even, then (obvious=
ly):
-     *=20
-     *   menu->top + menu->pagelen - c =3D=3D menu->top + c
-     *
-     * In that case, having an "else if" below leads to what has become kn=
own as the
-     * indicator break dance effect.  Instead of special-casing, we just f=
orget the=20
-     * "else".
-     */
-   =20
-    if (menu->current < menu->top + c) /* indicator above top threshold */
+    if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0) || c <=3D MenuCon=
text)
     {
-      if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0))
+      if (menu->current < menu->top + c)
 	menu->top =3D menu->current - c;
-      else
-	menu->top -=3D (menu->pagelen - c) * ((menu->top + menu->pagelen - 1 - me=
nu->current) / (menu->pagelen - c)) - c;
+      if (menu->current >=3D menu->top + menu->pagelen - c)
+	menu->top =3D menu->current - menu->pagelen + c + 1;
     }
-    if (menu->current >=3D menu->top + menu->pagelen - c) /* indicator bel=
ow bottom threshold */
+    else
     {
-      if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0))
-	menu->top =3D menu->current - menu->pagelen + c + 1;
-      else
-	menu->top +=3D (menu->pagelen - c) * ((menu->current - menu->top) / (menu=
->pagelen - c)) - c;
+      if (menu->current < menu->top + c)
+	menu->top -=3D (menu->pagelen - c) * ((menu->top + menu->pagelen - 1 - me=
nu->current) / (menu->pagelen - c)) - c;
+      else if (menu->current >=3D menu->top + menu->pagelen - c)
+	menu->top +=3D (menu->pagelen - c) * ((menu->current - menu->top) / (menu=
->pagelen - c)) - c;=09
     }
   }
=20

[Thomas Roessler <roessler@…>]

On 2005-11-17 18:20:23 +0100, Thomas Roessler wrote:

> I seem to be unable to reproduce the problems that we've seen with
> the attached patch against current CVS, but I might still overlook
> some corner case.

A patch without another bug would truly be too nice, right?

Try ths one instead.
--=20
Thomas Roessler =B7 Personal soap box at <http://log.does-not-exist.org/>.



diff -u -r3.23 menu.c
--- menu.c	1 Nov 2005 08:42:00 -0000	3.23
+++ menu.c	17 Nov 2005 17:22:29 -0000
@@ -379,36 +379,27 @@
=20
   if (!option (OPTMENUMOVEOFF) && menu->max <=3D menu->pagelen) /* less en=
tries than lines */
   {
-    if (menu->top !=3D 0) {
+    if (menu->top !=3D 0)=20
+    {
       menu->top =3D 0;
       set_option (OPTNEEDREDRAW);
     }
   }
   else=20
   {
-    /*=20
-     * If c =3D menu->pagelen / 2 and menu->pagelen is even, then (obvious=
ly):
-     *=20
-     *   menu->top + menu->pagelen - c =3D=3D menu->top + c
-     *
-     * In that case, having an "else if" below leads to what has become kn=
own as the
-     * indicator break dance effect.  Instead of special-casing, we just f=
orget the=20
-     * "else".
-     */
-   =20
-    if (menu->current < menu->top + c) /* indicator above top threshold */
+    if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0) || (c && c <=3D M=
enuContext))
     {
-      if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0))
+      if (menu->current < menu->top + c)
 	menu->top =3D menu->current - c;
-      else
-	menu->top -=3D (menu->pagelen - c) * ((menu->top + menu->pagelen - 1 - me=
nu->current) / (menu->pagelen - c)) - c;
+      if (menu->current >=3D menu->top + menu->pagelen - c)
+	menu->top =3D menu->current - menu->pagelen + c + 1;
     }
-    if (menu->current >=3D menu->top + menu->pagelen - c) /* indicator bel=
ow bottom threshold */
+    else
     {
-      if (option (OPTMENUSCROLL) || (menu->pagelen <=3D 0))
-	menu->top =3D menu->current - menu->pagelen + c + 1;
-      else
-	menu->top +=3D (menu->pagelen - c) * ((menu->current - menu->top) / (menu=
->pagelen - c)) - c;
+      if (menu->current < menu->top + c)
+	menu->top -=3D (menu->pagelen - c) * ((menu->top + menu->pagelen - 1 - me=
nu->current) / (menu->pagelen - c)) - c;
+      else if (menu->current >=3D menu->top + menu->pagelen - c)
+	menu->top +=3D (menu->pagelen - c) * ((menu->current - menu->top) / (menu=
->pagelen - c)) - c;=09
     }
   }

[Oswald Buddenhagen <ossi@…>]

On Thu, Nov 17, 2005 at 06:20:23PM +0100, Thomas Roessler wrote:
> Basically, if menu_context grows too large, we want menu_scroll
> behavior regardless of whether or not that option is set.
> 
yes ... if the total length is < 2 * (menu_context + 1).
but with the code right now in cvs, menu_scroll seems to be always
on with my menu_context setting ...

[Thomas Roessler <roessler@…>]

On 2005-11-17 18:45:03 +0100, Oswald Buddenhagen wrote:

>  yes ... if the total length is < 2 * (menu_context + 1). but
>  with the code right now in cvs, menu_scroll seems to be always
>  on with my menu_context setting ...

What is that setting, and how large is your screen?

-- 
Thomas Roessler			      <roessler@does-not-exist.org>

[Oswald Buddenhagen <ossi@…>]

On Thu, Nov 17, 2005 at 06:48:42PM +0100, Thomas Roessler wrote:
> What is that setting,
> 
still the same: 15.

> and how large is your screen?
>
right at this moment 120x60

[Thomas Roessler <roessler@…>]

On 2005-11-17 18:55:02 +0100, Oswald Buddenhagen wrote:

>  still the same: 15.

Actually, that was irrelevant.  The current CVS has that particular
bug fixed.  

What remains is that last-message can cause a little bit of
break-dance -- because last-message actually ends up *outside* the
allowed area of the screen.  I'm too dense today to see the reason
why that happens, so I'll leave it alone for the moment.

(One ugly fix would be to tell the menu code that it shall not add
more blank lines in the bottom if the last message is already
shown.)

-- 
Thomas Roessler			      <roessler@does-not-exist.org>