[Toybox] top: pressing SHIFT-RIGHT and RIGHT confuses [sort key column]

[Mark Hansen]

Thanks Rob. Appreciate the fix. I agree; advancing by word seems
clearer, at the cost of slightly more bounds checking, but I quickly
reviewed the new bounds checking and it seems right to me too.

For the archives, the fixes are in:
- https://codeberg.org/landley/toybox/commit/5fbbae3e292c29d268a417b9776b74f23d574fdf
- https://codeberg.org/landley/toybox/commit/266ac9daaa8d5da187af3a48951d0670e2b7b5b8

Thanks again for all your work on toybox, have a great day

Mark

On Thu, 30 Apr 2026 at 08:57, Rob Landley <rob at landley.net> wrote:
>
> On 4/27/26 20:30, Mark Hansen wrote:
> > Hi, first up, thanks for toybox. I'm always debugging Android apps
> > using Toybox builtins and I appreciate all your work.
> >
> > I just had some confusing behaviour in toybox top in Android.
> >
> > Background: top lets you press left/right to move the list, and
> > shift-left/shift-right to change sort column:
>
> Yup. As documented in the --help output.
>
> https://landley.net/toybox/help.html#top
>
> > ```
> >      Cursor UP/DOWN or LEFT/RIGHT to move list, SHIFT LEFT/RIGHT to change sort,
> >      space to force update, R to reverse sort, Q to exit.
> > ```
> >
> > And the intention from the source code looks to be to highlight the
> > sort-order column by putting the column name in square brackets:
>
> Left/right lets you scroll the display to see long command lines, shift
> left/right changes which column you're sorting by, yes.
>
> Judging by the title of the email... Ah, normal cursor right is _also_
> incrementing which field is [bracketed] in the display (although it
> doesn't change which field it's actually sorting by).
>
> Ah, I see the problem: show_pos() (and for that matter get_headers()) is
> skipping TT.scroll many fields, so the display logic needs to subtract
> TT.scroll from TT.sortpos.
>
> Sigh, that was/is stuff looks like sleep deprivation... Ok, fixed up.
>
> > I expect the problem is that here in `get_headers`, we print into a
> > string buffer starting at header position `TT.scroll`:
>
> Yup, that was it.
>
> > enh at google.com said: "i tried your suggested fix, and it seems to work":
> > ```
> > diff --git a/toys/posix/ps.c b/toys/posix/ps.c
> > index 86cd2197..8d53e26a 100644
> > --- a/toys/posix/ps.c
> > +++ b/toys/posix/ps.c
> > @@ -1726,7 +1726,7 @@ static void top_common(
> >           lines = header_line(lines, 0);
> >           // print line of header labels for currently displayed fields
> >           get_headers(TT.fields, pos = toybuf, sizeof(toybuf));
> > -        for (i = 0, is = ' '; *pos; pos++) {
> > +        for (i = TT.scroll, is = ' '; *pos; pos++) {
> >             was = is;
> >             is = *pos;
> >             if (isspace(was) && !isspace(is) && i++==TT.sortpos && pos!=toybuf)
> > ```
>
> That would have worked too. (I fixed it before reading down that far.)
>
> I rewrote the loop to advance by word because all it's doing is
> bracketing simple space separated words with no quoting, and declaring
> was/is variables for that says to me that I was VERY sleep deprived when
> I wrote that part...
>
> Actually, your trick makes it slightly smaller, I'll do that too.
>
> Thanks,
>
> Rob