Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keyboard layout setting needs setxkbmap #967

Open
gfgit opened this issue Nov 23, 2023 · 4 comments · May be fixed by #968
Open

Keyboard layout setting needs setxkbmap #967

gfgit opened this issue Nov 23, 2023 · 4 comments · May be fixed by #968

Comments

@gfgit
Copy link
Member

gfgit commented Nov 23, 2023

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
  • Distribution & Version: PostmarketOS edge
  • Kernel: 6.6
  • Qt Version: 5.15
  • liblxqt Version: 1.4.0
  • lxqt-build-tools Version:
  • Package version: 1.40
@tsujan
Copy link
Member

tsujan commented Nov 23, 2023

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
Copy link
Member Author

gfgit commented Nov 23, 2023

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
Copy link
Member

tsujan commented Nov 23, 2023

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 ;)

@gfgit gfgit linked a pull request Nov 27, 2023 that will close this issue
@stefonarch
Copy link
Member

stefonarch commented Mar 25, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants