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

@@ -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
+    /* This is the table storing the object private variables.
-     * thus not implemented as a lua_class_property_t.
      */
-    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)

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
+    if not c._private._persistent_properties_loaded then
-        c.data._persistent_properties_loaded = true
+        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
+    if c._private.awful_client_properties then
-        return c.data.awful_client_properties[prop]
+        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
+    if not c._private.awful_client_properties then
-        c.data.awful_client_properties = {}
+        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
+        if c._private.awful_client_properties and c._private.awful_client_properties[prop] ~= nil then
-            c:set_xproperty(xprop, c.data.awful_client_properties[prop])
+            c:set_xproperty(xprop, c._private.awful_client_properties[prop])
         end
     end
 end

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

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
+    if s._private.dpi or s._private.dpi_cache then
-        return s.data.dpi or s.data.dpi_cache
+        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

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
+    if not _tag._private.awful_tag_properties then
-        _tag.data.awful_tag_properties = {}
+        _tag._private.awful_tag_properties = {}
     end
 
-    if _tag.data.awful_tag_properties[prop] ~= value then
+    if _tag._private.awful_tag_properties[prop] ~= value then
-        _tag.data.awful_tag_properties[prop] = value
+        _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
+        if t._private.awful_tag_properties then
-            t.data.awful_tag_properties.screen = nil
+            t._private.awful_tag_properties.screen = nil
         end
     end
 end)

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

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) .. ")")

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,

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,

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.

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._private.drawable.valid    = true
-    ret.data.drawable.surface  = cairo.ImageSurface(cairo.Format.ARGB32, 0, 0)
+    ret._private.drawable.surface  = cairo.ImageSurface(cairo.Format.ARGB32, 0, 0)
-    ret.data.drawable.geometry = ret.geometry
+    ret._private.drawable.geometry = ret.geometry
-    ret.data.drawable.refresh  = function() end
+    ret._private.drawable.refresh  = function() end
-    ret.data._struts           = { top = 0, right = 0, left = 0, bottom = 0 }
+    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, {

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     ,

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

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._private.viewport.maximum_dpi)
-        assert(s.data.viewport.minimum_dpi)
+        assert(s._private.viewport.minimum_dpi)
-        assert(s.data.viewport.preferred_dpi)
+        assert(s._private.viewport.preferred_dpi)
         assert(s.dpi)
-        assert(s.maximum_dpi   == s.data.viewport.maximum_dpi  )
+        assert(s.maximum_dpi   == s._private.viewport.maximum_dpi  )
-        assert(s.minimum_dpi   == s.data.viewport.minimum_dpi  )
+        assert(s.minimum_dpi   == s._private.viewport.minimum_dpi  )
-        assert(s.preferred_dpi == s.data.viewport.preferred_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._private.viewport.minimum_dpi   ~= math.huge)
-        assert(s.data.viewport.preferred_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._private.viewport.maximum_dpi   >= s._private.viewport.minimum_dpi)
-        assert(s.data.viewport.preferred_dpi >= s.data.viewport.minimum_dpi)
+        assert(s._private.viewport.preferred_dpi >= s._private.viewport.minimum_dpi)
-        assert(s.data.viewport.preferred_dpi <= s.data.viewport.maximum_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._private.viewport.outputs == #a.outputs)
-        assert(s.data.viewport and s.data.viewport.id == a.id)
+        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[1]._private.viewport.id == 10)
-    assert(screen[2].data.viewport.id == 11)
+    assert(screen[2]._private.viewport.id == 11)
-    assert(grect.are_equal(screen[1].data.viewport.geometry, screen[1].geometry))
+    assert(grect.are_equal(screen[1]._private.viewport.geometry, screen[1].geometry))
-    assert(grect.are_equal(screen[2].data.viewport.geometry, screen[2].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(screen[1]._private.viewport.id == 11)
-    assert(grect.are_equal(screen[1].data.viewport.geometry, screen[1].geometry))
+    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._private.viewport ~= sa)
-    assert(s.data.viewport.id == 12)
+    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[1]._private.viewport.id == 15)
-    assert(screen[2].data.viewport.id == 16)
+    assert(screen[2]._private.viewport.id == 16)
 
     -- Connect custom handler and see if the internals accidently depend on
     -- implementation details.