Skip to content

Add prev_layout method to keyboardlayout.new #4014

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.

Sign up for GitHub

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

Open
eylles wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add prev_layout method to keyboardlayout.new #4014

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion lib/awful/widget/keyboardlayout.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,10 @@ end
-- @noreturn
-- @method next_layout

--- Select the previous layout.
-- @noreturn
-- @method prev_layout

--- Create a keyboard layout widget.
--
-- It shows current keyboard layout name in a textbox.
Expand Down Expand Up @@ -292,6 +296,10 @@ function keyboardlayout.new(args)
self.set_layout((self._current + 1) % (#self._layout + 1))
end

self.prev_layout = function()
self.set_layout((self._current - 1) % (#self._layout))

@Aire-One Aire-One Sep 1, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(self._current - 1) % (#self._layout)) would not loop back to MAX. (0-1)%MAX will be stuck at 0. So the implementation doesn't mirror the circular behavior from next_layout.

@eylles eylles Sep 1, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the math doesn't math, hence the frustration of the commit message, by all intents the math should not loop back and be stuck on 0, however something happens in some lower level of the API that avoids the off by one error, or rather has a reverse off by one error which causes the wrong incorrect math from this commit to do work and loop the layouts with a circular behaviour, but the "correct" math has the off by one behaviour where it gets stuck at 0 and doesn't loop...

yes it is incredibly frustrating, if you test adding those lines onto your awesome wm library it will work as intended, but the correct math won't...

i do not know why, but it does

@sclu1034 sclu1034 Sep 1, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(0-1)%MAX will be stuck at 0.

That's not how modulo works. The remainder in the division always changes, so this always "loops".
Unless MAX == 1, but then you'd have nothing to loop over anyways.

The only pitfall is that some languages carry the sign and some don't.
E.g. in JavaScript: -1 % 3 == -1 or -4 % 3 == -1.

But Lua doesn't carry the sign, so the result here is always a positive number:

Lua 5.1.5  Copyright (C) 1994-2012 Lua.org, PUC-Rio
> print((0 - 1) % 3)
2
> print((2 - 1) % 3)
1
> print((1 - 1) % 3)
0
> print((0 - 1) % 3)
2

@eylles eylles Sep 1, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if anything the concerning part here is not so much the looping of the modulo operation, but rather the off by one error that should appear but does not because of how the internal API is handling the indexing of the keyboard layout table (well, it is a table to lua but to the x11 API it ought to be an array, dynamic array or even a vector i'd guess) so the internal indexing that makes sense would probably be a starting on 0 index with some check like table_elements >= passed_index making the "3" index (assuming the user set 3 keyboard layouts for simplicity) loop back to (0) the first index of the table, or at least that would be my guess as why this works in such manner, but that is already speculating without looking at the source of the x11 API that lua interacts with...

Lua 5.3.6  Copyright (C) 1994-2020 Lua.org, PUC-Rio
-- positive traversion (next_layout)
> print((0 + 1) % (3 + 1))
1
> print((1 + 1) % (3 + 1))
2
> print((2 + 1) % (3 + 1))
3
> print((3 + 1) % (3 + 1))                                                                                                                                   
0
-- negative traversin (prev_layout)
> print((0 - 1) % 3)
2
> print((1 - 1) % 3)
0
> print((2 - 1) % 3)
1
> print((3 - 1) % 3)
2

end

self.set_layout = function(group_number)
if (0 > group_number) or (group_number > #self._layout) then
error("Invalid group number: " .. group_number ..
Expand All @@ -311,7 +319,8 @@ function keyboardlayout.new(args)

-- Mouse bindings
self.buttons = {
button({ }, 1, self.next_layout)
button({ }, 1, self.next_layout),
button({ }, 3, self.prev_layout)
}

return self
Expand Down