Ping? Also, any thoughts on the editline.3 diff[1] sent later in this thread?
[1] http://marc.info/?l=openbsd-tech&m=145150775627284&w=2 Ingo Schwarze wrote: > Hi Christian & Michael, > > Michael McConville wrote on Thu, Dec 24, 2015 at 04:19:03PM -0500: > > Christian Heckendorf wrote: > > >> A couple of somewhat recent changes in NetBSD's libedit permit > >> el_gets(3) to accept multibyte characters if the locale supports > >> it. > > Ugh. The amount of indirection in that code is disturbing, and the > amount of contortion is disgusting. Such stuff is highly error > prone, in particular since the manual is way below OpenBSD standards > (functions mentioned in the SYNOPSIS but not actually documented, > vague statements, confusion regarding bytes versus characters, ...). > Besides, it's not completely clear which parts of the interface are > public and which are internal to the library. > > In the vicinity of this particular diff: The IGNORE_EXTCHARS flag > appears to be private to the library, the users seems to have no > way to change it. Otherwise, the existing code in el_getc(3) > would already be broken because it clears the flag on exit even > if it was set on entry. But as an internal flag, it's completely > pointless. If CHARSET_IS_UTF8 is set, the present diff makes > sure it is never set. If CHARSET_IS_UTF8 is not set, it has no > effect because the only place where it is used also checks "bytes > 1", > which cannot happen in the C locale. > > But if we want to stay in sync with upstream, freely borrowing Bob's > whale flensing knife may not be the best idea. > > Also note that el_gets(3) is documented to return the number of > characters read, but actually, callers assume it returns the > number of *bytes* read, so what your diff is doing makes sense. > > Michael, as you already looked at NetBSD, is there a documentation > update to go with this diff? > > >> A notable user of this function is sftp(1) which will allow users to > >> input multibyte characters, in filenames for example, once the diff > >> is applied. > > I inspected your diff and found it to be both correct and in line > with the existing (horrible) style. In particular, it is very > similar to the code already used in el_getc(3). I also tested > it with ftp(1) - adding setlocale(LC_CTYPE, "") to that program, > though it's quite unclear where exactly that call should go, so > no diff for that just yet. > > But i would like to commit this eln.c diff, if i get any OK for it. > > > I remember someone recently mentioning that libedit is due for an > > update. Maybe we should try to include this in a full sync. > > Sure. But let's not deny focussed, non-intrusive diffs that make > specific, well-defined things better just because there are rumours > about grand plans. Besides, cherry-picking from upstream hardly > complicates later merges. > > OK? > > Yours, > Ingo > > > >> Index: eln.c > >> =================================================================== > >> RCS file: /cvs/src/lib/libedit/eln.c,v > >> retrieving revision 1.4 > >> diff -u -p -r1.4 eln.c > >> --- eln.c 20 May 2014 11:59:03 -0000 1.4 > >> +++ eln.c 24 Dec 2015 19:34:09 -0000 > >> @@ -74,9 +74,18 @@ el_gets(EditLine *el, int *nread) > >> { > >> const wchar_t *tmp; > >> > >> - el->el_flags |= IGNORE_EXTCHARS; > >> + if (!(el->el_flags & CHARSET_IS_UTF8)) > >> + el->el_flags |= IGNORE_EXTCHARS; > >> tmp = el_wgets(el, nread); > >> - el->el_flags &= ~IGNORE_EXTCHARS; > >> + if (tmp != NULL) { > >> + size_t nwread = 0; > >> + int i; > >> + for (i = 0; i < *nread; i++) > >> + nwread += ct_enc_width(tmp[i]); > >> + *nread = (int)nwread; > >> + } > >> + if (!(el->el_flags & CHARSET_IS_UTF8)) > >> + el->el_flags &= ~IGNORE_EXTCHARS; > >> return ct_encode_string(tmp, &el->el_lgcyconv); > >> } > >> > >> >
