Redone: Fix for Issue 2527 #2580

Sorky · 2019-01-12T10:09:19Z

Fix for hotkey_popup via awful menu showing on wrong screen

Next comments will show local testing and confirmation

* Update awesomerc.lua * Update init.lua (#6)

* Update init.lua Add return values to allow for a fallback theme in the case of an error Ensured debug messages are printed Fixed table option Improved error handling testing... local code, err, file = nil, nil, nil code, err = beautiful.myinit( file ) -- empty variable will fail > 2019-01-11 14:23:54 E: beautiful: error loading theme: no theme specified code, err = beautiful.myinit( path_this .. "themes/shelby/bad.lua" ) -- file that loadfile says is ok but dofile errors on > 2019-01-11 14:23:54 E: beautiful: error loading theme: /home/david/.config/awesome/themes/shelby/bad.lua > 2019-01-11 14:23:54 E: theme file error: /home/david/.config/awesome/themes/shelby/bad.lua:17: attempt to index a nil value (global 'beautiful') code, err = beautiful.myinit( function() end ) -- Should not do anything with this > 2019-01-11 14:23:54 E: beautiful: error loading theme: function: 0x55f6006795f0 > 2019-01-11 14:23:54 E: theme file error: nil code, err = beautiful.myinit( {} ) -- Same as not providing a file 2019-01-11 14:23:54 E: beautiful: error loading theme: table: 0x55f600679770 2019-01-11 14:23:54 E: theme file error: nil code, err = beautiful.myinit( path_this .. "themes/shelby/theme.lua" ) -- A valid file >

* Update luaa.c Required code changes Test cases... startx [exec awesome] user's rc.lua xdg/awesome/rc.lua startx [exec awesome -c <name>] '/home/user/.config/awesome/xyz.lua' '/home/user/xyz.lua' Xephyr [in build directory] DISPLAY=:1 ./awesome -c awesomerc.lua Made config_path conform to style of themes_path and icon_path (ie: removed the final '/')

Fixing Travis warning

codecov · 2019-01-12T10:38:28Z

Codecov Report

Merging #2580 into master will increase coverage by 0.01%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #2580      +/-   ##
==========================================
+ Coverage   84.66%   84.68%   +0.01%     
==========================================
  Files         496      496              
  Lines       33738    33762      +24     
==========================================
+ Hits        28563    28590      +27     
+ Misses       5175     5172       -3

Flag	Coverage Δ
#c_code	`73.51% <45.45%> (-0.04%)`	⬇️
#functionaltests	`70.99% <60%> (+0.03%)`	⬆️
#lua53	`84.68% <71.42%> (+0.01%)`	⬆️
#samples	`74.65% <ø> (ø)`	⬆️
#themes	`49.77% <60%> (-0.01%)`	⬇️
#unittests	`58.18% <75%> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
lib/beautiful/init.lua	`85.88% <100%> (+3.38%)`	⬆️
lib/awful/hotkeys_popup/init.lua	`100% <100%> (ø)`	⬆️
luaa.c	`64.64% <45.45%> (-0.7%)`	⬇️
property.c	`79.53% <0%> (-0.47%)`	⬇️
lib/gears/debug.lua	`67.79% <0%> (+1.69%)`	⬆️
common/util.h	`91.17% <0%> (+1.89%)`	⬆️
lib/awful/layout/suit/corner.lua	`91.15% <0%> (+6.19%)`	⬆️

Sorky · 2019-01-12T10:42:14Z

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 · 2019-01-12T10:42:37Z

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 · 2019-01-12T10:43:12Z

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 · 2019-01-12T10:43:39Z

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

actionless

i've already explained to you what changing hotkeys_popup to add function which makes sense only in context of using it as callback in menu won't be welcome

please redo it normally

actionless · 2019-01-12T13:37:13Z

and second question: how do you know it's fixing multi-screen issue if you told in another ticket what you don't have env to test it?

Sorky · 2019-01-12T14:05:29Z

and second question: how do you it's fixing multi-screen issue if you told in another ticket what you don't have env to test it?

Second question first...

I have a dual head system but it is running 4.2 and I used Xephyr to test the mods, but I only have that working with one display

I did actually mention that I found a way to test it in the 'Issue' - "I cloned my user and set it up so startx runs /_testing/awesome/build/awesome"

Here's a more detailed explanation...

I cloned my user and set it up so startx runs /_testing/awesome/build/awesome

I'm now able to login as 'clone' on a separate tty 'and simply startx running the build version

All this can be see in the 4 testing comments I posted with this PR...

Shows the test build is up to date and only has the changed stipulated
Shows my local awesomerc.lua changes so you can confirm they are the same as in this PR
Shows my local lib/awful/hotkeys_popup/init.lua changes so you can confirm they are the same as in this PR
Shows that I am running the build version on tty6 as user clone

Sorky · 2019-01-12T14:08:38Z

i've already explained to you what changing hotkeys_popup to add function which makes sense only in context of using it as callback in menu won't be welcome

please redo it normally

While I didn't understand you saying that, it works in any context (ie: the following will also work)...

globalkeys = gears.table.join(
    awful.key({ modkey,           }, "s",      hotkeys_popup.show,

The difference between the two is that hotkeys_popup.show shows on the display where the mouse is whereas hotkeys_popup.show_help opens on the display where the last used client is (nice for keyboard only users so I didn't want to replace/alter it)

actionless · 2019-01-12T14:16:13Z

it works in any context

but it drops first argument specifically for compatibility with being used as menu callback

actionless · 2019-01-12T14:17:13Z

The difference between the two is that hotkeys_popup.show shows on the display where the mouse is whereas hotkeys_popup.show_help opens on the display where the last used client is (nice for keyboard only users so I didn't want to replace/alter it)

instead i recommend you adding optional argument to function show_help to define that behavior explicitly (in the same style like its behavior is altered by show_awesome_keys option)

Sorky · 2019-01-12T14:48:29Z

it works in any context

but it drops first argument specifically for compatibility with with being used as menu callback

That is far from the case - The menu callback behaviour only occurs when the function call is done without brackets In short, the menu compatibility comes from the new 'show' function not having any arguments, so whatever the menu passes goes nowhere

The reason I pass nil as the first argument is because the client isn't a factor. In fact, if no client is currently in focus on the current screen but one is in focus on the other screen (the error case that I am fixing) the existing code c = c or capi.client.focus (which still executes with me passing in nil) actually results in a value of c that is for the client on the other screen. The fix is that by specifying 's' it doesn't change to the screen with the selected client and remains as the screen that has the mouse which is where the menu was clicked. See the following excerpt from awesome/lib/awful/hotkeys_popup/widget.lua...

    function widget_instance:show_help(c, s, show_args)
<snip>
        c = c or capi.client.focus
        s = s or (c and c.screen or awful.screen.focused())
<snip>
        local help_wibox = self._cached_wiboxes[s][joined_groups]
        help_wibox:show()

Sorky · 2019-01-12T15:04:46Z

Note: Comment above was updated for greater accuracy and clarity

actionless · 2019-01-12T15:20:39Z

anyway the described problem should be solved by creating new optional arg, not new function wrapper

Sorky · 2019-01-12T15:45:07Z

anyway the described problem should be solved by creating new optional arg, not new function wrapper

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

Sorky · 2019-01-12T16:58:47Z

Damn - Github merged my fork - I will close this and redo

Sorky added 6 commits January 12, 2019 20:44

Update awesomerc.lua

f7504a0

Update init.lua (#6)

30327f8

Fix PR 2527

109bf54

* Update awesomerc.lua * Update init.lua (#6)

Update init.lua

d3920b1

Fixing Travis warning

Sorky changed the title ~~Fix for pr 2527~~ Fix for Issue 2527 Jan 12, 2019

Sorky mentioned this pull request Jan 12, 2019

Question: Discussion: Trying to understand hotkeys_popup.show_help #2527

Closed

actionless requested changes Jan 12, 2019

View reviewed changes

Sorky added 5 commits January 13, 2019 03:16

Create icon5.png

b49bfec

Merge branch 'master' into fix-for-pr-2527

c0139c4

Delete init.lua

0243195

Create icon6.xpm

90cd23b

Create icon7.svg

eeb9f73

Sorky closed this Jan 12, 2019

Sorky changed the title ~~Fix for Issue 2527~~ Redone: Fix for Issue 2527 Jan 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redone: Fix for Issue 2527 #2580

Redone: Fix for Issue 2527 #2580

Sorky commented Jan 12, 2019 •

edited

Loading

codecov bot commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

actionless left a comment

actionless commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019

Sorky commented Jan 12, 2019 •

edited

Loading

actionless commented Jan 12, 2019 •

edited

Loading

actionless commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

actionless commented Jan 12, 2019

Sorky commented Jan 12, 2019

Sorky commented Jan 12, 2019

Redone: Fix for Issue 2527 #2580

Redone: Fix for Issue 2527 #2580

Conversation

Sorky commented Jan 12, 2019 • edited Loading

codecov bot commented Jan 12, 2019 • edited Loading

Codecov Report

Sorky commented Jan 12, 2019 • edited Loading

Sorky commented Jan 12, 2019 • edited Loading

Sorky commented Jan 12, 2019 • edited Loading

Sorky commented Jan 12, 2019 • edited Loading

actionless left a comment

Choose a reason for hiding this comment

actionless commented Jan 12, 2019 • edited Loading

Sorky commented Jan 12, 2019

Sorky commented Jan 12, 2019 • edited Loading

actionless commented Jan 12, 2019 • edited Loading

actionless commented Jan 12, 2019 • edited Loading

Sorky commented Jan 12, 2019 • edited Loading

Sorky commented Jan 12, 2019 • edited Loading

actionless commented Jan 12, 2019

Sorky commented Jan 12, 2019

Sorky commented Jan 12, 2019

Sorky commented Jan 12, 2019 •

edited

Loading

codecov bot commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

actionless commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

actionless commented Jan 12, 2019 •

edited

Loading

actionless commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading

Sorky commented Jan 12, 2019 •

edited

Loading