berlam / awesome-switcher

Switch clients in Awesome WM with the familiar preview functionality
ISC License
98 stars 15 forks source link

Added ability to select the next client with the mouse in the preview #5

Closed wyv3rn closed 7 years ago

wyv3rn commented 7 years ago

I'm kinda new to Lua, so any improvement suggestions are welcome. But basically the change should do it's job, i.e. select the appropriate clients as soon as the mouse enters the widget in the preview.

berlam commented 7 years ago

Hi @wyv3rn,

thank you for the contribution. I just want you to know, that I have seen the PR but am currently heavily involved in customer projects. After a quick view on the changes I am not completely sure, why a secondary table is created, when the requirement is to make the previews clickable. Will try to understand it on the weekend, when I have more time for open-source work!

wyv3rn commented 7 years ago

Hey @berlam,

at the position where I added the mouse handler, we know the index i of the preview widget in the leftRightTab. For setting the correct altTabIndex, we need the index j of the corresponding client in the altTabTable. However, i and j are not identical due to the fact that the clients are (re)arranged in two halfs for the leftRightTab (as far as I understood: right half starts with the most recent client in the history and left half ends with the oldest client). That's why I introduced the table leftRightTabToAltTabIndex for mapping i to j. An alternative coud be to recalculate j for the mouse handler the same way as it is done for setting up the previews.

For my fork, I'm gonna change it to an identical mapping soon anyway, as I think that's more intuitive (i.e. most recent client at first position, oldest client at last position).

berlam commented 7 years ago

Thank you for answering my questions and arguing. I will merge your PR.

wyv3rn commented 7 years ago

You're welcome. By the way:

For my fork, I'm gonna change it to an identical mapping soon anyway, as I think that's more intuitive (i.e. most recent client at first position, oldest client at last position).

I have done that in the meantime. This change makes both the leftRightTab and the additional leftRightTabToAltTabIndex table I introduced unnecessary.