[Toybox] fold implementation

Rob Landley rob at landley.net
Tue Apr 8 04:15:59 PDT 2014


On 04/04/14 03:23, David Seikel wrote:
> Seems like a good time to remind you about the "generic editor code"
> thread that has my first instalment that's been sitting there for
> months with no comment.  Well, one private comment from someone that's
> itching to use the more advanced editor, but I'm waiting for you to
> deal with the bite sized chunks you asked me for.

I've actually looked at it a couple times, the problem is I'm not
entirely sure what to do with it. But as long as I'm on an airplane
anyway, let's take another look:

In handlekeys: half of this is a big table mapping input sequences to
names. I'm not sure I'm happy with the strings as the output, but enums
wouldn't be an improvement so let's skip that for now.

The first half of this table is matching 00 through 1F in order, I'm not
sure the first entry of each pair is necessary? The rest of the table is
matching escape sequences starting with either ESC (27) or 0x9b (which
it says is the 8-bit encoding of ESC? What?) A bit redundant, but let's
worry about cleaning it up later...

It defines static variables outside of a GLOBALS() block.

handlekeys():
It says FDZERO() each time is "more portable"... to what?

We have xread(), might as well use it.

In the "Ran out of buffer" case, you're assuming the last byte is zero,
but where do you actually set that?

This function calls two callback functions, when it could as easily fill
out a struct and/or return a status? Haven't quite wrapped my head
around why you did it that way yet, but still reading.

RE: sizeof(blah)/sizeof(*blah), we have ARRAY_LEN(), might as well use it.

We care that buffer[] is null terminated _and_ we have buffindex?

Why have pendingESC and csi, rather set "pending" to 27 or 0x9b?
(pendingESC is outside the loop, csi is in the loop...)

When I rewrote the busybox vi escape parsing to not freak out over a
serial terminal, a bare escape wasn't special any more than any
unterminated sequence is special. If we have enough of a delay in the
middle of a sequence, degrade it to literals. Otherwise you hang who
knows how long in an indeterminite state halfway through input that
hasn't come. (Locally generated sequences come in as a single read(),
and sending them via ssh puts them in the same network packet, they only
really get broken up by serial hardware and that should have very hard
timeouts. 300 baud sent a character every 1/7th of a second.)

At the start of if (csi) there's a large comment that basically says
"go read the spec and understand this yourself".

Mouse report? Um, there can always be escape sequences we don't
understand. The proper thing to do is probably pass 'em through verbatim.

In dumbsh: the tty reset code is conceptually similar to the sane_tty()
stuff in init, I'd like to factor it out into a generic terminal reset
thing that reset() could also use, and maybe stty. But there are a
gazillion fiddly flags and I don't know enough about this area yet to
make it simple. I suspect I need to write an stty implementation first,
then retrofit calls to that code into init and such.

Have to put the laptop away, we're descending into Chicago.

Rob

 1396955759.0


More information about the Toybox mailing list