Bug 194458 - [patch] simplify xinput.sh to use new xinputrc
Summary: [patch] simplify xinput.sh to use new xinputrc
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: xorg-x11-xinit
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mike A. Harris
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks: 182541
TreeView+ depends on / blocked
 
Reported: 2006-06-08 10:52 UTC by Jens Petersen
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2006-07-05 07:27:09 UTC
Type: ---
Embargoed:


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

Description Jens Petersen 2006-06-08 10:52:12 UTC
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 10:52:12 UTC
Created attachment 130730 [details]
xinput.sh-xinputrc.patch

Comment 2 Jens Petersen 2006-06-15 08:03:27 UTC
ping

Comment 3 Jens Petersen 2006-06-16 09:05:50 UTC
Created attachment 131029 [details]
xorg-x11-xinit.spec.patch

Comment 5 Mike A. Harris 2006-07-01 00:51:12 UTC
--- 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 08:53:45 UTC
Yes, an if-elif block makes sense: thanks for that.

Comment 7 Jens Petersen 2006-07-04 09:23:16 UTC
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 07:27:09 UTC
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 08:37:05 UTC
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 08:47:14 UTC
Ok, thanks for the update.


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