* Re: 'read -q -t X' [not found] ` <200907240843.n6O8hvWB022988@news01.csr.com> @ 2009-07-24 11:38 ` Peter Stephenson 2009-07-24 21:39 ` Michael B. Trausch 0 siblings, 1 reply; 2+ messages in thread From: Peter Stephenson @ 2009-07-24 11:38 UTC (permalink / raw) To: Zsh Hackers' List; +Cc: Michael B. Trausch On Fri, 24 Jul 2009 09:43:57 +0100 Peter Stephenson <pws@csr.com> wrote: > This is a bug: -q and -k should be similar enough that the shell does > essentially that internally. Unfortunately, every time anyone has added > a "great new feature" to the read builtin they've simply hacked it in as > a completely separate case without fitting it properly into the existing > code, so the -t code isn't wired up to treat -q as a special case, unlike > -k. It doesn't actually seem to be that hard to fix, in fact. Why wasn't it done this way before? People do seem bent on making my job harder (or not proleptically making it easier, or whatever). I see the original code suggested the default on a timeout should be "y"---however, a timeout would normally return the same status as "n". I've made it return status 2 for a timeout for -q only. There's an argument for a timeout always being status 2, but that's a bigger change for other options (i.e. the ones where the timeout already worked properly :-/). It's not usually such an issue elsewhere since status 1 means the read failed, not that the user explicit said "no". Index: Doc/Zsh/builtins.yo =================================================================== RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v retrieving revision 1.124 diff -u -r1.124 builtins.yo --- Doc/Zsh/builtins.yo 19 Jul 2009 19:08:48 -0000 1.124 +++ Doc/Zsh/builtins.yo 24 Jul 2009 11:33:30 -0000 @@ -1113,9 +1113,10 @@ Read only one character from the terminal and set var(name) to `tt(y)' if this character was `tt(y)' or `tt(Y)' and to `tt(n)' otherwise. With this flag set the return status is zero only if the character was -`tt(y)' or `tt(Y)'. Note that this always reads from the terminal, even -if used with the tt(-p) or tt(-u) or tt(-z) flags or with redirected input. -This option may also be used within zle widgets. +`tt(y)' or `tt(Y)'. This option may be used with a timeout; if +the read times out, or encounters end of file, status 2 is returned. +Input is read from the terminal unless one of tt(-u) +or tt(-p) is present. This option may also be used within zle widgets. ) item(tt(-k) [ var(num) ])( Read only one (or var(num)) characters. All are assigned to the first Index: Src/builtin.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v retrieving revision 1.233 diff -u -r1.233 builtin.c --- Src/builtin.c 21 Jul 2009 09:35:26 -0000 1.233 +++ Src/builtin.c 24 Jul 2009 11:33:30 -0000 @@ -5040,8 +5040,8 @@ if(OPT_ISSET(ops,'l') || OPT_ISSET(ops,'c')) return compctlreadptr(name, args, ops, reply); - if ((OPT_ISSET(ops,'k') && !OPT_ISSET(ops,'u') && - !OPT_ISSET(ops,'p')) || OPT_ISSET(ops,'q')) { + if ((OPT_ISSET(ops,'k') || OPT_ISSET(ops,'q')) && + !OPT_ISSET(ops,'u') && !OPT_ISSET(ops,'p')) { if (!zleactive) { if (SHTTY == -1) { /* need to open /dev/tty specially */ @@ -5065,7 +5065,7 @@ gettyinfo(&shttyinfo); /* attach to the tty */ attachtty(mypgrp); - if (!isem && OPT_ISSET(ops,'k')) + if (!isem) setcbreak(); readfd = SHTTY; } @@ -5184,8 +5184,9 @@ #endif } else { if (readfd == -1 || - !read_poll(readfd, &readchar, keys && !zleactive, timeout)) { - if (OPT_ISSET(ops,'k') && !zleactive && !isem) + !read_poll(readfd, &readchar, keys && !zleactive, + timeout)) { + if (keys && !zleactive && !isem) settyinfo(&shttyinfo); else if (resettty && SHTTY != -1) settyinfo(&saveti); @@ -5194,7 +5195,7 @@ shout = oshout; SHTTY = -1; } - return 1; + return OPT_ISSET(ops,'q') ? 2 : 1; } } } @@ -5203,11 +5204,18 @@ memset(&mbs, 0, sizeof(mbs)); #endif - /* option -k means read only a given number of characters (default 1) */ - if (OPT_ISSET(ops,'k')) { + /* + * option -k means read only a given number of characters (default 1) + * option -q means get one character, and interpret it as a Y or N + */ + if (OPT_ISSET(ops,'k') || OPT_ISSET(ops,'q')) { int eof = 0; /* allocate buffer space for result */ +#ifdef MULTIBYTE_SUPPORT + bptr = buf = (char *)zalloc(nchars*MB_CUR_MAX+1); +#else bptr = buf = (char *)zalloc(nchars+1); +#endif do { if (izle) { @@ -5295,6 +5303,17 @@ } } + if (OPT_ISSET(ops,'q')) + { + /* + * Keep eof as status but status is now whether we read + * 'y' or 'Y'. If we timed out, status is 2. + */ + if (eof) + eof = 2; + else + eof = (bptr - buf != 1 || (buf[0] != 'y' && buf[0] != 'Y')); + } if (OPT_ISSET(ops,'e') || OPT_ISSET(ops,'E')) fwrite(buf, bptr - buf, 1, stdout); if (!OPT_ISSET(ops,'e')) @@ -5306,59 +5325,6 @@ return eof; } - /* option -q means get one character, and interpret it as a Y or N */ - if (OPT_ISSET(ops,'q')) { - char readbuf[2]; - - /* set up the buffer */ - readbuf[1] = '\0'; - - /* get, and store, reply */ - if (izle) { -#ifdef MULTIBYTE_SUPPORT - int key; - char c; - - for (;;) { - zleentry(ZLE_CMD_GET_KEY, izle_timeout, NULL, &key); - if (key < 0) - break; - c = (char)key; - /* - * If multibyte, it can't be y, so we don't care - * what key gets set to; just read to end of character. - */ - if (!isset(MULTIBYTE) || - mbrlen(&c, 1, &mbs) != MB_INCOMPLETE) - break; - } -#else - int key; - zleentry(ZLE_CMD_GET_KEY, izle_timeout, NULL, &key); -#endif - - readbuf[0] = (key == 'y' ? 'y' : 'n'); - } else { - readbuf[0] = ((char)getquery(NULL, 0)) == 'y' ? 'y' : 'n'; - - /* dispose of result appropriately, etc. */ - if (haso) { - fclose(shout); /* close(SHTTY) */ - shout = oshout; - SHTTY = -1; - } - } - - if (OPT_ISSET(ops,'e') || OPT_ISSET(ops,'E')) - printf("%s\n", readbuf); - if (!OPT_ISSET(ops,'e')) - setsparam(reply, ztrdup(readbuf)); - - if (resettty && SHTTY != -1) - settyinfo(&saveti); - return readbuf[0] == 'n'; - } - /* All possible special types of input have been exhausted. Take one line, and assign words to the parameters until they run out. Leftover words go onto the last parameter. If an array is specified, all the words become Index: Test/B04read.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/B04read.ztst,v retrieving revision 1.3 diff -u -r1.3 B04read.ztst --- Test/B04read.ztst 25 Apr 2005 10:41:26 -0000 1.3 +++ Test/B04read.ztst 24 Jul 2009 11:33:30 -0000 @@ -24,6 +24,18 @@ 0:read specified number of chars >foo + for char in y Y n N X $'\n'; do + read -q -u0 <<<$char + print $? + done +0:read yes or no, default no +>0 +>0 +>1 +>1 +>1 +>1 + read -d: <<<foo:bar print $REPLY 0:read up to delimiter Index: Test/D07multibyte.ztst =================================================================== RCS file: /cvsroot/zsh/zsh/Test/D07multibyte.ztst,v retrieving revision 1.33 diff -u -r1.33 D07multibyte.ztst --- Test/D07multibyte.ztst 8 May 2009 14:30:36 -0000 1.33 +++ Test/D07multibyte.ztst 24 Jul 2009 11:33:30 -0000 @@ -232,6 +232,12 @@ <«»ignored >«» + read -q -u0 mb + print $? +0:multibyte character makes read -q return false +<« +>1 + # See if the system grokks first-century Greek... ioh="Ἐν ἀρχῇ ἦν ὁ λόγος, καὶ ὁ λόγος ἦν πρὸς τὸν θεόν, καὶ θεὸς ἦν ὁ λόγος." for (( i = 1; i <= ${#ioh}; i++ )); do -- Peter Stephenson <pws@csr.com> Software Engineer Tel: +44 (0)1223 692070 Cambridge Silicon Radio Limited Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK 'member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom' ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: 'read -q -t X' 2009-07-24 11:38 ` 'read -q -t X' Peter Stephenson @ 2009-07-24 21:39 ` Michael B. Trausch 0 siblings, 0 replies; 2+ messages in thread From: Michael B. Trausch @ 2009-07-24 21:39 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh Hackers' List [-- Attachment #1: Type: TEXT/PLAIN, Size: 8004 bytes --] On Fri, 24 Jul 2009, Peter Stephenson wrote: > On Fri, 24 Jul 2009 09:43:57 +0100 > Peter Stephenson <pws@csr.com> wrote: >> This is a bug: -q and -k should be similar enough that the shell does >> essentially that internally. Unfortunately, every time anyone has added >> a "great new feature" to the read builtin they've simply hacked it in as >> a completely separate case without fitting it properly into the existing >> code, so the -t code isn't wired up to treat -q as a special case, unlike >> -k. > > It doesn't actually seem to be that hard to fix, in fact. Why wasn't it > done this way before? People do seem bent on making my job harder (or not > proleptically making it easier, or whatever). > > I see the original code suggested the default on a timeout should be > "y"---however, a timeout would normally return the same status as "n". > I've made it return status 2 for a timeout for -q only. There's an > argument for a timeout always being status 2, but that's a bigger change > for other options (i.e. the ones where the timeout already worked properly > :-/). It's not usually such an issue elsewhere since status 1 means > the read failed, not that the user explicit said "no". Awesome, thanks for the patch. I will apply it when I get the chance. Much appreciated! --- Mike > > Index: Doc/Zsh/builtins.yo > =================================================================== > RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v > retrieving revision 1.124 > diff -u -r1.124 builtins.yo > --- Doc/Zsh/builtins.yo 19 Jul 2009 19:08:48 -0000 1.124 > +++ Doc/Zsh/builtins.yo 24 Jul 2009 11:33:30 -0000 > @@ -1113,9 +1113,10 @@ > Read only one character from the terminal and set var(name) to > `tt(y)' if this character was `tt(y)' or `tt(Y)' and to `tt(n)' otherwise. > With this flag set the return status is zero only if the character was > -`tt(y)' or `tt(Y)'. Note that this always reads from the terminal, even > -if used with the tt(-p) or tt(-u) or tt(-z) flags or with redirected input. > -This option may also be used within zle widgets. > +`tt(y)' or `tt(Y)'. This option may be used with a timeout; if > +the read times out, or encounters end of file, status 2 is returned. > +Input is read from the terminal unless one of tt(-u) > +or tt(-p) is present. This option may also be used within zle widgets. > ) > item(tt(-k) [ var(num) ])( > Read only one (or var(num)) characters. All are assigned to the first > Index: Src/builtin.c > =================================================================== > RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v > retrieving revision 1.233 > diff -u -r1.233 builtin.c > --- Src/builtin.c 21 Jul 2009 09:35:26 -0000 1.233 > +++ Src/builtin.c 24 Jul 2009 11:33:30 -0000 > @@ -5040,8 +5040,8 @@ > if(OPT_ISSET(ops,'l') || OPT_ISSET(ops,'c')) > return compctlreadptr(name, args, ops, reply); > > - if ((OPT_ISSET(ops,'k') && !OPT_ISSET(ops,'u') && > - !OPT_ISSET(ops,'p')) || OPT_ISSET(ops,'q')) { > + if ((OPT_ISSET(ops,'k') || OPT_ISSET(ops,'q')) && > + !OPT_ISSET(ops,'u') && !OPT_ISSET(ops,'p')) { > if (!zleactive) { > if (SHTTY == -1) { > /* need to open /dev/tty specially */ > @@ -5065,7 +5065,7 @@ > gettyinfo(&shttyinfo); > /* attach to the tty */ > attachtty(mypgrp); > - if (!isem && OPT_ISSET(ops,'k')) > + if (!isem) > setcbreak(); > readfd = SHTTY; > } > @@ -5184,8 +5184,9 @@ > #endif > } else { > if (readfd == -1 || > - !read_poll(readfd, &readchar, keys && !zleactive, timeout)) { > - if (OPT_ISSET(ops,'k') && !zleactive && !isem) > + !read_poll(readfd, &readchar, keys && !zleactive, > + timeout)) { > + if (keys && !zleactive && !isem) > settyinfo(&shttyinfo); > else if (resettty && SHTTY != -1) > settyinfo(&saveti); > @@ -5194,7 +5195,7 @@ > shout = oshout; > SHTTY = -1; > } > - return 1; > + return OPT_ISSET(ops,'q') ? 2 : 1; > } > } > } > @@ -5203,11 +5204,18 @@ > memset(&mbs, 0, sizeof(mbs)); > #endif > > - /* option -k means read only a given number of characters (default 1) */ > - if (OPT_ISSET(ops,'k')) { > + /* > + * option -k means read only a given number of characters (default 1) > + * option -q means get one character, and interpret it as a Y or N > + */ > + if (OPT_ISSET(ops,'k') || OPT_ISSET(ops,'q')) { > int eof = 0; > /* allocate buffer space for result */ > +#ifdef MULTIBYTE_SUPPORT > + bptr = buf = (char *)zalloc(nchars*MB_CUR_MAX+1); > +#else > bptr = buf = (char *)zalloc(nchars+1); > +#endif > > do { > if (izle) { > @@ -5295,6 +5303,17 @@ > } > } > > + if (OPT_ISSET(ops,'q')) > + { > + /* > + * Keep eof as status but status is now whether we read > + * 'y' or 'Y'. If we timed out, status is 2. > + */ > + if (eof) > + eof = 2; > + else > + eof = (bptr - buf != 1 || (buf[0] != 'y' && buf[0] != 'Y')); > + } > if (OPT_ISSET(ops,'e') || OPT_ISSET(ops,'E')) > fwrite(buf, bptr - buf, 1, stdout); > if (!OPT_ISSET(ops,'e')) > @@ -5306,59 +5325,6 @@ > return eof; > } > > - /* option -q means get one character, and interpret it as a Y or N */ > - if (OPT_ISSET(ops,'q')) { > - char readbuf[2]; > - > - /* set up the buffer */ > - readbuf[1] = '\0'; > - > - /* get, and store, reply */ > - if (izle) { > -#ifdef MULTIBYTE_SUPPORT > - int key; > - char c; > - > - for (;;) { > - zleentry(ZLE_CMD_GET_KEY, izle_timeout, NULL, &key); > - if (key < 0) > - break; > - c = (char)key; > - /* > - * If multibyte, it can't be y, so we don't care > - * what key gets set to; just read to end of character. > - */ > - if (!isset(MULTIBYTE) || > - mbrlen(&c, 1, &mbs) != MB_INCOMPLETE) > - break; > - } > -#else > - int key; > - zleentry(ZLE_CMD_GET_KEY, izle_timeout, NULL, &key); > -#endif > - > - readbuf[0] = (key == 'y' ? 'y' : 'n'); > - } else { > - readbuf[0] = ((char)getquery(NULL, 0)) == 'y' ? 'y' : 'n'; > - > - /* dispose of result appropriately, etc. */ > - if (haso) { > - fclose(shout); /* close(SHTTY) */ > - shout = oshout; > - SHTTY = -1; > - } > - } > - > - if (OPT_ISSET(ops,'e') || OPT_ISSET(ops,'E')) > - printf("%s\n", readbuf); > - if (!OPT_ISSET(ops,'e')) > - setsparam(reply, ztrdup(readbuf)); > - > - if (resettty && SHTTY != -1) > - settyinfo(&saveti); > - return readbuf[0] == 'n'; > - } > - > /* All possible special types of input have been exhausted. Take one line, > and assign words to the parameters until they run out. Leftover words go > onto the last parameter. If an array is specified, all the words become > Index: Test/B04read.ztst > =================================================================== > RCS file: /cvsroot/zsh/zsh/Test/B04read.ztst,v > retrieving revision 1.3 > diff -u -r1.3 B04read.ztst > --- Test/B04read.ztst 25 Apr 2005 10:41:26 -0000 1.3 > +++ Test/B04read.ztst 24 Jul 2009 11:33:30 -0000 > @@ -24,6 +24,18 @@ > 0:read specified number of chars > >foo > > + for char in y Y n N X $'\n'; do > + read -q -u0 <<<$char > + print $? > + done > +0:read yes or no, default no > +>0 > +>0 > +>1 > +>1 > +>1 > +>1 > + > read -d: <<<foo:bar > print $REPLY > 0:read up to delimiter > Index: Test/D07multibyte.ztst > =================================================================== > RCS file: /cvsroot/zsh/zsh/Test/D07multibyte.ztst,v > retrieving revision 1.33 > diff -u -r1.33 D07multibyte.ztst > --- Test/D07multibyte.ztst 8 May 2009 14:30:36 -0000 1.33 > +++ Test/D07multibyte.ztst 24 Jul 2009 11:33:30 -0000 > @@ -232,6 +232,12 @@ > <«»ignored > >«» > > + read -q -u0 mb > + print $? > +0:multibyte character makes read -q return false > +<« > +>1 > + > # See if the system grokks first-century Greek... > ioh="Ἐν ἀρχῇ ἦν ὁ λόγος, καὶ ὁ λόγος ἦν πρὸς τὸν θεόν, καὶ θεὸς ἦν ὁ λόγος." > for (( i = 1; i <= ${#ioh}; i++ )); do > > > ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2009-07-24 21:47 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <alpine.DEB.2.00.0907231851500.2551@zest.trausch.us>
[not found] ` <200907240843.n6O8hvWB022988@news01.csr.com>
2009-07-24 11:38 ` 'read -q -t X' Peter Stephenson
2009-07-24 21:39 ` Michael B. Trausch
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).