Add prev_layout method to keyboardlayout.new#4014
Add prev_layout method to keyboardlayout.new#4014eylles wants to merge 1 commit intoawesomeWM:masterfrom
Conversation
No, the math for the layout index decreasing does not make the most sense ever but i stopped questioning it and trying to make the in theory correct math work and just accepted that this is what produces the expected behaviour, as whatever happens in the c part of awesome or the x11 side prevents what looks like an off by one error from happening, or maybe it just makes it work somehow i honestly got no idea whatsoever why this works as intended nor i want to know why, all i care is it behaves how i want for all the amounts of layouts i have tried on my machine...
| end | ||
|
|
||
| self.prev_layout = function() | ||
| self.set_layout((self._current - 1) % (#self._layout)) |
There was a problem hiding this comment.
(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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
(0-1)%MAXwill 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
There was a problem hiding this comment.
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|
So tests failing, strange math and seemingly nonsensical behaviour regarding an off by one error that doesn't show aside, could ya test this on your awesome configs just to verify that it isn't a case of "works on my machine" because i'm using awesome 4.3 and this does in fact produce the intended behaviour when added into the libraries of other awesomewm setups. |
|
I don't have a config to test with. As for "tests failing", you've added a new function, but no test case for it. So there is nothing that could fail to begin with. |
This works, no idea how or why it ain't an off by one error but all i know is that it works as intended not as expected from the math...