Hm, i was a bit late yesterday and am subscribed to no list around here which seemed to be sufficient, so i've subscribed to bugs@ and confirmed and send my mail. It seems this list is usually driven by a tracker.. So, then. If /bin/sh is the login shell, starting it won't allow input of german umlauts in ISO8859-15 locale with fonts plus well set up. Issue a set with arguments ($set +o, $set -o), and it'll work as expected. Fix: So it turns out the problem is the histedit() function. If it isn't called from within optschanged() then everything is fine. Now i've spend some hours trying to adjust that, but i've no idea on sh(1) and libedit(3) internals, so i still don't understand the problem, though i've found a getaround. The problem occurs when 'el' must be initialized. If that actually happens (upon sh(1) startup), then it'll change handling of the high bit (or say, i'm just fiddling around with german umlauts here). I've split up histedit() in individual parts, i've used sleeps and recursive calls and whatever, but it didn't help. The only solution i've found is to call histedit() from within main() again (or outsource the pure initialization code to histedit_init(), but anyway call it once from within main()). I've no idea if that may have side-impacts, but applying the patch fixes the issue for me. This patch applies to the master branch of git://git.freebsd.org/freebsd. So sorry for the mail yesterday. And how can i Cc: jilles@ and pfg@ from within this tracker now?? --steffen Patch attached with submission follows: How-To-Repeat: Start a shell.
An update: i've compiled the shell again, linked against libedit as of today (master branch, 493ac26f0cdccc, [Add mvts(4) driver for internal thermal sensor.., 2012-08-18]), and the problem persists. (I'm still using 9.0-RELEASE#0, just in case it's even deeper than libedit.) --steffen
----- Forwarded message from "Steffen \"Daode\" Nurpmeso" <sdaoden@gmail.com> ----- Date: Sun, 19 Aug 2012 21:08:11 +0200 From: "Steffen \"Daode\" Nurpmeso" <sdaoden@gmail.com> To: freebsd-bugs@FreeBSD.org Cc: Subject: Re: bin/170651: On 9.0-RELEASE#0 and master sh(1) gobbles high bit at first User-Agent: S-nail <12.5 7/5/10;s-nail-9-g517ac44-dirty> Yet another update. I've #defined DEBUG_READ in libedit and actually found my problem. The reason why the german umlauts don't appear is that they actually result in "ed-unassigned" errors because of the input-to-command map. I'm using multi-platform multi-shell init scripts which i've just recently updated to be compatible to some pretty ancient Bourne-compatible shell, and now it turns out that the plain FreeBSD /bin/sh is classified as "NONE" type at all, so that no set command is used to configure shell behaviour upon startup. I didn't think about this before, hmm. Anyway, once the login shell starts, it's a vanilla shell, but with emacs mode enabled, or at least this is what the '$ set -o' says which then also turns over all the ED_UNASSIGNED to ED_INSERT commands. If nobody jumps into this here i'll try to figure out why, how and where that actually happens and fix it, maybe next week. Ciao, --steffen _______________________________________________ freebsd-bugs@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-bugs To unsubscribe, send any mail to "freebsd-bugs-unsubscribe@freebsd.org" ----- End forwarded message -----
State Changed From-To: open->closed Per submitter request
This PR bin/170651 does in fact look valid. It looks like libedit only partially picks up an LC_CTYPE change. Whether it shows '\OOO' or real characters with bit 7 set changes immediately; whether it accepts characters with bit 7 set changes only after doing something that causes libedit to be reinitialized (such as 'set +o'). This is probably a bug in libedit and/or sh's use of it, although there are bigger bugs in libedit and sh's use of it. -- Jilles Tjoelker
State Changed From-To: closed->open per jilles request
Jilles Tjoelker <jilles@stack.nl> wrote: |This PR bin/170651 does in fact look valid. It looks like libedit only |partially picks up an LC_CTYPE change. Whether it shows '\OOO' or real |characters with bit 7 set changes immediately; whether it accepts |characters with bit 7 set changes only after doing something that causes |libedit to be reinitialized (such as 'set +o'). | |This is probably a bug in libedit and/or sh's use of it, although there |are bigger bugs in libedit and sh's use of it. | |-- |Jilles Tjoelker As all locale changes effectively boil down in var.c:{update,init}charset(), either call histedit() in updatecharset(), which is what the first (-easy) diff does, or introduce a new function which only performs the necessary libedit call-in to update the mapping (second patch, -harder). Since histedit() always causes a reparse of editrc(5) (and AFAIU), i would vote for the harder patch :) Thanks and ciao, --steffen
Steffen "Daode" Nurpmeso <sdaoden@gmail.com> wrote: |Jilles Tjoelker <jilles@stack.nl> wrote: ||This PR bin/170651 does in fact look valid. It looks like libedit only ||partially picks up an LC_CTYPE change. Whether it shows '\OOO' or real ||characters with bit 7 set changes immediately; whether it accepts ||characters with bit 7 set changes only after doing something that causes ||libedit to be reinitialized (such as 'set +o'). || ||This is probably a bug in libedit and/or sh's use of it, although there ||are bigger bugs in libedit and sh's use of it. || ||-- ||Jilles Tjoelker | |As all locale changes effectively boil down in \ |var.c:{update,init}charset(), |either call histedit() in updatecharset(), which is what the first (-easy) |diff does, or introduce a new function which only performs the necessary |libedit call-in to update the mapping (second patch, -harder). |Since histedit() always causes a reparse of editrc(5) (and AFAIU), i would |vote for the harder patch :) Hmm, looking deeper at the code as such i indeed see some more issues. E.g., bindings get all lost if a set command occurs, as in '$ bind ^H sh-complete', '$ set +o' and it's gone ... and everything around in there. So i think this should encapsulate itself further than what currently happens and what i've sent to you already, and instead act upon real changes on the actual iflag/Vflag/Eflag combination only. I've still not wrapped my head around that entirely, but i (also still) wonder why there is not a single histedit_init() in main(), and maximally a histedit_destroy() after an exec (forgotten where), as *iflag* is a pretty constant condition!? Then a single histedit_on_change() could be called and conditionalize further execution on 'is_init && STATE_CHANGE_OCCURRED'. I would like to give that a try now.. I'll append a patch which does some cleanup on really ugly code. Thanks and ciao, --steffen diff --git a/bin/sh/input.c b/bin/sh/input.c index 12f285f..1da03c6 100644 --- a/bin/sh/input.c +++ b/bin/sh/input.c @@ -59,8 +59,10 @@ __FBSDID("$FreeBSD$"); #include "error.h" #include "alias.h" #include "parser.h" -#include "myhistedit.h" #include "trap.h" +#ifndef NO_HISTORY +# include "myhistedit.h" +#endif #define EOF_NLEFT -99 /* value of parsenleft when EOF pushed back */ @@ -102,8 +104,6 @@ static struct parsefile *parsefile = &basepf; /* current input file */ int init_editline = 0; /* editline library initialized? */ int whichprompt; /* 1 == PS1, 2 == PS2 */ -EditLine *el; /* cookie for editline package */ - static void pushfile(void); static int preadfd(void); static void popstring(void);
Steffen "Daode" Nurpmeso <sdaoden@gmail.com> wrote: |Steffen "Daode" Nurpmeso <sdaoden@gmail.com> wrote: ||Jilles Tjoelker <jilles@stack.nl> wrote: |||This PR bin/170651 does in fact look valid. It looks like libedit only [.] |||-- |||Jilles Tjoelker [.] |So i think this should encapsulate itself further than what |currently happens and what i've sent to you already, and instead |act upon real changes on the actual iflag/Vflag/Eflag combination |only. [.] This is what i come up with, and it works smoothly. One exception is maybe the histedit_init=2 case with the bundled delayed histedit(1) from within main(). If that would be possible it'll most likely safe a lot of (up to three in my case since my shell init scripts only set LC_ALL and LANG) calls to histedit() which actually would *force* a reinitialization, but i'm not really convinced it'll work at this very place and in respect to what bind(1) and such return from if used from within the init scripts... So, i hate to say it, the following additional patch is maybe necessary for real in addition. (Sob.) |diff --git a/bin/sh/histedit.c b/bin/sh/histedit.c |index bd47c0d..0f6d2ba 100644 |--- a/bin/sh/histedit.c |+++ b/bin/sh/histedit.c |@@ -90,7 +90,6 @@ histedit(int force) | if (! histedit_init) | return; | |- histedit_init = 2; | /* options.c ensures these are mutual exclusive */ | nedstate = (Eflag ? 1 : 0) | (Vflag ? 2 : 0); | |diff --git a/bin/sh/main.c b/bin/sh/main.c |index 684ce8c..ec2275b 100644 |--- a/bin/sh/main.c |+++ b/bin/sh/main.c |@@ -151,6 +151,7 @@ main(int argc, char *argv[]) | chkmail(1); | #ifndef NO_HISTORY | histedit_init = 1; |+ histedit(1); | #endif | } | if (argv[0] && argv[0][0] == '-') { |@@ -164,10 +165,6 @@ state1: | read_profile("/etc/suid_profile"); | } | state2: |-#ifndef NO_HISTORY |- if (iflag && histedit_init != 2) |- histedit(1); |-#endif | state = 3; | if (!privileged && iflag) { | if ((shinit = lookupvar("ENV")) != NULL && *shinit != '\0') { Thanks for FreeBSD, eh? --steffen diff --git a/bin/sh/histedit.c b/bin/sh/histedit.c index 6371599..bd47c0d 100644 --- a/bin/sh/histedit.c +++ b/bin/sh/histedit.c @@ -67,7 +67,9 @@ __FBSDID("$FreeBSD$"); History *hist; /* history cookie */ EditLine *el; /* editline cookie */ int displayhist; +int histedit_init; static FILE *el_in, *el_out, *el_err; +static int e1v2; static char *fc_replace(const char *, char *, char *); static int not_fcnumber(const char *); @@ -76,12 +78,21 @@ static int str_to_event(const char *, int); /* * Set history and editing status. Called whenever the status may * have changed (figures out what to do). + * If force is set then an editline reinit is issued even if the actual edit + * mode hasn't changed - necessary after the locale has changed because + * editline bases it's decision what is reported or not upon isprint(3) */ void -histedit(void) +histedit(int force) { + int nedstate; -#define editing (Eflag || Vflag) + if (! histedit_init) + return; + + histedit_init = 2; + /* options.c ensures these are mutual exclusive */ + nedstate = (Eflag ? 1 : 0) | (Vflag ? 2 : 0); if (iflag) { if (!hist) { @@ -97,7 +108,7 @@ histedit(void) else out2fmt_flush("sh: can't initialize history\n"); } - if (editing && !el && isatty(0)) { /* && isatty(2) ??? */ + if (nedstate && ! el && isatty(0)) { /* && isatty(2) ??? */ /* * turn editing on */ @@ -130,17 +141,14 @@ bad: out2fmt_flush("sh: can't initialize editing\n"); } INTON; - } else if (!editing && el) { + } else if (! nedstate && el) { INTOFF; el_end(el); el = NULL; INTON; } - if (el) { - if (Vflag) - el_set(el, EL_EDITOR, "vi"); - else if (Eflag) - el_set(el, EL_EDITOR, "emacs"); + if (el && (nedstate != e1v2 || force)) { + el_set(el, EL_EDITOR, (nedstate & 1) ? "emacs" : "vi"); el_set(el, EL_BIND, "^I", "sh-complete", NULL); el_source(el, NULL); } @@ -155,7 +163,10 @@ bad: hist = NULL; } INTON; + nedstate = 0; } + + e1v2 = nedstate; } diff --git a/bin/sh/input.c b/bin/sh/input.c index 12f285f..1da03c6 100644 --- a/bin/sh/input.c +++ b/bin/sh/input.c @@ -59,8 +59,10 @@ __FBSDID("$FreeBSD$"); #include "error.h" #include "alias.h" #include "parser.h" -#include "myhistedit.h" #include "trap.h" +#ifndef NO_HISTORY +# include "myhistedit.h" +#endif #define EOF_NLEFT -99 /* value of parsenleft when EOF pushed back */ @@ -102,8 +104,6 @@ static struct parsefile *parsefile = &basepf; /* current input file */ int init_editline = 0; /* editline library initialized? */ int whichprompt; /* 1 == PS1, 2 == PS2 */ -EditLine *el; /* cookie for editline package */ - static void pushfile(void); static int preadfd(void); static void popstring(void); diff --git a/bin/sh/main.c b/bin/sh/main.c index 5eb12e0..684ce8c 100644 --- a/bin/sh/main.c +++ b/bin/sh/main.c @@ -73,6 +73,9 @@ __FBSDID("$FreeBSD$"); #include "exec.h" #include "cd.h" #include "builtins.h" +#ifndef NO_HISTORY +# include "myhistedit.h" +#endif int rootpid; int rootshell; @@ -144,8 +147,12 @@ main(int argc, char *argv[]) setstackmark(&smark2); procargs(argc, argv); pwd_init(iflag); - if (iflag) + if (iflag) { chkmail(1); +#ifndef NO_HISTORY + histedit_init = 1; +#endif + } if (argv[0] && argv[0][0] == '-') { state = 1; read_profile("/etc/profile"); @@ -157,6 +164,10 @@ state1: read_profile("/etc/suid_profile"); } state2: +#ifndef NO_HISTORY + if (iflag && histedit_init != 2) + histedit(1); +#endif state = 3; if (!privileged && iflag) { if ((shinit = lookupvar("ENV")) != NULL && *shinit != '\0') { diff --git a/bin/sh/myhistedit.h b/bin/sh/myhistedit.h index e31276d..24123e1 100644 --- a/bin/sh/myhistedit.h +++ b/bin/sh/myhistedit.h @@ -35,8 +35,8 @@ extern History *hist; extern EditLine *el; extern int displayhist; +extern int histedit_init; -void histedit(void); +void histedit(int); void sethistsize(const char *); void setterm(const char *); - diff --git a/bin/sh/options.c b/bin/sh/options.c index 4c2c8ab..5198a52 100644 --- a/bin/sh/options.c +++ b/bin/sh/options.c @@ -58,7 +58,7 @@ __FBSDID("$FreeBSD$"); #include "mystring.h" #include "builtins.h" #ifndef NO_HISTORY -#include "myhistedit.h" +# include "myhistedit.h" #endif char *arg0; /* value of $0 */ @@ -131,7 +131,7 @@ optschanged(void) { setinteractive(iflag); #ifndef NO_HISTORY - histedit(); + histedit(0); #endif setjobctl(mflag); } diff --git a/bin/sh/trap.c b/bin/sh/trap.c index 521c511..b0d8b11 100644 --- a/bin/sh/trap.c +++ b/bin/sh/trap.c @@ -56,8 +56,9 @@ __FBSDID("$FreeBSD$"); #include "trap.h" #include "mystring.h" #include "builtins.h" -#include "myhistedit.h" - +#ifndef NO_HISTORY +# include "myhistedit.h" +#endif /* * Sigmode records the current value of the signal handlers for the various diff --git a/bin/sh/var.c b/bin/sh/var.c index 6041459..d1af678 100644 --- a/bin/sh/var.c +++ b/bin/sh/var.c @@ -65,7 +65,7 @@ __FBSDID("$FreeBSD$"); #include "parser.h" #include "builtins.h" #ifndef NO_HISTORY -#include "myhistedit.h" +# include "myhistedit.h" #endif @@ -523,6 +523,9 @@ updatecharset(void) charset = nl_langinfo(CODESET); localeisutf8 = !strcmp(charset, "UTF-8"); +#ifndef NO_HISTORY + histedit(1); +#endif } void
For bugs matching the following criteria: Status: In Progress Changed: (is less than) 2014-06-01 Reset to default assignee and clear in-progress tags. Mail being skipped
This PR is quite old by now. Is it still relevant?