lxqt / lxqt-config

Tools to configure LXQt and the underlying operating system
https://lxqt.github.io
GNU Lesser General Public License v2.1
79 stars 60 forks source link

Keyboard layout setting needs setxkbmap #967

Open gfgit opened 7 months ago

gfgit commented 7 months ago
Expected Behavior

If setxkbmap is not installed and user changes keyboard layout, warn the user with a dialog and revert changes in UI. Also if it can't load current layout on startup, show like a placeholder text in the list view.

Current Behavior

When opening settings, no layout is set in the view. When adding one, error is silently ignored, UI shows the new layout making it seem successful. If you close and re-open you see again empty layout list view.

Possible Solution
Steps to Reproduce (for bugs)
  1. Check setxkbmap is not installed
  2. Open keyboard and mouse settings page
  3. See no current layout set
  4. Add/Change layout
  5. See no error message but the change is not applied
Context

I'm using PostmarketOS edge with LXQt UI (package postmarketos-ui-lxqt) which does not install setxkbmap by default. This is an issue on their side and I report it as well. It took me some time to figure out the fault was this missing package so it would be nice to be informed by a dialog in those cases. This can also be extended to checking process exit status or other ways to confirm change is successful before showing the user the new layout as "correctly set".

This pattern may be present in other parts of code but I'm still learning LXQt internals. I'm happy to make a pull request if you agree on the concept.

System Information
tsujan commented 7 months ago

The report makes sense to me.

This can also be extended to checking process exit status

I don't remember the code (and don't have time to read it now), but I don't think QProcess::ExitStatus would make any difference in this case (although it should be checked for other reasons). QProcess::waitForStarted() may be the solution.

gfgit commented 7 months ago

So you say if setxkbmap is correctly installed but it fails to apply changes, it would still exit 0 leaving us with no clue? Will experiment a bit and get back.

tsujan commented 7 months ago

On the contrary, I meant that if setxkbmap didn't exist, QProcess::ExitStatus wouldn't show a problem, but QProcess::waitForStarted() would. And yes, please test that; I'm not sure about my memory ;)

stefonarch commented 3 months ago

On arch it's quite impossible to remove setxkbmap:

$ yay -R xorg-setxkbmap 
checking dependencies...
error: failed to prepare transaction (could not satisfy dependencies)
:: removing xorg-setxkbmap breaks dependency 'xorg-setxkbmap' required by xorg-server-common
 -> exit status 1
stef@archlinux:lxqt-config$ yay -R xorg-setxkbmap xorg-server-common 
checking dependencies...
error: failed to prepare transaction (could not satisfy dependencies)
:: removing xorg-server-common breaks dependency 'xorg-server-common' required by xorg-server
:: removing xorg-server-common breaks dependency 'xorg-server-common' required by xorg-server-xephyr
:: removing xorg-server-common breaks dependency 'xorg-server-common' required by xorg-server-xvfb
:: removing xorg-server-common breaks dependency 'xorg-server-common' required by xorg-xwayland

but the check could be needed as well for other situations, will test on X11 later.