doc(awful: hotkeys_popup): change variable names to fix ldoc result and add one missing docstring

[actionless]

No description provided.

[codecov-io]

Current coverage is 79.07% (diff: 40.65%)

Merging #1396 into master will not change coverage

@@             master      #1396   diff @@
==========================================
  Files           268        268          
  Lines         16791      16791          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          13278      13278          
  Misses         3513       3513          
  Partials          0          0          

Powered by Codecov. Last update 316734a...bcfb0bc

[actionless]

for some reason this job failed for the first time: https://travis-ci.org/awesomeWM/awesome/jobs/191910409

[Elv13]

Please add some tests to boost the coverage, showing the popup in one of the suit should already help a lot.

[actionless]

i can try but let's merge this asap because it can rot soon because of big indent changes

[blueyed]

it can rot soon because of big indent changes

Should not be an issue as long as nobody else is touching it.. and then even it should be trivial to resolve.

for some reason this job failed for the first time: https://travis-ci.org/awesomeWM/awesome/jobs/191910409

Would be good to have the output here then, since it's lost on restart.

[actionless]

But doc change isn't introducing any new functionality, so I don't see how this PR related to test coverage

[Elv13]

Even if its just indentation, its a 600 line PR with 40% coverage and we are in "stable" mode right now. I know it's "a lot" to ask for an (effectively) 2 line PR, but it's to be a little more strict with this. The menubar and hotkey popup are the 2 least covered modules.

[actionless]

But I am not so much concerned with the docs to rebase it again and again for few months in a row

[Elv13]

oh, come on, the change to bring the coverage to 70% is like 7 lines in test-awesomerc.lua

[actionless]

And I already wrote above what I am ready to do it but it's not related to this pr particularly

[actionless]

Also I need to prepare 2 other unrelated patches for this module (#1401 and some other one which I can't find now, regarding beautiful theme keys for this popup). I will submit them after a PR with the tests.

[Elv13]

Ok, fine then

[actionless]

thanks!

[psychon]

oh, come on, the change to bring the coverage to 70% is like 7 lines in test-awesomerc.lua

Welcome to the maintainer job in a volunteer-based project. ;-)
Thanks for keeping things running while I'm away!

[actionless]

i think what for writing proper tests for widget i need to expose wibox instance from local variable to became a field of hotkey_widget instance (so in the test i can see if widget is visible, or if it contain a pagination inside, etc)

what do you think, is it a good idea to do such changes to support test or should i try to find another way?

https://github.com/awesomeWM/awesome/blob/master/lib/awful/hotkeys_popup/widget.lua#L106

[blueyed]

Sounds good to me, but I am no authority in this regard/region.

[Elv13]

I think you should start by exposing the widget itself. This way you can use the documentation example framework in your documentation. Also, as I proposed some time ago, it could be used to generate the wallpaper in the xresources theme*

* It would force the wallpaper to be applied after the first mainloop iteration. This will require some extra code. This can be done later, but still requires the widget to be exposed.

[psychon]

All that I can say is: Testing this kind of code might be hard. This code gets a list of keys (we can mock that...) and produces a widget. Testing if the widget is the right one is a bit hard.