Awesome: Split up awful.util

[psychon]

See https://github.com/awesomeWM/awesome/pull/1531#issuecomment-278449926

Currently awful.util contains functionality

• To print deprecation warnings (util.deprecate and util.deprecate_class)
• To do something to handle colors that Pango does not like (util.ensure_pango_color).
• To have a modulo-operator which always returns a positive result (turns result 0 into the maximum value) (util.cycle)
• To create directories and do shell injections (util.mkdir)
• To evaluate code (util.eval)
• To XML-(un)escape a string (util.escape, util.unescape)
• To Lua-pattern-escape a string (util.quote_pattern)
• To do some (ugly?) magic to make a pattern case-insenstive (util.query_to_pattern)
• To have a thin (useless?) wrapper around loadfile (util.checkfile)
• To restart, but only if the config does not contain any Lua syntax errors (util.restart)
• To get various paths (util.get_xdg_config_home, util.get_xdg_cache_home, util.get_configuration_dir, util.get_cache_dir, util.get_themes_dir, util.get_awesome_icon_dir, util.getdir)
• To do some icon lookup (util.geticonpath, see #908)
• To check some things with the file system (file_readable, dir_readable, is_dir)
• To produce a powerset (subsets)
• To do lots of weird things to tables (all the various functions in table)
• To do poor-man's line wrapping and line counting (linewrap, linecount)
• To round a number (round)

I tried to add one bullet point to the above list per "functionality". The idea is to move each of them into a new submodule of gears (that will be quite a number). For QA reasons, I will require tests for each of these new modules. For speed reasons, these tests should be busted unit tests, i.e. in spec (unless some module really, really, really requires to be run in awesome). At the same time, we could think about dropping some of the functionality without having a replacement, if that turns out to be sensible.

[Veratil]

I was curious about this, so I started with util.ensure_pango_color and everything else sort of branches from file hits using it. It looks like it could easily be moved to gears.color without issue.

Hits for util.ensure_pango_color (and any other util.* hits in the file):

• awful/widget/taglist.lua (1 hit)
  
  
  
  • util.escape (2 hits)
  
  
• awful/widget/tasklist.lua (4 hits)
  
  
  
  • util.escape (5 hits)
  
  
• awful/prompt.lua (2 hits)
  
  
  
  • util.table.hasitem (3 hits)
  
  
  • util.mkdir (1 hit)
  
  
  • util.escape (5 hits)
  
  
  • util.deprecate (8 hits)
  
  
• menubar/init.lua (1 hit)
  
  
  
  • util.getdir("cache") (2 hits)
  
  
  • util.query_to_pattern (1 hit)
  
  
  • util.round (1 hit)
  
  
  • util.get_cache_dir (1 hit)
  
  

Info from the other util.* hits above:

• util.escape is used in all the below above, and also mentioned in a comment in wibox/widget/textbox.lua.
  
  
  
  • util.unescape is never used in any lib/* file.
  
  
  • util.mkdir only has the hit in awful/prompt.lua.
  
  
  • util.getdir("cache") returns util.get_cache_dir and only has the hit in menubar/init.lua.
  
  
  • util.round is used in the above files, and also in wibox/layout/flex.lua and wibox/layout/ratio.lua.
  
  
  • util.query_to_pattern only has the hit in menubar/init.lua. Also it calls util.quote_pattern which is only used by util.query_to_pattern.
  
  
  • util.table.* has 67 hits (not including the function names and util.table = {} lines) throughout all lib/* files.
  
  

From what I can tell most of these are easily moved. API breakage is the only real issue.

[Veratil]

For util.geticonpath I'd suggest we just get rid of that function altogether and also pull out menubar's icon_theme to a new gears module. More info on the 3 icon functions in my other comment.

[Veratil]

For the filesystem functions:

• mkdir is only used in awful/prompt.lua for saving the prompt history
• file_readable is used in awful.util.geticonpath, menubar/utils.lua, menubar/icon_theme.lua, and naughty/core.lua for finding icons
• dir_readable in menubar/utils.lua and menubar/icon_theme.lua for icons
• is_dir is unused

[Veratil]

@Elv13 @psychon @blueyed @actionless (first, sorry to tag you all like this, but you were all discussing this in the originating PR comments)

Would it be alright with you all if I started moving things out into new gears modules (or existing like the pango color function)? I know a few of you are very busy lately, so I don't mind doing a little housekeeping like this.

I'll make small PRs per module/move since I know that's preferred over large PRs.

[psychon]

Sounds good, thanks! Best idea would be to handle the easy things first (so don't start moving the deprecation warnings stuff since that really needs a rewrite and not just an easy "move this over").
Let's hope the naming of modules does not turn into a bike shedding contest. ;-)

[Veratil]

Alright, so here's my plan of action.

Housekeeping Todo List:

• [x] Deprecation functions: util.deprecate, util.deprecate_class
  New Home: gears.debug
  @Elv13 has decreed deprecation to debug

• [x] Pango color: util.ensure_pango_color
  New Home: gears.color

• [x] Math: util.round, util.cycle
  New Home: gears.math

• [x] Filesystem: util.mkdir, util.file_readable, util.dir_readable, util.is_dir
  New Home: gears.filesystem

• [x] XML strings: util.escape, util.unescape
  New Home: gears.string
  Combined with other string functions below.

• [x] Awesome-related directories: util.get_xdg_config_dir, util.get_xdg_cache_home, util.get_configuration_dir, util.get_cache_dir, util.get_themes_dir, util.get_awesome_icon_dir, util.getdir
  New Home: gears.filesystem
getdir is being renamed get_dir for consistency.

• [ ] Awesome config "safe" restart: util.restart
  New Home: TBD
  Since this are Awesome-related, possible to keep in awful?

• [x] Table-related: util.table.join, util.table.crush, util.table.from_sparse, util.table.hasitem, util.table.keys, util.table.keys_filter, util.table.reverse, util.table.clone, util.table.iterate, util.table.merge; util.subsets (could be included in Math though)
  New Home: gears.table

• [ ] Icons: util.geticonpath
  New Home: TBD
  Since we have three different functions (menubar.icon_theme.find_icon_path and menubar.utils.lookup_icon), we'll need to combine somehow or pick the best of the bunch.

• [x] String line-related: util.linecount, util.linewrap
  New Home: gears.string

• [ ] Lua-related: util.eval, util.checkfile
  New Home: TBD
  ~There is currently no code anywhere that uses the checkfile function.~ checkfile is used by restart. eval is only used in rc.lua as the exe_callback for running lua code.

• [x] String to pattern: util.query_to_pattern, util.quote_pattern
  New Home: gears.string

• [ ] awful.util.shell -> awful.spawn.shell? Will require some if awful.spawn.shell then shell = awful.spawn.shell; elseif awful.util.shell then deprecate(); shell = awful.util.shell end where currently used and a metatable magic trick in case users use it in their config(edit @Elv13)

[actionless]

:+1: it looks so well-organized, i hope you're not going to send an invoice after you'll finish

Awesome-related directories: util.get_xdg_config_dir, util.get_xdg_cache_home, util.get_configuration_dir, util.get_cache_dir, util.get_themes_dir, util.get_awesome_icon_dir, util.getdir
New Home: TBD
Since these are Awesome-related, possible to keep in awful?

get_theme_dir and others are called from the themes so it will be better to move out them from awful

[bew]

Maybe all gears package that 'replace' or 'enhance' the Lua stdlib should be in a gears.std package (so gears.std.string, gears.std.table, etc..)

I think this will allow a better separation, and avoid mixing thoses 'enhancement' with other gears stuff.

---

Additionnaly, we could add a function in this new package to inject the 'enhanced' version in the Lua stdlib:

require('gears.std').inject_stdlib()

-- use directly:
new_tbl = table.join({1, 2}, {3, 5})

[Veratil]

👍 it looks so well-organized, i hope you're not going to send an invoice after you'll finish

No promises. ;)

Awesome-related directories: util.get_xdg_config_dir, util.get_xdg_cache_home, util.get_configuration_dir, util.get_cache_dir, util.get_themes_dir, util.get_awesome_icon_dir, util.getdir
New Home: TBD
Since these are Awesome-related, possible to keep in awful?

get_theme_dir and others are called from the themes so it will be better to move out them from awful

Good note! Perhaps these could go gears.filesystem then? They do deal with that area.

---

Maybe all gears package that 'replace' or 'enhance' the Lua stdlib should be in a gears.std package (so gears.std.string, gears.std.table, etc..)

I think this will allow a better separation, and avoid mixing thoses 'enhancement' with other gears stuff.

That's not a bad idea, but we're already under the gears banner so I don't think we'd need another std level.

Additionnaly, we could add a function in this new package to inject the 'enhanced' version in the Lua stdlib:

I have mixed feelings on this. I'll let the higher ups decide that front. I'm just going to move things around for now. :)

[Veratil]

1584 combines 3 line items in the above list:

• XML strings util.escape, util.unescape
• String line-related util.linecount, util.linewrap
• String to pattern util.quote_pattern, util.query_to_pattern

---

For util.ensure_pango_color I will have to figure out how to resolve a cyclic dependency that's popping up.

[Veratil]

ensure_pango_color will have to depend on #1582 (gears.table) being merged.

Cycle (after moving function): wibox.widget.base -> awful.util -> gears.color -> gears.surface -> wibox.hierarchy -> wibox.widget.base
Solution: In wibox/widget/base.lua

• Change L11
  from local util = require("awful.util")
  to local gtable = require("gears.table")
• Change L443
  from widgets = util.table.from_sparse(widgets)
  to widgets = gtable.from_sparse(widgets)

[Veratil]

@actionless Opinion on moving the Awesome-related directory functions to gears.filesystem? Since they are related to getting filesystem directories I thing that's a sane connection.

[actionless]

sounds fine by me

[Veratil]

I checked on eval, checkfile, and restart.

• eval is only used in rc.lua for running lua code
• checkfile ~isn't used at all anywhere~ is used by restart
• restart isn't used at all anywhere

I think it might be fine to leave those where they are unless we really want to move them to a gears module.

[Veratil]

@psychon so other than the 3 items unchecked in the todo list this is pretty much done. The only thing left is to figure out how to merge the 3 icon functions. What state do we want to call this issue now?

[Elv13]

I added another one like 5s before you posted this message;)

[psychon]

I'll just consider this one to be done "good enough"-ly. "restart" can stay where it is, so can "eval". "geticonpath" has (basically) its own issue, so is not all that relevant to this one.
If you really want to deprecate awful.util.shell, then something like `setmetatable(awful.util, {__index = function(_, k) if k == "shell" then print("deprecated") return awful.shell.shell end end, __newindex = function(_, k, v) if k == "shell" then print("deprecated") awful.shell.shell = v end end })' would be needed. That's...quite some deprecation, but feel free.