From 45823f230ab6604ddf6222db27faefad1aad414f Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sat, 12 Oct 2019 21:01:18 -0400 Subject: [PATCH] capi: Move from `.data` to `._private` for the property data. This will bring the CAPI classes closer to the gears.object ones. Fixes #2897 --- common/luaclass.c | 13 ++++-- lib/awful/client.lua | 20 ++++----- lib/awful/screen.lua | 8 ++-- lib/awful/screen/dpi.lua | 30 +++++++------- lib/awful/tag.lua | 23 +++++----- lib/gears/object/properties.lua | 5 ++- lib/wibox/init.lua | 1 - tests/examples/sequences/template.lua | 2 +- tests/examples/shims/awesome.lua | 5 ++- tests/examples/shims/client.lua | 19 +++++---- tests/examples/shims/drawin.lua | 19 +++++---- tests/examples/shims/screen.lua | 5 ++- tests/examples/shims/tag.lua | 5 ++- tests/test-dpi.lua | 60 +++++++++++++-------------- 14 files changed, 119 insertions(+), 96 deletions(-) diff --git a/common/luaclass.c b/common/luaclass.c index e3533a35d..d5c784f39 100644 --- a/common/luaclass.c +++ b/common/luaclass.c @@ -437,16 +437,23 @@ luaA_class_index(lua_State *L) lua_class_property_t *prop = luaA_class_property_get(L, class, 2); - /* Is this the special 'data' property? This is available on all objects and - * thus not implemented as a lua_class_property_t. + /* This is the table storing the object private variables. */ - if (A_STREQ(attr, "data")) + if (A_STREQ(attr, "_private")) { luaA_checkudata(L, 1, class); luaA_getuservalue(L, 1); lua_getfield(L, -1, "data"); return 1; } + else if (A_STREQ(attr, "data")) + { + luaA_deprecate(L, "Use `._private` instead of `.data`"); + luaA_checkudata(L, 1, class); + luaA_getuservalue(L, 1); + lua_getfield(L, -1, "data"); + return 1; + } /* Property does exist and has an index callback */ if(prop) diff --git a/lib/awful/client.lua b/lib/awful/client.lua index ccf28325a..630733264 100644 --- a/lib/awful/client.lua +++ b/lib/awful/client.lua @@ -1167,8 +1167,8 @@ end) -- @return The property. -- @deprecated awful.client.property.get function client.property.get(c, prop) - if not c.data._persistent_properties_loaded then - c.data._persistent_properties_loaded = true + if not c._private._persistent_properties_loaded then + c._private._persistent_properties_loaded = true for p in pairs(client.data.persistent_properties_registered) do local value = c:get_xproperty("awful.client.property." .. p) if value ~= nil then @@ -1176,8 +1176,8 @@ function client.property.get(c, prop) end end end - if c.data.awful_client_properties then - return c.data.awful_client_properties[prop] + if c._private.awful_client_properties then + return c._private.awful_client_properties[prop] end end @@ -1191,14 +1191,14 @@ end -- @param value The value. -- @deprecated awful.client.property.set function client.property.set(c, prop, value) - if not c.data.awful_client_properties then - c.data.awful_client_properties = {} + if not c._private.awful_client_properties then + c._private.awful_client_properties = {} end - if c.data.awful_client_properties[prop] ~= value then + if c._private.awful_client_properties[prop] ~= value then if client.data.persistent_properties_registered[prop] then c:set_xproperty("awful.client.property." .. prop, value) end - c.data.awful_client_properties[prop] = value + c._private.awful_client_properties[prop] = value c:emit_signal("property::" .. prop) end end @@ -1216,8 +1216,8 @@ function client.property.persist(prop, kind) -- Make already-set properties persistent for c in pairs(capi.client.get()) do - if c.data.awful_client_properties and c.data.awful_client_properties[prop] ~= nil then - c:set_xproperty(xprop, c.data.awful_client_properties[prop]) + if c._private.awful_client_properties and c._private.awful_client_properties[prop] ~= nil then + c:set_xproperty(xprop, c._private.awful_client_properties[prop]) end end end diff --git a/lib/awful/screen.lua b/lib/awful/screen.lua index 139b4c154..2684a2319 100644 --- a/lib/awful/screen.lua +++ b/lib/awful/screen.lua @@ -290,7 +290,7 @@ function screen.object.get_outputs(s) local ret = {} local outputs = s._custom_outputs - or (s.data.viewport and s.data.viewport.outputs or s._outputs) + or (s._private.viewport and s._private.viewport.outputs or s._outputs) -- The reason this exists is because output with name as keys is very -- convenient for quick name lookup by the users, but inconvenient in @@ -671,9 +671,9 @@ function screen.object.split(s, ratios, mode, _geo) table.insert(ret, ns) if s then - ns.data.viewport = s.data.viewport + ns._private.viewport = s._private.viewport - if not ns.data.viewport then + if not ns._private.viewport then ns.outputs = s.outputs end end @@ -708,7 +708,7 @@ end -- @staticfct awful.screen.set_auto_dpi_enabled function screen.set_auto_dpi_enabled(enabled) for s in capi.screen do - s.data.dpi_cache = nil + s._private.dpi_cache = nil end data.autodpi = enabled end diff --git a/lib/awful/screen/dpi.lua b/lib/awful/screen/dpi.lua index 6d3186d6e..896eb6e7f 100644 --- a/lib/awful/screen/dpi.lua +++ b/lib/awful/screen/dpi.lua @@ -150,7 +150,7 @@ end -- Compute more useful viewport metadata frrom_sparse(add)om the list of output. -- @treturn table An viewport with more information. local function update_screen_viewport(s) - local viewport = s.data.viewport + local viewport = s._private.viewport if #ascreen._viewports == 0 then ascreen._viewports = update_viewports(false) @@ -171,7 +171,7 @@ local function update_screen_viewport(s) end if big_a then - viewport, s.data.viewport = big_a, big_a + viewport, s._private.viewport = big_a, big_a end end @@ -203,7 +203,7 @@ end function module.remove_screen_handler(viewport) for s in capi.screen do - if s.data.viewport and s.data.viewport.id == viewport.id then + if s._private.viewport and s._private.viewport.id == viewport.id then s:fake_remove() return end @@ -212,12 +212,12 @@ end function module.resize_screen_handler(old_viewport, new_viewport) for s in capi.screen do - if s.data.viewport and s.data.viewport.id == old_viewport.id then + if s._private.viewport and s._private.viewport.id == old_viewport.id then local ngeo = new_viewport.geometry s:fake_resize( ngeo.x, ngeo.y, ngeo.width, ngeo.height ) - s.data.viewport = new_viewport + s._private.viewport = new_viewport return end end @@ -274,32 +274,32 @@ function module._get_viewports() end local function get_dpi(s) - if s.data.dpi or s.data.dpi_cache then - return s.data.dpi or s.data.dpi_cache + if s._private.dpi or s._private.dpi_cache then + return s._private.dpi or s._private.dpi_cache end - if not s.data.viewport then + if not s._private.viewport then update_screen_viewport(s) end -- Pick a DPI (from best to worst). local dpi = ascreen._get_xft_dpi() - or (s.data.viewport and s.data.viewport.preferred_dpi or nil) + or (s._private.viewport and s._private.viewport.preferred_dpi or nil) or get_fallback_dpi() -- Pick the screen DPI depending on the `autodpi` settings. -- Historically, AwesomeWM size unit was the pixel. This assumption is -- present in a lot, if not most, user config and is why this cannot be -- enable by default for existing users. - s.data.dpi_cache = data.autodpi and dpi + s._private.dpi_cache = data.autodpi and dpi or ascreen._get_xft_dpi() or get_fallback_dpi() - return s.data.dpi_cache + return s._private.dpi_cache end local function set_dpi(s, dpi) - s.data.dpi = dpi + s._private.dpi = dpi end screen.connect_signal("request::create", module.create_screen_handler) @@ -362,7 +362,7 @@ capi.screen.connect_signal("property::_viewports", function(a) -- Drop the cache. for s in capi.screen do - s.data.dpi_cache = nil + s._private.dpi_cache = nil end capi.screen.emit_signal("property::viewports", ascreen._get_viewports()) @@ -402,11 +402,11 @@ return function(screen, d) "preferred_dpi" } do screen.object["get_"..prop] = function(s) - if not s.data.viewport then + if not s._private.viewport then update_screen_viewport(s) end - local a = s.data.viewport or {} + local a = s._private.viewport or {} return a[prop] or nil end diff --git a/lib/awful/tag.lua b/lib/awful/tag.lua index 23f7cfe3d..557fadb9c 100644 --- a/lib/awful/tag.lua +++ b/lib/awful/tag.lua @@ -283,7 +283,7 @@ function tag.add(name, props) local newtag = capi.tag{ name = name } -- Start with a fresh property table to avoid collisions with unsupported data - newtag.data.awful_tag_properties = {screen=properties.screen, index=properties.index} + newtag._private.awful_tag_properties = {screen=properties.screen, index=properties.index} newtag.activated = true @@ -402,7 +402,7 @@ function tag.object.delete(self, fallback_tag, force) end -- delete the tag - self.data.awful_tag_properties.screen = nil + self._private.awful_tag_properties.screen = nil self.activated = false -- Update all indexes @@ -1519,7 +1519,7 @@ end -- @tparam tag _tag The tag. -- @return The data table. function tag.getdata(_tag) - return _tag.data.awful_tag_properties + return _tag._private.awful_tag_properties end --- Get a tag property. @@ -1532,8 +1532,9 @@ end -- @return The property. function tag.getproperty(_tag, prop) if not _tag then return end -- FIXME: Turn this into an error? - if _tag.data.awful_tag_properties then - return _tag.data.awful_tag_properties[prop] + + if _tag._private.awful_tag_properties then + return _tag._private.awful_tag_properties[prop] end end @@ -1548,12 +1549,12 @@ end -- @param prop The property name. -- @param value The value. function tag.setproperty(_tag, prop, value) - if not _tag.data.awful_tag_properties then - _tag.data.awful_tag_properties = {} + if not _tag._private.awful_tag_properties then + _tag._private.awful_tag_properties = {} end - if _tag.data.awful_tag_properties[prop] ~= value then - _tag.data.awful_tag_properties[prop] = value + if _tag._private.awful_tag_properties[prop] ~= value then + _tag._private.awful_tag_properties[prop] = value _tag:emit_signal("property::" .. prop) end end @@ -1731,8 +1732,8 @@ capi.screen.connect_signal("removed", function(s) for _, t in pairs(s.tags) do t.activated = false - if t.data.awful_tag_properties then - t.data.awful_tag_properties.screen = nil + if t._private.awful_tag_properties then + t._private.awful_tag_properties.screen = nil end end end) diff --git a/lib/gears/object/properties.lua b/lib/gears/object/properties.lua index 8f29ee018..e58f18c79 100644 --- a/lib/gears/object/properties.lua +++ b/lib/gears/object/properties.lua @@ -54,7 +54,8 @@ function object.capi_index_fallback(class, args) end -- Use the fallback property table - return cobj.data[prop] + assert(prop ~= "_private") + return cobj._private[prop] end local setter = args.setter or function(cobj, prop, value) @@ -74,7 +75,7 @@ function object.capi_index_fallback(class, args) end -- Use the fallback property table - cobj.data[prop] = value + cobj._private[prop] = value -- Emit the signal if args.auto_emit then diff --git a/lib/wibox/init.lua b/lib/wibox/init.lua index 1f4c7ff57..0cbe610f8 100644 --- a/lib/wibox/init.lua +++ b/lib/wibox/init.lua @@ -277,7 +277,6 @@ local function new(args) return ret end - w._private = {} ret.drawin = w ret._drawable = wibox.drawable(w.drawable, { wibox = ret }, "wibox drawable (" .. object.modulename(3) .. ")") diff --git a/tests/examples/sequences/template.lua b/tests/examples/sequences/template.lua index 990f01e7b..7f4e788dd 100644 --- a/tests/examples/sequences/template.lua +++ b/tests/examples/sequences/template.lua @@ -752,7 +752,7 @@ function module.display_tags() selected = t.selected, icon = t.icon, screen = t.screen, - data = t.data, + _private = t._private, clients = t.clients, layout = t.layout, master_width_factor = t.master_width_factor, diff --git a/tests/examples/shims/awesome.lua b/tests/examples/shims/awesome.lua index 24272af5b..ac4035725 100644 --- a/tests/examples/shims/awesome.lua +++ b/tests/examples/shims/awesome.lua @@ -6,7 +6,10 @@ local gears_obj = require("gears.object") -- handlers. local function _shim_fake_class() local obj = gears_obj() - obj.data = {} + obj._private = {} + + -- Deprecated. + obj.data = obj._private local meta = { __index = function()end, diff --git a/tests/examples/shims/client.lua b/tests/examples/shims/client.lua index 758c293fc..cf2093059 100644 --- a/tests/examples/shims/client.lua +++ b/tests/examples/shims/client.lua @@ -28,11 +28,11 @@ local properties = {} for _, prop in ipairs { "maximized", "maximized_horizontal", "maximized_vertical", "fullscreen" } do properties["get_"..prop] = function(self) - return self.data[prop] or false + return self._private[prop] or false end properties["set_"..prop] = function(self, value) - self.data[prop] = value or false + self._private[prop] = value or false if value then self:emit_signal("request::geometry", prop, nil) @@ -43,12 +43,12 @@ for _, prop in ipairs { end function properties.get_screen(self) - return self.data.screen or screen[1] + return self._private.screen or screen[1] end function properties.set_screen(self, s) s = screen[s] - self.data.screen = s + self._private.screen = s if self.x < s.geometry.x or self.x > s.geometry.x+s.geometry.width then self:geometry { x = s.geometry.x } @@ -66,14 +66,17 @@ function client.gen_fake(args) local ret = gears_obj() awesome._forward_class(ret, client) - ret.data = {} + ret._private = {} ret.type = "normal" ret.valid = true ret.size_hints = {} ret.border_width = 1 ret.icon_sizes = {{16,16}} ret.name = "Example Client" - ret.data._struts = { top = 0, right = 0, left = 0, bottom = 0 } + ret._private._struts = { top = 0, right = 0, left = 0, bottom = 0 } + + -- Deprecated. + ret.data = ret._private -- This is a hack because there's a `:is_transient_for(c2)` method -- and a `transient_for` property. It will cause a stack overflow @@ -194,10 +197,10 @@ function client.gen_fake(args) function ret:struts(new) for k, v in pairs(new or {}) do - ret.data._struts[k] = v + ret._private._struts[k] = v end - return ret.data._struts + return ret._private._struts end -- Set a dummy one for now since set_screen will corrupt it. diff --git a/tests/examples/shims/drawin.lua b/tests/examples/shims/drawin.lua index 0a9780ff6..61ca495c8 100644 --- a/tests/examples/shims/drawin.lua +++ b/tests/examples/shims/drawin.lua @@ -6,7 +6,10 @@ local cairo = require("lgi").cairo local function new_drawin(_, args) local ret = gears_obj() - ret.data = {drawable = gears_obj()} + ret._private = {drawable = gears_obj()} + + -- Deprecated. + ret.data = ret._private ret.x=0 ret.y=0 @@ -31,11 +34,11 @@ local function new_drawin(_, args) } end - ret.data.drawable.valid = true - ret.data.drawable.surface = cairo.ImageSurface(cairo.Format.ARGB32, 0, 0) - ret.data.drawable.geometry = ret.geometry - ret.data.drawable.refresh = function() end - ret.data._struts = { top = 0, right = 0, left = 0, bottom = 0 } + ret._private.drawable.valid = true + ret._private.drawable.surface = cairo.ImageSurface(cairo.Format.ARGB32, 0, 0) + ret._private.drawable.geometry = ret.geometry + ret._private.drawable.refresh = function() end + ret._private._struts = { top = 0, right = 0, left = 0, bottom = 0 } for _, k in pairs{ "_buttons", "get_xproperty", "set_xproperty" } do ret[k] = function() end @@ -43,10 +46,10 @@ local function new_drawin(_, args) function ret:struts(new) for k, v in pairs(new or {}) do - ret.data._struts[k] = v + ret._private._struts[k] = v end - return ret.data._struts + return ret._private._struts end local md = setmetatable(ret, { diff --git a/tests/examples/shims/screen.lua b/tests/examples/shims/screen.lua index e94ba3a5d..b969fa3b4 100644 --- a/tests/examples/shims/screen.lua +++ b/tests/examples/shims/screen.lua @@ -31,9 +31,12 @@ local function create_screen(args) local s = gears_obj() awesome._forward_class(s, screen) - s.data = {} + s._private = {} s.valid = true + -- Deprecated. + s.data = s._private + -- Copy the geo in case the args are mutated local geo = { x = args.x , diff --git a/tests/examples/shims/tag.lua b/tests/examples/shims/tag.lua index 5a4f3cf2c..d9c092730 100644 --- a/tests/examples/shims/tag.lua +++ b/tests/examples/shims/tag.lua @@ -15,11 +15,14 @@ local function new_tag(_, args) local ret = gears_obj() awesome._forward_class(ret, tag) - ret.data = {} + ret._private = {} ret.name = tostring(args.name) or "test" ret.activated = true ret.selected = not has_selected_tag(args.screen) + -- Deprecated. + ret.data = ret._private + function ret:clients(_) --TODO handle new local list = {} for _, c in ipairs(client.get()) do diff --git a/tests/test-dpi.lua b/tests/test-dpi.lua index 6b0c28d4b..970bcffa4 100644 --- a/tests/test-dpi.lua +++ b/tests/test-dpi.lua @@ -64,27 +64,27 @@ local function fake_replay(viewports) local s = screen[k] -- Check if the extra metadata are computed. - assert(s.data.viewport.maximum_dpi) - assert(s.data.viewport.minimum_dpi) - assert(s.data.viewport.preferred_dpi) + assert(s._private.viewport.maximum_dpi) + assert(s._private.viewport.minimum_dpi) + assert(s._private.viewport.preferred_dpi) assert(s.dpi) - assert(s.maximum_dpi == s.data.viewport.maximum_dpi ) - assert(s.minimum_dpi == s.data.viewport.minimum_dpi ) - assert(s.preferred_dpi == s.data.viewport.preferred_dpi) + assert(s.maximum_dpi == s._private.viewport.maximum_dpi ) + assert(s.minimum_dpi == s._private.viewport.minimum_dpi ) + assert(s.preferred_dpi == s._private.viewport.preferred_dpi) -- Make sure it enters either the main `if` or the fallback one. - assert(s.data.viewport.minimum_dpi ~= math.huge) - assert(s.data.viewport.preferred_dpi ~= math.huge) + assert(s._private.viewport.minimum_dpi ~= math.huge) + assert(s._private.viewport.preferred_dpi ~= math.huge) -- Check the range. - assert(s.data.viewport.maximum_dpi >= s.data.viewport.minimum_dpi) - assert(s.data.viewport.preferred_dpi >= s.data.viewport.minimum_dpi) - assert(s.data.viewport.preferred_dpi <= s.data.viewport.maximum_dpi) + assert(s._private.viewport.maximum_dpi >= s._private.viewport.minimum_dpi) + assert(s._private.viewport.preferred_dpi >= s._private.viewport.minimum_dpi) + assert(s._private.viewport.preferred_dpi <= s._private.viewport.maximum_dpi) -- Check if the screen was created using the right viewport -- (order *is* relevant). - assert(#s.data.viewport.outputs == #a.outputs) - assert(s.data.viewport and s.data.viewport.id == a.id) + assert(#s._private.viewport.outputs == #a.outputs) + assert(s._private.viewport and s._private.viewport.id == a.id) end -- The original shall not be modified, the CAPI for this is read-only. @@ -314,7 +314,7 @@ end, -- Test adding an output. function() - local viewport = screen[1].data.viewport + local viewport = screen[1]._private.viewport add_output(fake_viewports[1], { name = "foobar", @@ -328,24 +328,24 @@ function() }) -- It should have been kept. - assert(viewport == screen[1].data.viewport) + assert(viewport == screen[1]._private.viewport) -- If this isn't true, then auto-dpi didn't do its job. assert(screen[1].dpi ~= saved_dpi1) -- Now that there is multiple DPIs for the same viewport, the number -- should double. - assert(#screen[1].data.viewport.outputs == 3) + assert(#screen[1]._private.viewport.outputs == 3) assert(screen[1].maximum_dpi == saved_dpi2*2) assert(screen[1].minimum_dpi == saved_dpi3/2) assert(screen[1].dpi == saved_dpi1/2) remove_output(fake_viewports[1], "leet") - assert(#screen[1].data.viewport.outputs == 2) + assert(#screen[1]._private.viewport.outputs == 2) remove_output(fake_viewports[1], "foobar") -- At this point, only 1 DPI is left. - assert(#screen[1].data.viewport.outputs == 1) + assert(#screen[1]._private.viewport.outputs == 1) assert(screen[1].maximum_dpi == saved_dpi1/2) assert(screen[1].minimum_dpi == saved_dpi1/2) assert(screen[1].dpi == saved_dpi1/2) @@ -368,25 +368,25 @@ function() } assert(screen.count() == 2) - assert(screen[1].data.viewport.id == 10) - assert(screen[2].data.viewport.id == 11) - assert(grect.are_equal(screen[1].data.viewport.geometry, screen[1].geometry)) - assert(grect.are_equal(screen[2].data.viewport.geometry, screen[2].geometry)) + assert(screen[1]._private.viewport.id == 10) + assert(screen[2]._private.viewport.id == 11) + assert(grect.are_equal(screen[1]._private.viewport.geometry, screen[1].geometry)) + assert(grect.are_equal(screen[2]._private.viewport.geometry, screen[2].geometry)) assert(#ascreen._viewports == 2) remove_viewport(10) assert(#ascreen._viewports == 1) assert(screen.count() == 1) - assert(screen[1].data.viewport.id == 11) - assert(grect.are_equal(screen[1].data.viewport.geometry, screen[1].geometry)) + assert(screen[1]._private.viewport.id == 11) + assert(grect.are_equal(screen[1]._private.viewport.geometry, screen[1].geometry)) return true end, -- Test resizing. function() - local s, sa = screen[1], screen[1].data.viewport + local s, sa = screen[1], screen[1]._private.viewport assert(screen.count() == 1) assert(#ascreen._viewports == 1) @@ -403,8 +403,8 @@ function() assert(screen.count() == 1) assert(s == screen[1]) - assert(s.data.viewport ~= sa) - assert(s.data.viewport.id == 12) + assert(s._private.viewport ~= sa) + assert(s._private.viewport.id == 12) -- Now 2 smaller (resolution) screens side by side to make sure it doesn't -- go haywire with overlapping @@ -423,7 +423,7 @@ function() assert(screen.count() == 2) assert(s == screen[1]) - assert(s.data.viewport.id == 13) + assert(s._private.viewport.id == 13) assert(s.geometry.x == 1337) assert(s.geometry.width == 680) @@ -453,8 +453,8 @@ function() } assert(screen.count() == 2) - assert(screen[1].data.viewport.id == 15) - assert(screen[2].data.viewport.id == 16) + assert(screen[1]._private.viewport.id == 15) + assert(screen[2]._private.viewport.id == 16) -- Connect custom handler and see if the internals accidently depend on -- implementation details.