Bug 194458 - [patch] simplify xinput.sh to use new xinputrc
[patch] simplify xinput.sh to use new xinputrc
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: xorg-x11-xinit (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mike A. Harris
: EasyFix, i18n
Depends On:
Blocks: 182541
  Show dependency treegraph
 
Reported: 2006-06-08 06:52 EDT by Jens Petersen
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-05 03:27:09 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
xinput.sh-xinputrc.patch (1018 bytes, patch)
2006-06-08 06:52 EDT, Jens Petersen
no flags Details | Diff
xorg-x11-xinit.spec.patch (844 bytes, patch)
2006-06-16 05:05 EDT, Jens Petersen
no flags Details | Diff
xinput.sh-xinputrc-take2.patch (2.09 KB, patch)
2006-07-04 05:23 EDT, Jens Petersen
no flags Details | Diff

  None (edit)
Description Jens Petersen 2006-06-08 06:52:12 EDT
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.
Comment 1 Jens Petersen 2006-06-08 06:52:12 EDT
Created attachment 130730 [details]
xinput.sh-xinputrc.patch
Comment 2 Jens Petersen 2006-06-15 04:03:27 EDT
ping
Comment 3 Jens Petersen 2006-06-16 05:05:50 EDT
Created attachment 131029 [details]
xorg-x11-xinit.spec.patch
Comment 5 Mike A. Harris 2006-06-30 20:51:12 EDT
--- 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.

Comment 6 Jens Petersen 2006-07-02 04:53:45 EDT
Yes, an if-elif block makes sense: thanks for that.
Comment 7 Jens Petersen 2006-07-04 05:23:16 EDT
Created attachment 131894 [details]
xinput.sh-xinputrc-take2.patch

Here is an updated patch which implements the change
using if-elif-fi.
Comment 8 Mike A. Harris 2006-07-05 03:27:09 EDT
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
Comment 9 Jens Petersen 2006-07-05 04:37:05 EDT
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/).
Comment 10 Mike A. Harris 2006-07-05 04:47:14 EDT
Ok, thanks for the update.

Note You need to log in before you can comment on or make changes to this bug.