Redone: Fix PR 2527

[Sorky]

Fix for hotkey_popup via awful menu showing on wrong screen

#2527

Next comments will show local testing and confirmation

[Sorky]

Confirming local development git is up to date and has no other changes...

clone@OH-Server:/_testing/awesome$ git status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git checkout -- ..." to discard changes in working directory)

    modified:   awesomerc.lua
    modified:   lib/awful/hotkeys_popup/init.lua

Untracked files:
(use "git add ..." to include in what will be committed)

    .orig.awesomerc.lua
    lib/awful/hotkeys_popup/.orig.init.lua

no changes added to commit (use "git add" and/or "git commit -a")

[Sorky]

Confirming changes are the same...

clone@OH-Server:/_testing/awesome$ diff awesomerc.lua .orig.awesomerc.lua 
17c17
< local hotkeys_popup = require("awful.hotkeys_popup")
---
> local hotkeys_popup = require("awful.hotkeys_popup").widget
107c107
<    { "hotkeys", hotkeys_popup.show },
---
>    { "hotkeys", function() return false, hotkeys_popup.show_help end},

[Sorky]

Confirming changes are the same...

clone@OH-Server:/_testing/awesome$ diff lib/awful/hotkeys_popup/init.lua lib/awful/hotkeys_popup/.orig.init.lua 
8a9
> 
12,27d12
< local awful = { screen = require( "awful.screen" ) }
< 
< --- awful.menu callable version of show popup with hotkeys help.
< --
< -- example usage: myawesomemenu = {{ "hotkeys", hotkeys_popup.show },
< --
< -- see `awful.hotkeys_popup.widget.show_help` for more information
< 
< hotkeys_popup.show = function() hotkeys_popup.widget.show_help( _, awful.screen.focused() ) end
< 
< --- Show popup with hotkeys help (shortcut to awful.hotkeys_popup.widget.show_help).
< --
< -- example usage: myawesomemenu = {{ "hotkeys", function() return false, hotkeys_popup.show_help end},
< --
< -- see `awful.hotkeys_popup.widget.show_help` for more information
< 

[Sorky]

Confirming running build versiion with modified awesomerc.lua...

clone@OH-Server:~$ ps -a | grep awesome
10062 tty1     00:01:54 awesome
25434 tty6     00:00:01 awesome
clone@OH-Server:~$ ps -Flww -p 25434
F S UID        PID  PPID  C PRI  NI ADDR SZ WCHAN    RSS PSR STIME TTY          TIME CMD
0 S clone    25434 25423  0  80   0 - 66525 poll_s 42472   3 20:32 tty6     00:00:01 /_testing/awesome/build/awesome -c /_testing/awesome/build/awesomerc.lua

[codecov]

Codecov Report

Merging #2583 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2583      +/-   ##
==========================================
+ Coverage   84.65%   84.65%   +<.01%     
==========================================
  Files         496      496              
  Lines       33738    33740       +2     
==========================================
+ Hits        28562    28564       +2     
  Misses       5176     5176

Flag	Coverage Δ
#c_code	73.53% <ø> (ø)	⬆️
#functionaltests	70.96% <100%> (ø)	⬆️
#lua53	84.65% <100%> (ø)	⬆️
#samples	74.65% <ø> (ø)	⬆️
#themes	49.78% <100%> (+0.03%)	⬆️
#unittests	58.17% <ø> (ø)	⬆️

Impacted Files	Coverage Δ
lib/awful/hotkeys_popup/init.lua	100% <100%> (ø)	⬆️

[actionless]

as it was stated in #2580: described problem should be solved by creating new optional arg, not introducing new wrapper for the function

[Sorky]

Why add that sort of complexity? The wrapper is IMHO quite elegant, aligns with the existing one in init.lua and allows a very neat rc.lua

[actionless]

i've read your opinion fine from the first time though i still have a different vision on this

adding new function wrapping some existing one instead of just an arg for existing one, that's a good example of redundant complexity

[Sorky]

Thank you for trying to explain why (it helps me know what to do in the future) - Are there any more Pros or Cons of the two approaches?

[actionless]

just when you trying to do something -- look on other awesomewm modules and see how similar tasks are usually solved

it helps me know what to do in the future

i already told you in your original PR review: look how show_awesome_keys option is handled by show_help() function and add your your new option

and next step, after you will add your new option to show_help, modify rc.lua to pass that option as true when calling hotkeys popup from the menu

just to make it completely unequivocal -- there is no new function need to be added

[Sorky]

Forget the previous - I get it - You don't want a shim/stub

As you have surmised, I want it as it fixes the bug and cleans up my rc.lua

Removing stuff users should not care about &simplifying the code is all I'm trying to do

At least we know that the 'false, ' can go

I'll listen to the majority consensus

[actionless]

yup, we could wait for more reviews

[Sorky]

Before I re-test alternatives, I've provided two other implementations for consideration on style...

#2585
#2586