Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

menubar: consider XDG_HOME_DIR and XDG_DATA_DIRS #1639

Closed
wants to merge 5 commits into from

Conversation

romildo
Copy link
Contributor

@romildo romildo commented Mar 7, 2017

The freedesktop specifications let desktop files to be stored in
different directories indicated by the environment variables
XDG_DATA_HOME and XDG_DATA_DIRS. Only use the default value for these
variables if the variables are not defined.

This is important for systmes like NixOS which does not follow the LFS
and installs files differently.

@codecov
Copy link

codecov bot commented Mar 7, 2017

Codecov Report

Merging #1639 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1639      +/-   ##
==========================================
+ Coverage   81.77%   81.81%   +0.04%     
==========================================
  Files         301      301              
  Lines       18695    18722      +27     
==========================================
+ Hits        15287    15318      +31     
+ Misses       3408     3404       -4
Flag Coverage Δ
#functionaltests 63.31% <100%> (+0.17%)
#samples 65.52% <24%> (-0.09%)
#unittests 75.35% <24%> (-0.3%)
Impacted Files Coverage Δ
lib/gears/filesystem.lua 66.66% <100%> (+7.14%)
lib/menubar/menu_gen.lua 52.05% <100%> (+1.35%)
lib/gears/table.lua 100% <100%> (ø)
lib/gears/string.lua 77.5% <100%> (+8.53%)
lib/awful/tooltip.lua 86.01% <0%> (+0.42%)
lib/gears/debug.lua 66.66% <0%> (+5.26%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b73f9e9...26db184. Read the comment docs.

@Veratil
Copy link
Contributor

Veratil commented Mar 7, 2017

Your split and map functions could be useful outside of this, along with determining the xdg_data_* variables.

For modules in PR waiting:
split could move to gears.string
map could move to gears.table
xdg_data_home and xdg_data_dirs could move to functions in gears.filesystem

Then add the few requires() and this would be as simple as:

local gtable = require("gears.table")
local gstring = require("gears.string")
local gfs = require("gears.filesystem")
menu_gen.all_menu_dirs = gtable.map (
    function(dir) return dir .. '/applications/' end,
    gstring.split(gfs.get_xdg_data_home() .. ':' .. gfs.get_xdg_data_dirs(), ':'))

with the added benefit of everybody else getting to use it if they need. :)

@Veratil
Copy link
Contributor

Veratil commented Mar 15, 2017

@romildo gears.string and gears.table have been merged if you want to add those functions to their respective modules. gears.filesystem is approved and going to be merged soon, too.

Don't forget the documentation! 😁

The freedesktop specifications let desktop files be stored in
different directories indicated by the environment variables
XDG_DATA_HOME and XDG_DATA_DIRS.

Only use the default value for these variables if the variables are
not defined.

This is important for systmes like NixOS which does not follow the LFS
and installs files differently.
@romildo
Copy link
Contributor Author

romildo commented Mar 18, 2017

@Veratil I have reworked the PR so that the helper functions are defined in the appropriate module in the gears library.

@Elv13
Copy link
Member

Elv13 commented Mar 18, 2017

The CI found 1 problem

Checking lib/gears/string.lua                     1 warning

    lib/gears/string.lua:89:38: trailing whitespace in a comment

-- @tparam function f the function to be applied to each value on the table
-- @tparam table tbl the container table whose values will be operated on
-- @treturn table Return a table of the results
function gtable.map(f, tbl)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you swap both parameters for consistency with the other function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In defense of the argument order, this is traditionally the way the map function arguments are in all languages.
Python: map(function_to_apply, list_of_inputs)
Haskell: map :: (a -> b) -> [a] -> [b]
PHP: array array_map ( callable $callback , array $array1 [, array $... ] )
Even the Lua example has (func, tbl)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, then

@Elv13 Elv13 added this to the next: pull requests to be merged soon milestone Mar 18, 2017
@Elv13
Copy link
Member

Elv13 commented Mar 27, 2017

Merged

@Elv13 Elv13 closed this Mar 27, 2017
@romildo romildo deleted the xdg-data-dirs branch March 27, 2017 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants