Avoid loading duplicate .desktop files in menubar

[dyfrgi]

Currently, if a user copies a .desktop file into some other directory, they will get duplicate entries in menubar. This PR changes that so that only one entry is kept.

The Desktop Entry Specification says that entries have an id derived from their path.

To determine the ID of a desktop file, make its full path relative to the $XDG_DATA_DIRS component in which the desktop file is installed, remove the "applications/" prefix, and turn '/' into '-'.

Only the first file found with a given id, following the order of directories in $XDG_DATA_DIRS, should be used. This PR changes the logic used by menubar to do exactly this. This allows users to e.g. override a system defined .desktop file by putting a changed one in ~/.local/share/applications/. Changing '/' into '-' allows overriding a file stored inside a directory hierarchy of the system without creating directories in .local.

To do this, the GIO Async context was hoisted above the iteration through directories as otherwise following directory precedence would be more complex.

There is one divergence from the spec in this implementation. The spec says:

If the desktop file is not installed in an applications subdirectory of one of the $XDG_DATA_DIRS components, it does not have an ID.

In this implementation, we scan through menu_bar.all_menu_dirs, and all these directories are treated the same, regardless of whether they are "an applications subdirectory of one of the $XDG_DATA_DIRS components". Some users set this, though it's pretty uncommon. I think this is unlikely to cause the few people who use this to add more directories any problems with accidentally-ignored .desktop files.

Fixes #1816.

[psychon]

Looks fine to me. I looked at this locally with git diff -w and that makes the diff a lot smaller. :-)

[psychon]

Isn't this line redundant? entry came from result, so is already referenced there.

[dyfrgi]

Good point. My poor Lua knowledge is showing here, about what variables are references and which are not. The Lua reference manual set me straight. I'll remove this.

[codecov]

Codecov Report

Merging #3822 (d87bb3e) into master (b13ac3e) will decrease coverage by 0.03%.
The diff coverage is 67.16%.

❗ Current head d87bb3e differs from pull request most recent head 99db3a8. Consider uploading reports for the commit 99db3a8 to get more accurate results

@@            Coverage Diff             @@
##           master    #3822      +/-   ##
==========================================
- Coverage   90.99%   90.97%   -0.03%     
==========================================
  Files         900      901       +1     
  Lines       57506    57535      +29     
==========================================
+ Hits        52329    52340      +11     
- Misses       5177     5195      +18     

Flag	Coverage Δ
gcov	90.97% <67.16%> (-0.03%)	⬇️
luacov	93.65% <67.16%> (-0.06%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
lib/menubar/menu_gen.lua	73.41% <43.75%> (-26.59%)	⬇️
lib/menubar/utils.lua	88.11% <55.55%> (-3.11%)	⬇️
lib/awful/client/focus.lua	92.04% <100.00%> (+0.18%)	⬆️
tests/test-focus-bydirection.lua	100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

[dyfrgi]

I started working on some unit tests for this, but I gave up due to the lack of support for async() in busted 2.x. https://github.com/dyfrgi/awesome/compare/desktop-file-id...dyfrgi:awesome:menu_gen-unit-test?expand=1 shows the starting point, though. With busted 2.0, we'd need a custom test runner. I don't quite have it in me to dig into lgi + gio async work and busted custom test runners for this PR, so I hope that manual testing is sufficient.

It would probably be possible to refactor things so as to make this easier to test without handling async in the test runner, but it's not obvious to me.

[actionless]

there are several tests which have accommodation for that (via re-running test function few times), and using count argument as the counter of how many times test function have been attempted to execute, ex:
https://github.com/awesomeWM/awesome/blob/master/tests/test-client-shape.lua#L8

[Elv13]

@dyfrgi Do you still plan to work on the test? As @actionless said, this is better done using an integration test suite since it has a full event loop and retry function if they return nil.

[dyfrgi]

Whoops, I never actually sent the reply I drafted. Yes, I plan to work on it. Have done some work on it a couple weeks back but only got it to 95%. I'll post something for review soon.