Red Hat Bugzilla – Bug 194458
[patch] simplify xinput.sh to use new xinputrc
Last modified: 2007-11-30 17:11:34 EST
Description of problem: The I18N team would like to simplify the xinput.d system currently implemented in xinput.sh to a simpler system in FC 6 we're calling xinputrc which avoids the current somewhat overcomplex input method configuration per locale and replaces it with a system xinputrc config file and a user .xinputrc file. This will make it simpler to setup IM for users and sysadmins, and will also be used by the new im-chooser tool written by Tagoh. The attached simple patch implements the change.
Created attachment 130730 [details] xinput.sh-xinputrc.patch
ping
Created attachment 131029 [details] xorg-x11-xinit.spec.patch
--- xinput.sh 1 Nov 2005 02:00:56 -0000 1.1 +++ xinput.sh 7 Jun 2006 15:56:23 -0000 @@ -20,18 +20,12 @@ tmplang=${LC_CTYPE:-${LANG:-"en_US.UTF-8"}} -## try to source ~/.xinput.d/ll_CC or /etc/X11/xinit/xinput.d/ll_CC to -## setup the input method for locale (CC is needed for Chinese for example) # unset env vars to be safe unset XIM XIM_PROGRAM XIM_ARGS XMODIFIERS GTK_IM_MODULE QT_IM_MODULE -lang_region=$(echo $tmplang | sed -e 's/\..*//') -for f in $HOME/.xinput.d/${lang_region} \ - $HOME/.xinput.d/default \ - /etc/X11/xinit/xinput.d/${lang_region} \ - /etc/X11/xinit/xinput.d/default ; do + +for f in $HOME/.xinputrc /etc/X11/xinit/xinputrc; do [ -r $f ] && source $f && break done [SNIP] Looks mostly ok to me. The for loop doesn't seem as readable to me as an if-elif block. Other than that though, it looks ok. I'm going to rewrite that part, and attach the patch here which I end up committing.
Yes, an if-elif block makes sense: thanks for that.
Created attachment 131894 [details] xinput.sh-xinputrc-take2.patch Here is an updated patch which implements the change using if-elif-fi.
I implemented a fix almost identical to yours last week but forgot to commit to CVS. I've merged some of your readability changes, and cleaned it up a bit now, and committed to CVS and built in rawhide. Please test and let me know if there are problems. Also, I'm curious if the "unset XIM XIM_PROGRAM ..." line really needs to be before the .xinputrc/xinputrc processing, or if it should come after it entirely. --------------------------------- # unset env vars to be safe unset XIM XIM_PROGRAM XIM_ARGS XMODIFIERS GTK_IM_MODULE QT_IM_MODULE if [ -r "$USER_XINPUTRC" ]; then source "$USER_XINPUTRC" elif [ -r "$SYS_XINPUTRC" ]; then source "$SYS_XINPUTRC" fi [ -n "$GTK_IM_MODULE" ] && export GTK_IM_MODULE [ -n "$QT_IM_MODULE" ] && export QT_IM_MODULE --------------------------------- It seems at a glance that perhaps the "unset" line seems should preceed the final 2 lines, or that the final 2 lines there should preceed the if block, to group them together. Or is there a reason the if block needs to be invoked after to the "unset", but before the GTK_IM_MODULE etc. stuff? Just another slight cosmetic oddity I thought perhaps should be cleaned up while in there also, but chose to leave it for now, as it isn't completely clear to me without knowing how that would affect the non-included "xinputrc" files. TIA
Thanks for the build. No the placing of the unset is intentional: to prevent erroneous/obsolete IM settings of those variables in say .i18n or i18n from breaking configuration with xinputrc (or the old xinput.d/).
Ok, thanks for the update.