Bug 194458 - [patch] simplify xinput.sh to use new xinputrc
Summary: [patch] simplify xinput.sh to use new xinputrc
Alias: None
Product: Fedora
Classification: Fedora
Component: xorg-x11-xinit (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
Target Milestone: ---
Assignee: Mike A. Harris
QA Contact:
Keywords: EasyFix, i18n
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:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2006-07-05 07:27:09 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
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 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]

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

Comment 3 Jens Petersen 2006-06-16 09:05:50 UTC
Created attachment 131029 [details]

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 @@
-## 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
-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

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]

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

# unset env vars to be safe

if [ -r "$USER_XINPUTRC" ]; then
    source "$USER_XINPUTRC"
elif [ -r "$SYS_XINPUTRC" ]; then
    source "$SYS_XINPUTRC"

[ -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.


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.