From 620241e05678ccdf63db4eee4eb5b41016537814 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Fri, 12 Jul 2019 17:56:19 -0400 Subject: [PATCH 01/25] doc: Fix the type of naughty.notification.id. Copy/paste error. --- lib/naughty/notification.lua | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/naughty/notification.lua b/lib/naughty/notification.lua index 5ad11de6..b00b880b 100644 --- a/lib/naughty/notification.lua +++ b/lib/naughty/notification.lua @@ -74,8 +74,7 @@ local notification = {} -- This is the equivalent to a PID as allows external applications to select -- notifications. -- @property id --- @param string --- @see title +-- @param number --- Text of the notification. -- From e076bc664e0764a3d3a0164dabd9b58d334355f4 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 14 Jul 2019 00:03:12 -0400 Subject: [PATCH 02/25] naughty: Bump the SPEC version compliance to v1.2. * action icons * persistence * residence * categories * animated icons * more ways to get icons In addition, the commit also tries its best to attach notifications to objects using various dubious semi compliant hints or the DBus PID. It works often enough to be useful. --- lib/naughty/core.lua | 46 ++++++- lib/naughty/dbus.lua | 259 ++++++++++++++++++++++++++++++++--- lib/naughty/notification.lua | 202 +++++++++++++++++++++++++-- tests/run.sh | 2 +- 4 files changed, 474 insertions(+), 35 deletions(-) diff --git a/lib/naughty/core.lua b/lib/naughty/core.lua index 4985ade5..ac4d29a4 100644 --- a/lib/naughty/core.lua +++ b/lib/naughty/core.lua @@ -134,10 +134,36 @@ gtable.crush(naughty, require("naughty.constants")) -- @property auto_reset_timeout -- @tparam[opt=true] boolean auto_reset_timeout +--- Enable or disable naughty ability to claim to support animations. +-- +-- When this is true, applications which query `naughty` feature support +-- will see that animations are supported. Note that there is *very little* +-- support for this and enabled it will cause bugs. +-- +-- @property image_animations_enabled +-- @param[opt=false] boolean + +--- Enable or disable the persistent notifications. +-- +-- This is very annoying when using `naughty.layout.box` popups, but tolerable +-- when using `naughty.list.notifications`. +-- +-- Note that enabling this **does nothing** in `naughty` itself. The timeouts +-- are still honored and notifications still destroyed. It is the user +-- responsibility to disable the dismiss timer. However, this tells the +-- applications that notification persistence is supported so they might +-- stop using systray icons for the sake of displaying or other changes like +-- that. +-- +-- @property persistence_enabled +-- @param[opt=false] boolean + local properties = { - suspended = false, - expiration_paused = false, - auto_reset_timeout= true, + suspended = false, + expiration_paused = false, + auto_reset_timeout = true, + image_animations_enabled = false, + persistence_enabled = false, } --TODO v5 Deprecate the public `naughty.notifications` (to make it private) @@ -514,6 +540,18 @@ end -- including, but not limited to, all `naughty.notification` properties. -- @signal request::preset +--- Emitted when an action requires an icon it doesn't know. +-- +-- The implementation should look in the icon theme for an action icon or +-- provide something natively. +-- +-- If an icon is found, the handler must set the `icon` property on the `action` +-- object to a path or a `gears.surface`. +-- +-- @signal request::icon +-- @tparam naughty.action action The action. +-- @tparam string icon_name The icon name. + -- Register a new notification object. local function register(notification, args) -- Add the some more properties @@ -564,6 +602,8 @@ local function set_index_miss(_, key, value) if not value then resume() end + + naughty.emit_signal("property::"..key) else rawset(naughty, key, value) end diff --git a/lib/naughty/dbus.lua b/lib/naughty/dbus.lua index 6672de97..27c83b3f 100644 --- a/lib/naughty/dbus.lua +++ b/lib/naughty/dbus.lua @@ -14,6 +14,7 @@ local string = string local capi = { awesome = awesome } local gtable = require("gears.table") local gsurface = require("gears.surface") +local gdebug = require("gears.debug") local protected_call = require("gears.protected_call") local lgi = require("lgi") local cairo, Gio, GLib, GObject = lgi.cairo, lgi.Gio, lgi.GLib, lgi.GObject @@ -28,6 +29,10 @@ local cst = require("naughty.constants") local nnotif = require("naughty.notification") local naction = require("naughty.action") +local capabilities = { + "body", "body-markup", "icon-static", "actions", "action-icons" +} + --- Notification library, dbus bindings local dbus = { config = {} } @@ -37,8 +42,8 @@ local bus_connection -- DBUS Notification constants -- https://developer.gnome.org/notification-spec/#urgency-levels local urgency = { - low = "\0", - normal = "\1", + low = "\0", + normal = "\1", critical = "\2" } @@ -123,7 +128,7 @@ end local notif_methods = {} function notif_methods.Notify(sender, object_path, interface, method, parameters, invocation) - local appname, replaces_id, icon, title, text, actions, hints, expire = + local appname, replaces_id, app_icon, title, text, actions, hints, expire = unpack(parameters.value) local args = {} @@ -167,14 +172,26 @@ function notif_methods.Notify(sender, object_path, interface, method, parameters notification:destroy(cst.notification_closed_reason.dismissed_by_user) end elseif action_id ~= nil and action_text ~= nil then + local a = naction { name = action_text, - position = action_id, + id = action_id, + position = (i - 1)/2 + 1, } + -- Right now `gears` doesn't have a great icon implementation + -- and `naughty` doesn't depend on `menubar`, so delegate the + -- icon "somewhere" using a request. + if hints["action-icons"] and action_id ~= "" then + naughty.emit_signal("request::icon", a, action_id) + end + a:connect_signal("invoked", function() sendActionInvoked(notification.id, action_id) - notification:destroy(cst.notification_closed_reason.dismissed_by_user) + + if not notification.resident then + notification:destroy(cst.notification_closed_reason.dismissed_by_user) + end end) table.insert(args.actions, a) @@ -189,22 +206,34 @@ function notif_methods.Notify(sender, object_path, interface, method, parameters member = method, sender = sender, bus = "session" } if not preset.callback or (type(preset.callback) == "function" and - preset.callback(legacy_data, appname, replaces_id, icon, title, text, actions, hints, expire)) then - if icon ~= "" then - args.icon = icon - elseif hints.icon_data or hints.image_data then + preset.callback(legacy_data, appname, replaces_id, app_icon, title, text, actions, hints, expire)) then + + + if app_icon ~= "" then + args.app_icon = app_icon + end + + if hints.icon_data or hints.image_data or hints["image-data"] then -- Icon data is a bit complex and hence needs special care: -- .value breaks with the array of bytes (ay) that we get here. -- So, bypass it and look up the needed value differently - local icon_data + local icon_condidates = {} for k, v in parameters:get_child_value(7 - 1):pairs() do - if k == "icon_data" then - icon_data = v - elseif k == "image_data" and icon_data == nil then - icon_data = v + if k == "image-data" then + icon_condidates[1] = v -- not deprecated + break + elseif k == "image_data" then -- deprecated + icon_condidates[2] = v + elseif k == "icon_data" then -- deprecated + icon_condidates[3] = v end end + -- The order is mandated by the spec. + local icon_data = icon_condidates[1] + or icon_condidates[2] + or icon_condidates[3] + -- icon_data is an array: -- 1 -> width -- 2 -> height @@ -218,20 +247,87 @@ function notif_methods.Notify(sender, object_path, interface, method, parameters -- GVariant.data to get that as an LGI byte buffer. That one can -- then by converted to a string via its __tostring metamethod. local data = tostring(icon_data:get_child_value(7 - 1).data) - args.icon = convert_icon(icon_data[1], icon_data[2], icon_data[3], icon_data[6], data) + args.image = convert_icon(icon_data[1], icon_data[2], icon_data[3], icon_data[6], data) + + -- Convert all animation frames. + if naughty.image_animations_enabled then + args.images = {args.image} + + if #icon_data > 7 then + for frame=8, #icon_data do + data = tostring(icon_data:get_child_value(frame-1).data) + + table.insert( + args.images, + convert_icon( + icon_data[1], + icon_data[2], + icon_data[3], + icon_data[6], + data + ) + ) + end + end + end end + + -- Alternate ways to set the icon. The specs recommends to allow both + -- the icon and image to co-exist since they serve different purpose. + -- However in case the icon isn't specified, use the image. + args.image = args.image + or hints["image-path"] -- not deprecated + or hints["image_path"] -- deprecated + + if naughty.image_animations_enabled then + args.images = args.images or {} + end + if replaces_id and replaces_id ~= "" and replaces_id ~= 0 then args.replaces_id = replaces_id end if expire and expire > -1 then args.timeout = expire / 1000 end + args.freedesktop_hints = hints + -- Not very pretty, but given the current format is documented in the + -- public API... well, whatever... + if hints and hints.urgency then + for name, key in pairs(urgency) do + local b = string.char(hints.urgency) + if key == b then + args.urgency = name + end + end + end + + args.urgency = args.urgency or "normal" + -- Try to update existing objects when possible notification = naughty.get_by_id(replaces_id) if notification then + if not notification._private._unique_sender then + -- If this happens, the notification is either trying to + -- highjack content created within AwesomeWM or it is garbage + -- to begin with. + gdebug.print_warning( + "A notification has been received, but tried to update ".. + "the content of a notification it does not own." + ) + elseif notification._private._unique_sender ~= sender then + -- Nothing says you cannot and some scripts may do it + -- accidentally, but this is rather unexpected. + gdebug.print_warning( + "Notification "..notification.title.." is being updated".. + "by a different DBus connection ("..sender.."), this is ".. + "suspicious. The original connection was ".. + notification._private._unique_sender + ) + end + for k, v in pairs(args) do if k == "destroy" then k = "destroy_cb" end notification[k] = v @@ -240,6 +336,9 @@ function notif_methods.Notify(sender, object_path, interface, method, parameters -- Even if no property changed, restart the timeout. notification:reset_timeout() else + -- Only set the sender for new notifications. + args._unique_sender = sender + notification = nnotif(args) end @@ -261,16 +360,14 @@ end function notif_methods.GetServerInformation(_, _, _, _, _, invocation) -- name of notification app, name of vender, version, specification version invocation:return_value(GLib.Variant("(ssss)", { - "naughty", "awesome", capi.awesome.version, "1.0" + "naughty", "awesome", capi.awesome.version, "1.2" })) end function notif_methods.GetCapabilities(_, _, _, _, _, invocation) -- We actually do display the body of the message, we support , -- and in the body and we handle static (non-animated) icons. - invocation:return_value(GLib.Variant("(as)", { - { "body", "body-markup", "icon-static", "actions" } - })) + invocation:return_value(GLib.Variant("(as)", {capabilities})) end local function method_call(_, sender, object_path, interface, method, parameters, invocation) @@ -330,6 +427,101 @@ local function on_bus_acquire(conn, _) GObject.Closure(method_call)) end +local bus_proxy, pid_for_unique_name = nil, {} + +Gio.DBusProxy.new_for_bus( + Gio.BusType.SESSION, + Gio.DBusProxyFlags.DO_NOT_LOAD_PROPERTIES, + nil, + "org.freedesktop.DBus", + "/org/freedesktop/DBus", + "org.freedesktop.DBus", + nil, + function(proxy) + bus_proxy = proxy + end, + nil +) + +--- Get the clients associated with a notification. +-- +-- Note that is based on the process PID. PIDs roll over, so don't use this +-- with very old notifications. +-- +-- Also note that some multi-process application can use a different process +-- for the clients and the service used to send the notifications. +-- +-- Since this is based on PIDs, it is impossible to know which client sent the +-- notification if the process has multiple clients (windows). Using the +-- `client.type` can be used to further filter this list into more probable +-- candidates (tooltips, menus and dialogs are unlikely to send notifications). +-- +-- @tparam naughty.notification notif A notification object. +-- @treturn table A table with all associated clients. +function dbus.get_clients(notif) + -- First, the trivial case, but I never found an app that implements it. + -- It isn't standardized, but mentioned as possible. + local win_id = notif.freedesktop_hints and (notif.freedesktop_hints.window_ID + or notif.freedesktop_hints["window-id"] + or notif.freedesktop_hints.windowID + or notif.freedesktop_hints.windowid) + + if win_id then + for _, c in ipairs(client.get()) do + if c.window_id == win_id then + return {win_id} + end + end + end + + -- Less trivial, but mentioned in the spec. Note that this isn't + -- recommended by the spec, let alone mandatory. It is mentioned it can + -- exist. This wont work with Flatpak or Snaps. + local pid = notif.freedesktop_hints and ( + notif.freedesktop_hints.PID or notif.freedesktop_hints.pid + ) + + if ((not bus_proxy) or not notif._private._unique_sender) and not pid then + return {} + end + + if (not pid) and (not pid_for_unique_name[notif._private._unique_sender]) then + local owner = GLib.Variant("(s)", {notif._private._unique_sender}) + + -- It is sync, but this isn't done very often and since it is DBus + -- daemon itself, it is very responsive. Doing this using the async + -- variant would cause the clients to be unavailable in the notification + -- rules. + pid = bus_proxy:call_sync("GetConnectionUnixProcessID", + owner, + Gio.DBusCallFlags.NONE, + -1 + ) + + if (not pid) or (not pid.value) then return {} end + + pid = pid.value and pid.value[1] + + if not pid then return {} end + + pid_for_unique_name[notif._private._unique_sender] = pid + end + + pid = pid or pid_for_unique_name[notif._private._unique_sender] + + if not pid then return {} end + + local ret = {} + + for _, c in ipairs(client.get()) do + if c.pid == pid then + table.insert(ret, c) + end + end + + return ret +end + local function on_name_acquired(conn, _) bus_connection = conn end @@ -345,6 +537,35 @@ Gio.bus_own_name(Gio.BusType.SESSION, "org.freedesktop.Notifications", -- For testing dbus._notif_methods = notif_methods +local function remove_capability(cap) + for k, v in ipairs(capabilities) do + if v == cap then + table.remove(capabilities, k) + break + end + end +end + +-- Update the capabilities. +naughty.connect_signal("property::persistence_enabled", function() + remove_capability("persistence") + + if naughty.persistence_enabled then + table.insert(capabilities, "persistence") + end +end) +naughty.connect_signal("property::image_animations_enabled", function() + remove_capability("icon-multi") + remove_capability("icon-static") + + table.insert(capabilities, naughty.persistence_enabled + and "icon-multi" or "icon-static" + ) +end) + +-- For the tests. +dbus._capabilities = capabilities + return dbus -- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80 diff --git a/lib/naughty/notification.lua b/lib/naughty/notification.lua index b00b880b..56f7b22a 100644 --- a/lib/naughty/notification.lua +++ b/lib/naughty/notification.lua @@ -15,12 +15,13 @@ -- @copyright 2017 Emmanuel Lepage Vallee -- @coreclassmod naughty.notification --------------------------------------------------------------------------- -local gobject = require("gears.object") -local gtable = require("gears.table") -local timer = require("gears.timer") -local cst = require("naughty.constants") -local naughty = require("naughty.core") -local gdebug = require("gears.debug") +local gobject = require("gears.object") +local gtable = require("gears.table") +local gsurface = require("gears.surface") +local timer = require("gears.timer") +local cst = require("naughty.constants") +local naughty = require("naughty.core") +local gdebug = require("gears.debug") local notification = {} @@ -95,6 +96,72 @@ local notification = {} -- @property timeout -- @param number +--- The notification urgency level. +-- +-- The default urgency levels are: +-- +-- * low +-- * normal +-- * critical +-- +-- @property urgency +-- @param string + +--- The notification category. +-- +-- The category should be named using the `x-vendor.class.name` naming scheme or +-- use one of the default categories: +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +-- +--
NameDescription
deviceA generic device-related notification that +-- doesn't fit into any other category.
device.addedA device, such as a USB device, was added to the system.
device.errorA device had some kind of error.
device.removedA device, such as a USB device, was removed from the system.
emailA generic e-mail-related notification that doesn't fit into +-- any other category.
email.arrivedA new e-mail notification.
email.bouncedA notification stating that an e-mail has bounced.
imA generic instant message-related notification that doesn't fit into +-- any other category.
im.errorAn instant message error notification.
im.receivedA received instant message notification.
networkA generic network notification that doesn't fit into any other +-- category.
network.connectedA network connection notification, such as successful +-- sign-on to a network service.
+-- This should not be confused with device.added for new network devices.
network.disconnectedA network disconnected notification. This should not +-- be confused with
+-- device.removed for disconnected network devices.
network.errorA network-related or connection-related error.
presenceA generic presence change notification that doesn't fit into any +-- other category,
+-- such as going away or idle.
presence.offlineAn offline presence change notification.
presence.onlineAn online presence change notification.
transferA generic file transfer or download notification that doesn't +-- fit into any other category.
transfer.completeA file transfer or download complete notification.
transfer.errorA file transfer or download error.
+-- +-- @property category +-- @tparam string|nil category + +--- True if the notification should be kept when an action is pressed. +-- +-- By default, invoking an action will destroy the notification. Some actions, +-- like the "Snooze" action of alarm clock, will cause the notification to +-- be updated with a date further in the future. +-- +-- @property resident +-- @param[opt=false] boolean + --- Delay in seconds after which hovered popup disappears. -- @property hover_timeout -- @param number @@ -143,14 +210,59 @@ local notification = {} -- @property font -- @param string ---- Path to icon. +--- "All in one" way to access the default image or icon. +-- +-- A notification can provide a combination of an icon, a static image, or if +-- enabled, a looping animation. Add to that the ability to import the icon +-- information from the client or from a `.desktop` file, there is multiple +-- conflicting sources of "icons". +-- +-- On the other hand, the vast majority of notifications don't multiple or +-- ambiguous sources of icons. This property will pick the first of the +-- following. +-- +-- * The `image`. +-- * The `app_icon`. +-- * The `icon` from a client with `normal` type. +-- * The `icon` of a client with `dialog` type. +-- -- @property icon -- @tparam string|surface icon +-- @see app_icon +-- @see image --- Desired icon size in px. -- @property icon_size -- @param number +--- The icon provided in the `app_icon` field of the DBus notification. +-- +-- This should always be either the URI (path) to an icon or a valid XDG +-- icon name to be fetched from the theme. +-- +-- @property app_icon +-- @param string + +--- The notification image. +-- +-- This is usually provided as a `gears.surface` object. The image is used +-- instead of the `app_icon` by notification assets which are auto-generated +-- or stored elsewhere than the filesystem (databases, web, Android phones, etc). +-- +-- @property image +-- @tparam string|surface image + +--- The notification (animated) images. +-- +-- Note that calling this without first setting +-- `naughty.image_animations_enabled` to true will throw an exception. +-- +-- Also note that there is *zero* support for this anywhere else in `naughty` +-- and very, very few applications support this. +-- +-- @property images +-- @tparam nil|table images + --- Foreground color. -- --@DOC_awful_notification_fg_EXAMPLE@ @@ -284,11 +396,20 @@ local notification = {} -- @property ignore_suspend If set to true this notification -- will be shown even if notifications are suspended via `naughty.suspend`. +--- A list of clients associated with this notification. +-- +-- When used with DBus notifications, this returns all clients sharing the PID +-- of the notification sender. Note that this is highly unreliable. +-- Applications that use a different process to send the notification or +-- applications (and scripts) calling the `notify-send` command wont have any +-- client. +-- +-- @property clients +-- @param table + --FIXME remove the screen attribute, let the handlers decide -- document all handler extra properties ---FIXME add methods such as persist - --- Destroy notification by notification object. -- -- @method destroy @@ -398,7 +519,8 @@ local properties = { "fg" , "bg" , "height" , "border_color" , "shape" , "opacity" , "margin" , "ignore_suspend", "destroy" , "preset" , "callback", "actions" , - "run" , "id" , "ignore" , "auto_reset_timeout" + "run" , "id" , "ignore" , "auto_reset_timeout", + "urgency" , "image" , "images" , } for _, prop in ipairs(properties) do @@ -425,12 +547,68 @@ for _, prop in ipairs(properties) do if reset then self:reset_timeout() end - - return end end +local hints_default = { + urgency = "normal", + resident = false, +} + +for _, prop in ipairs { "category", "resident" } do + notification["get_"..prop] = notification["get_"..prop] or function(self) + return self._private[prop] or ( + self._private.freedesktop_hints and self._private.freedesktop_hints[prop] + ) or hints_default[prop] + end + + notification["set_"..prop] = notification["set_"..prop] or function(self, value) + self._private[prop] = value + self:emit_signal("property::"..prop, value) + end +end + +function notification.get_icon(self) + if self._private.icon then + return self._private.icon == "" and nil or self._private.icon + elseif self.image and self.image ~= "" then + return self.image + elseif self._private.app_icon and self._private.app_icon ~= "" then + return self._private.app_icon + end + + local clients = notification.get_clients(self) + + for _, c in ipairs(clients) do + if c.type == "normal" then + self._private.icon = gsurface(c.icon) + return self._private.icon + end + end + + for _, c in ipairs(clients) do + if c.type == "dialog" then + self._private.icon = gsurface(c.icon) + return self._private.icon + end + end + + return nil +end + +function notification.get_clients(self) + -- Clients from the future don't send notification, it's useless to reload + -- the list over and over. + if self._private.clients then return self._private.clients end + + if not self._private._unique_sender then return {} end + + self._private.clients = require("naughty.dbus").get_clients(self) + + return self._private.clients +end + --TODO v6: remove this local function convert_actions(actions) gdebug.deprecate( diff --git a/tests/run.sh b/tests/run.sh index 99cb2e2b..bfb85a03 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -250,7 +250,7 @@ for f in $tests; do error="$(echo "$error" | grep -vE ".{19} W: awesome: (Can't read color .* from GTK)" || true)" if [[ $fail_on_warning ]]; then # Filter out ignored warnings. - error="$(echo "$error" | grep -vE ".{19} W: awesome: (a_glib_poll|Cannot reliably detect EOF|beautiful: can't get colorscheme from xrdb|Can't read color .* from GTK+3 theme)" || true)" + error="$(echo "$error" | grep -vE ".{19} W: awesome: (a_glib_poll|Cannot reliably detect EOF|beautiful: can't get colorscheme from xrdb|Can't read color .* from GTK+3 theme|A notification|Notification)" || true)" fi if [[ -n "$error" ]]; then color_red From 2ff4a5294c08ed8244a0be84a8ae5e81de0e5b59 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 14 Jul 2019 17:16:47 -0400 Subject: [PATCH 03/25] naughty.widget.icon: Use `surface.load_silently`. Icons can be names from the icon theme. This wont load until a better icon theme API is added (since this module doesn't depend on menubar). --- lib/naughty/widget/icon.lua | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/naughty/widget/icon.lua b/lib/naughty/widget/icon.lua index ccb60b6b..4f7dbc40 100644 --- a/lib/naughty/widget/icon.lua +++ b/lib/naughty/widget/icon.lua @@ -19,6 +19,7 @@ local imagebox = require("wibox.widget.imagebox") local gtable = require("gears.table") local beautiful = require("beautiful") +local gsurface = require("gears.surface") local dpi = require("beautiful.xresources").apply_dpi local icon = {} @@ -85,7 +86,11 @@ function icon:set_notification(notif) self._private.icon_changed_callback) end - self:set_image(notif.icon) + local icn = gsurface.load_silently(notif.icon) + + if icn then + self:set_image(icn) + end self._private.notification = notif @@ -141,7 +146,11 @@ local function new(args) gtable.crush(tb, icon, true) function tb._private.icon_changed_callback() - tb:set_image(tb._private.notification.icon) + local icn = gsurface.load_silently(tb._private.notification.icon) + + if icn then + tb:set_image() + end end if args.notification then From 047245fd033f935ab89bda6b604d440b8ff718db Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 14 Jul 2019 23:52:07 -0400 Subject: [PATCH 04/25] notification: Add a default urgency value. --- lib/naughty/constants.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/naughty/constants.lua b/lib/naughty/constants.lua index 693c08a7..59b12e58 100644 --- a/lib/naughty/constants.lua +++ b/lib/naughty/constants.lua @@ -55,7 +55,8 @@ ret.config.defaults = { ontop = true, margin = dpi(5), border_width = dpi(1), - position = "top_right" + position = "top_right", + urgency = "normal", } ret.notification_closed_reason = { From efbc707279c26f8a7dd6f229a02e9298384f4c36 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Apr 2019 16:08:31 -0400 Subject: [PATCH 05/25] naughty: Move the boilerplate rc.lua error handling to naughty. This removes the imperative "mutex" logic from rc.lua, where it doesn't belong. It also makes it closer to the "vision" of making `rc.lua` fully modular. --- lib/naughty/core.lua | 6 ++++++ lib/naughty/init.lua | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/lib/naughty/core.lua b/lib/naughty/core.lua index ac4d29a4..adf1cbe9 100644 --- a/lib/naughty/core.lua +++ b/lib/naughty/core.lua @@ -512,6 +512,12 @@ function naughty.set_expiration_paused(p) end end +--- Emitted when an error occurred and requires attention. +-- @signal request::display_error +-- @tparam string message The error message. +-- @tparam boolean startup If the error occurred during the initial loading of +-- rc.lua (and thus caused the fallback to kick in). + --- Emitted when a notification is created. -- @signal added -- @tparam naughty.notification notification The notification object diff --git a/lib/naughty/init.lua b/lib/naughty/init.lua index e94f366d..f360d531 100644 --- a/lib/naughty/init.lua +++ b/lib/naughty/init.lua @@ -5,6 +5,7 @@ --------------------------------------------------------------------------- local naughty = require("naughty.core") +local capi = {awesome = awesome} if dbus then naughty.dbus = require("naughty.dbus") end @@ -17,6 +18,27 @@ naughty.container = require("naughty.container") naughty.action = require("naughty.action") naughty.notification = require("naughty.notification") +-- Handle runtime errors during startup +if capi.awesome.startup_errors then + naughty.emit_signal( + "request::display_error", capi.awesome.startup_errors, true + ) +end + +-- Handle runtime errors after startup +do + local in_error = false + capi.awesome.connect_signal("debug::error", function (err) + -- Make sure we don't go into an endless error loop + if in_error then return end + in_error = true + + naughty.emit_signal("request::display_error", tostring(err), false) + + in_error = false + end) +end + return naughty -- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80 From c10312b5114724f97dc23902b5c383857515723e Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sat, 3 Aug 2019 01:43:48 -0400 Subject: [PATCH 06/25] naughty: Fix a race condition which cause startup errors to be missed. This commit will be "reverted" when the screen refactoring is merged since it has a better fix. --- lib/naughty/init.lua | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/naughty/init.lua b/lib/naughty/init.lua index f360d531..1df33c37 100644 --- a/lib/naughty/init.lua +++ b/lib/naughty/init.lua @@ -20,9 +20,13 @@ naughty.notification = require("naughty.notification") -- Handle runtime errors during startup if capi.awesome.startup_errors then - naughty.emit_signal( - "request::display_error", capi.awesome.startup_errors, true - ) + -- Wait until `rc.lua` is executed before creating the notifications. + -- Otherwise nothing is handling them (yet). + awesome.connect_signal("startup", function() + naughty.emit_signal( + "request::display_error", capi.awesome.startup_errors, true + ) + end) end -- Handle runtime errors after startup From 41149ed335bd9cd8b3cc6d59312b21b2658b2485 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 5 May 2019 23:00:58 -0400 Subject: [PATCH 07/25] class: Share the module level signal system implementation. Avoid adding another copy. --- lib/gears/object.lua | 36 ++++++++++++++++++++++++++++++++++++ lib/naughty/core.lua | 33 +++++++-------------------------- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/lib/gears/object.lua b/lib/gears/object.lua index a7be7f35..b8b5d645 100644 --- a/lib/gears/object.lua +++ b/lib/gears/object.lua @@ -134,6 +134,42 @@ function object:emit_signal(name, ...) end end +function object._setup_class_signals(t) + local conns = {} + + function t.connect_signal(name, func) + assert(name) + conns[name] = conns[name] or {} + table.insert(conns[name], func) + end + + --- Emit a notification signal. + -- @tparam string name The signal name. + -- @param ... The signal callback arguments + function t.emit_signal(name, ...) + assert(name) + for _, func in pairs(conns[name] or {}) do + func(...) + end + end + + --- Disconnect a signal from a source. + -- @tparam string name The name of the signal + -- @tparam function func The attached function + -- @treturn boolean If the disconnection was successful + function t.disconnect_signal(name, func) + for k, v in ipairs(conns[name] or {}) do + if v == func then + table.remove(conns[name], k) + return true + end + end + return false + end + + return conns +end + local function get_miss(self, key) local class = rawget(self, "_class") diff --git a/lib/naughty/core.lua b/lib/naughty/core.lua index adf1cbe9..802848b9 100644 --- a/lib/naughty/core.lua +++ b/lib/naughty/core.lua @@ -14,9 +14,10 @@ -- Package environment local capi = { screen = screen } -local gdebug = require("gears.debug") -local screen = require("awful.screen") -local gtable = require("gears.table") +local gdebug = require("gears.debug") +local screen = require("awful.screen") +local gtable = require("gears.table") +local gobject = require("gears.object") local naughty = {} @@ -249,7 +250,7 @@ function naughty.suspend() properties.suspended = true end -local conns = {} +local conns = gobject._setup_class_signals(naughty) --- Connect a global signal on the notification engine. -- @@ -261,41 +262,21 @@ local conns = {} -- -- @tparam string name The name of the signal -- @tparam function func The function to attach +-- @staticfct naughty.connect_signal -- @usage naughty.connect_signal("added", function(notif) -- -- do something -- end) --- @staticfct naughty.connect_signal -function naughty.connect_signal(name, func) - assert(name) - conns[name] = conns[name] or {} - table.insert(conns[name], func) -end --- Emit a notification signal. -- @tparam string name The signal name. -- @param ... The signal callback arguments -- @staticfct naughty.emit_signal -function naughty.emit_signal(name, ...) - assert(name) - for _, func in pairs(conns[name] or {}) do - func(...) - end -end --- Disconnect a signal from a source. -- @tparam string name The name of the signal -- @tparam function func The attached function --- @treturn boolean If the disconnection was successful -- @staticfct naughty.disconnect_signal -function naughty.disconnect_signal(name, func) - for k, v in ipairs(conns[name] or {}) do - if v == func then - table.remove(conns[name], k) - return true - end - end - return false -end +-- @treturn boolean If the disconnection was successful local function resume() properties.suspended = false From c36d35756f6814f9db70d9848eddd3e9b14bdd1e Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 15 Jul 2019 00:55:34 -0400 Subject: [PATCH 08/25] naughty: Begin to deprecate the presets. The old preset code had a primitive implementation of the rule API used in `naughty.dbus`. Now that `gears.matcher` is extracted from `awful.rules`, it is possible to share the code. The first step is to only enable the old API when the new `request::preset` isn't connected. This is the same way the legacy popup is only enabled when nothing is connected to `request::display`. --- lib/naughty/constants.lua | 15 +++++++++ lib/naughty/core.lua | 15 +++++++-- lib/naughty/dbus.lua | 15 +-------- lib/naughty/layout/box.lua | 3 +- lib/naughty/layout/legacy.lua | 2 +- lib/naughty/notification.lua | 59 +++++++++++++++++++++++++---------- 6 files changed, 72 insertions(+), 37 deletions(-) diff --git a/lib/naughty/constants.lua b/lib/naughty/constants.lua index 59b12e58..6e8a1a52 100644 --- a/lib/naughty/constants.lua +++ b/lib/naughty/constants.lua @@ -48,6 +48,21 @@ ret.config.presets = { }, } +ret.config._urgency = { + low = "\0", + normal = "\1", + critical = "\2" +} + +ret.config.mapping = { + {{urgency = ret.config._urgency.low }, ret.config.presets.low}, --compat + {{urgency = ret.config._urgency.normal }, ret.config.presets.normal}, --compat + {{urgency = ret.config._urgency.critical}, ret.config.presets.critical}, --compat + {{urgency = "low" }, ret.config.presets.low}, + {{urgency = "normal" }, ret.config.presets.normal}, + {{urgency = "critical"}, ret.config.presets.critical}, +} + ret.config.defaults = { timeout = 5, text = "", diff --git a/lib/naughty/core.lua b/lib/naughty/core.lua index 802848b9..e82d21b4 100644 --- a/lib/naughty/core.lua +++ b/lib/naughty/core.lua @@ -224,7 +224,9 @@ local function update_index(n) remove_from_index(n) -- Add to the index again - local s = get_screen(n.screen or n.preset.screen or screen.focused()) + local s = get_screen(n.screen + or (n.preset and n.preset.screen) + or screen.focused()) naughty.notifications[s] = naughty.notifications[s] or {} table.insert(naughty.notifications[s][n.position], n) end @@ -399,6 +401,11 @@ function naughty.get_has_display_handler() return conns["request::display"] and #conns["request::display"] > 0 or false end +-- Presets are "deprecated" when notification rules are used. +function naughty.get__has_preset_handler() + return conns["request::preset"] and #conns["request::preset"] > 0 or false +end + --- Set new notification timeout. -- -- This function is deprecated, use `notification:reset_timeout(new_timeout)`. @@ -545,7 +552,9 @@ local function register(notification, args) rawset(notification, "get_suspended", get_suspended) --TODO v5 uncouple the notifications and the screen - local s = get_screen(args.screen or notification.preset.screen or screen.focused()) + local s = get_screen(args.screen + or (notification.preset and notification.preset.screen) + or screen.focused()) -- insert the notification to the table table.insert(naughty._active, notification) @@ -560,7 +569,7 @@ local function register(notification, args) naughty.emit_signal("added", notification, args) end - assert(rawget(notification, "preset")) + assert(rawget(notification, "preset") or naughty._has_preset_handler) naughty.emit_signal("property::active") diff --git a/lib/naughty/dbus.lua b/lib/naughty/dbus.lua index 27c83b3f..67be524c 100644 --- a/lib/naughty/dbus.lua +++ b/lib/naughty/dbus.lua @@ -12,7 +12,6 @@ local pairs = pairs local type = type local string = string local capi = { awesome = awesome } -local gtable = require("gears.table") local gsurface = require("gears.surface") local gdebug = require("gears.debug") local protected_call = require("gears.protected_call") @@ -56,11 +55,7 @@ local urgency = { -- @tfield table 2 normal urgency -- @tfield table 3 critical urgency -- @table config.mapping -dbus.config.mapping = { - {{urgency = urgency.low}, cst.config.presets.low}, - {{urgency = urgency.normal}, cst.config.presets.normal}, - {{urgency = urgency.critical}, cst.config.presets.critical} -} +dbus.config.mapping = cst.mapping local function sendActionInvoked(notificationId, action) if bus_connection then @@ -149,14 +144,6 @@ function notif_methods.Notify(sender, object_path, interface, method, parameters if appname ~= "" then args.appname = appname end - for _, obj in pairs(dbus.config.mapping) do - local filter, preset = obj[1], obj[2] - if (not filter.urgency or filter.urgency == hints.urgency) and - (not filter.category or filter.category == hints.category) and - (not filter.appname or filter.appname == appname) then - args.preset = gtable.join(args.preset, preset) - end - end local preset = args.preset or cst.config.defaults local notification if actions then diff --git a/lib/naughty/layout/box.lua b/lib/naughty/layout/box.lua index c1936329..79ab70e9 100644 --- a/lib/naughty/layout/box.lua +++ b/lib/naughty/layout/box.lua @@ -148,8 +148,7 @@ end local function init(self, notification) local args = self._private.args - local preset = notification.preset - assert(preset) + local preset = notification.preset or {} local position = args.position or notification.position or beautiful.notification_position or preset.position or "top_right" diff --git a/lib/naughty/layout/legacy.lua b/lib/naughty/layout/legacy.lua index a2b79149..484d4651 100644 --- a/lib/naughty/layout/legacy.lua +++ b/lib/naughty/layout/legacy.lua @@ -309,7 +309,7 @@ function naughty.default_notification_handler(notification, args) return end - local preset = notification.preset + local preset = notification.preset or {} local text = args.message or args.text or preset.message or preset.text local title = args.title or preset.title local s = get_screen(args.screen or preset.screen or screen.focused()) diff --git a/lib/naughty/notification.lua b/lib/naughty/notification.lua index 56f7b22a..fd13beb5 100644 --- a/lib/naughty/notification.lua +++ b/lib/naughty/notification.lua @@ -653,6 +653,33 @@ local function convert_actions(actions) end end +-- The old API used monkey-patched variable presets. +-- +-- Monkey-patched anything is always an issue and prevent module from safely +-- doing anything without stepping on each other foot. In the background, +-- presets were selected with a rule-like API anyway. +local function select_legacy_preset(n, args) + for _, obj in pairs(cst.config.mapping) do + local filter, preset = obj[1], obj[2] + if (not filter.urgency or filter.urgency == args.urgency) and + (not filter.category or filter.category == args.category) and + (not filter.appname or filter.appname == args.appname) then + args.preset = gtable.join(args.preset or {}, preset) + end + end + + -- gather variables together + rawset(n, "preset", gtable.join( + cst.config.defaults or {}, + args.preset or cst.config.presets.normal or {}, + rawget(n, "preset") or {} + )) + + for k, v in pairs(n.preset) do + n._private[k] = v + end +end + --- Create a notification. -- -- @tab args The argument table containing any of the arguments below. @@ -734,22 +761,17 @@ local function create(args) -- Avoid modifying the original table local private = {} + rawset(n, "_private", private) - -- gather variables together - rawset(n, "preset", gtable.join( - cst.config.defaults or {}, - args.preset or cst.config.presets.normal or {}, - rawget(n, "preset") or {} - )) + -- Allow extensions to create override the preset with custom data + if not naughty._has_preset_handler then + select_legacy_preset(n, args) + end if is_old_action then convert_actions(args.actions) end - for k, v in pairs(n.preset) do - private[k] = v - end - for k, v in pairs(args) do private[k] = v end @@ -767,27 +789,30 @@ local function create(args) -- It's an automatic property n.is_expired = false - rawset(n, "_private", private) - gtable.crush(n, notification, true) n.id = n.id or notification._gen_next_id() - -- Allow extensions to create override the preset with custom data - naughty.emit_signal("request::preset", n, args) - -- Register the notification before requesting a widget n:emit_signal("new", args) + -- The rules are attached to this. + if naughty._has_preset_handler then + naughty.emit_signal("request::preset", n, args) + end + -- Let all listeners handle the actual visual aspects - if (not n.ignore) and (not n.preset.ignore) then + if (not n.ignore) and ((not n.preset) or n.preset.ignore ~= true) then naughty.emit_signal("request::display" , n, args) naughty.emit_signal("request::fallback", n, args) end -- Because otherwise the setter logic would not be executed if n._private.timeout then - n:set_timeout(n._private.timeout or n.preset.timeout) + n:set_timeout(n._private.timeout + or (n.preset and n.preset.timeout) + or cst.config.timeout + ) end return n From b9d19988b94dc8c7d1249b34cd7930ca34d18bda Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Sun, 28 Apr 2019 16:09:51 -0400 Subject: [PATCH 09/25] rc.lua: Replace the error handling code with request::display_error. It is easier to understand, shorter and more modular. --- awesomerc.lua | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/awesomerc.lua b/awesomerc.lua index 5d2bd2c1..94840722 100644 --- a/awesomerc.lua +++ b/awesomerc.lua @@ -20,34 +20,15 @@ local hotkeys_popup = require("awful.hotkeys_popup") require("awful.hotkeys_popup.keys") -- {{{ Error handling --- @DOC_ERROR_HANDLING@ -- Check if awesome encountered an error during startup and fell back to -- another config (This code will only ever execute for the fallback config) -if awesome.startup_errors then +naughty.connect_signal("request::display_error", function(message, startup) naughty.notification { - preset = naughty.config.presets.critical, - title = "Oops, there were errors during startup!", - message = awesome.startup_errors + urgency = "critical", + title = "Oops, an error happened"..(startup and " during startup!" or "!"), + message = message } -end - --- Handle runtime errors after startup -do - local in_error = false - awesome.connect_signal("debug::error", function (err) - -- Make sure we don't go into an endless error loop - if in_error then return end - in_error = true - - naughty.notification { - preset = naughty.config.presets.critical, - title = "Oops, an error happened!", - message = tostring(err) - } - - in_error = false - end) -end +end) -- }}} -- {{{ Variable definitions From 84ae41422f04525bd10005945fb388c286f004a2 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 15 Jul 2019 02:12:20 -0400 Subject: [PATCH 10/25] notification: Prevent the timer to be started early in the constructor. Otherwise the timer will be started before the preset is applied. --- lib/naughty/notification.lua | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/naughty/notification.lua b/lib/naughty/notification.lua index fd13beb5..c2bfa04c 100644 --- a/lib/naughty/notification.lua +++ b/lib/naughty/notification.lua @@ -439,7 +439,11 @@ end function notification:reset_timeout(new_timeout) if self.timer then self.timer:stop() end - self.timeout = new_timeout or self.timeout + -- Do not set `self.timeout` to `self.timeout` since that would create the + -- timer before the constructor ends. + if new_timeout and self.timer then + self.timeout = new_timeout or self.timeout + end if self.timer and not self.timer.started then self.timer:start() From 78c616c3580867a9d9d84e186557e2ea19f48b3b Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 15 Jul 2019 16:25:49 -0400 Subject: [PATCH 11/25] notification: Allow nil to be set to the timeout to rmeove it --- lib/naughty/notification.lua | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/naughty/notification.lua b/lib/naughty/notification.lua index c2bfa04c..81205386 100644 --- a/lib/naughty/notification.lua +++ b/lib/naughty/notification.lua @@ -457,6 +457,8 @@ function notification:set_id(new_id) end function notification:set_timeout(timeout) + timeout = timeout or 0 + local die = function (reason) if reason == cst.notification_closed_reason.expired then self.is_expired = true From e77ca1f4d8bd4e9d4a37533681250fcc70dbd520 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 15 Jul 2019 17:05:28 -0400 Subject: [PATCH 12/25] naughty.widget.legacy: Use values from the notification objects. Previously it used the `args` table passed to the constructor. This will not work with rules since they modify the object, not the args. --- lib/naughty/layout/legacy.lua | 81 +++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 36 deletions(-) diff --git a/lib/naughty/layout/legacy.lua b/lib/naughty/layout/legacy.lua index 484d4651..21e97444 100644 --- a/lib/naughty/layout/legacy.lua +++ b/lib/naughty/layout/legacy.lua @@ -297,6 +297,14 @@ end naughty.connect_signal("destroyed", cleanup) +-- Don't copy paste the list of fallback, it is hard to spot mistakes. +local function get_value(notification, args, preset, prop) + return notification[prop] -- set by the rules + or args[prop] -- magic and undocumented, but used by the legacy API + or preset[prop] --deprecated + or beautiful["notification_"..prop] -- from the theme +end + function naughty.default_notification_handler(notification, args) -- This is a fallback for users whose config doesn't have the newer -- `request::display` section. @@ -310,9 +318,14 @@ function naughty.default_notification_handler(notification, args) end local preset = notification.preset or {} - local text = args.message or args.text or preset.message or preset.text - local title = args.title or preset.title - local s = get_screen(args.screen or preset.screen or screen.focused()) + + local title = get_value(notification, args, preset, "title" ) + local text = get_value(notification, args, preset, "message") + or args.text or preset.text + + local s = get_screen( + get_value(notification, args, preset, "screen") or screen.focused() + ) if not s then local err = "naughty.notify: there is no screen available to display the following notification:" @@ -321,45 +334,41 @@ function naughty.default_notification_handler(notification, args) return end - local timeout = args.timeout or preset.timeout - local icon = args.icon or preset.icon - local icon_size = args.icon_size or preset.icon_size - or beautiful.notification_icon_size - local ontop = args.ontop or preset.ontop - local hover_timeout = args.hover_timeout or preset.hover_timeout - local position = args.position or preset.position - local actions = args.actions - local destroy_cb = args.destroy + local timeout = get_value(notification, args, preset, "timeout" ) + local icon = get_value(notification, args, preset, "icon" ) + local icon_size = get_value(notification, args, preset, "icon_size" ) + local ontop = get_value(notification, args, preset, "ontop" ) + local hover_timeout = get_value(notification, args, preset, "hover_timeout") + local position = get_value(notification, args, preset, "position" ) + + local actions = notification.actions or args.actions + local destroy_cb = args.destroy notification.screen = s notification.destroy_cb = destroy_cb notification.timeout = timeout -- beautiful - local font = args.font or preset.font or beautiful.notification_font or - beautiful.font or capi.awesome.font - local fg = args.fg or preset.fg or - beautiful.notification_fg or beautiful.fg_normal or '#ffffff' - local bg = args.bg or preset.bg or - beautiful.notification_bg or beautiful.bg_normal or '#535d6c' - local border_color = args.border_color or preset.border_color or - beautiful.notification_border_color or beautiful.bg_focus or '#535d6c' - local border_width = args.border_width or preset.border_width or - beautiful.notification_border_width - local shape = args.shape or preset.shape or - beautiful.notification_shape - local width = args.width or preset.width or - beautiful.notification_width - local height = args.height or preset.height or - beautiful.notification_height - local max_width = args.max_width or preset.max_width or - beautiful.notification_max_width - local max_height = args.max_height or preset.max_height or - beautiful.notification_max_height - local margin = args.margin or preset.margin or - beautiful.notification_margin - local opacity = args.opacity or preset.opacity or - beautiful.notification_opacity + local font = get_value(notification, args, preset, "font" ) + or beautiful.font or capi.awesome.font + + local fg = get_value(notification, args, preset, "fg" ) + or beautiful.fg_normal or '#ffffff' + + local bg = get_value(notification, args, preset, "bg" ) + or beautiful.bg_normal or '#535d6c' + + local border_color = get_value(notification, args, preset, "border_color") + or beautiful.bg_focus or '#535d6c' + + local border_width = get_value(notification, args, preset, "border_width") + local shape = get_value(notification, args, preset, "shape" ) + local width = get_value(notification, args, preset, "width" ) + local height = get_value(notification, args, preset, "height" ) + local max_width = get_value(notification, args, preset, "max_width" ) + local max_height = get_value(notification, args, preset, "max_height" ) + local margin = get_value(notification, args, preset, "margin" ) + local opacity = get_value(notification, args, preset, "opacity" ) notification.position = position From f8cbb549136d106329addf18e6159e381de35b6b Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 15 Jul 2019 17:44:58 -0400 Subject: [PATCH 13/25] naughty: Expose 3 previously internal properties. * app_name: To be used in filters when no clients are found. * max_width: Allow to set it from the rules, it might be different when a `widget_template` is used. * widget_template: Now it can be set from the rules without further boilerplate code. --- lib/naughty/dbus.lua | 5 ++++- lib/naughty/layout/box.lua | 9 +++++++-- lib/naughty/notification.lua | 28 +++++++++++++++++++++++++++- 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/naughty/dbus.lua b/lib/naughty/dbus.lua index 67be524c..3c8d2089 100644 --- a/lib/naughty/dbus.lua +++ b/lib/naughty/dbus.lua @@ -141,9 +141,12 @@ function notif_methods.Notify(sender, object_path, interface, method, parameters return end end + if appname ~= "" then - args.appname = appname + args.appname = appname --TODO v6 Remove this. + args.app_name = appname end + local preset = args.preset or cst.config.defaults local notification if actions then diff --git a/lib/naughty/layout/box.lua b/lib/naughty/layout/box.lua index 79ab70e9..a4846794 100644 --- a/lib/naughty/layout/box.lua +++ b/lib/naughty/layout/box.lua @@ -17,6 +17,7 @@ local popup = require("awful.popup") local awcommon = require("awful.widget.common") local placement = require("awful.placement") local abutton = require("awful.button") +local dpi = require("beautiful").xresources.apply_dpi local default_widget = require("naughty.widget._default") @@ -136,9 +137,13 @@ end local function generate_widget(args, n) local w = wibox.widget.base.make_widget_from_value( - args.widget_template or default_widget + args.widget_template or (n and n.widget_template) or default_widget ) + if w.set_width then + w:set_width(n.max_width or beautiful.notification_max_width or dpi(500)) + end + -- Call `:set_notification` on all children awcommon._set_common_property(w, "notification", n or args.notification) @@ -224,7 +229,7 @@ local function new(args) new_args = args and setmetatable(new_args, {__index = args}) or new_args -- Generate the box before the popup is created to avoid the size changing - new_args.widget = generate_widget(new_args) + new_args.widget = generate_widget(new_args, new_args.notification) local ret = popup(new_args) ret._private.args = new_args diff --git a/lib/naughty/notification.lua b/lib/naughty/notification.lua index 81205386..72ecab22 100644 --- a/lib/naughty/notification.lua +++ b/lib/naughty/notification.lua @@ -407,6 +407,31 @@ local notification = {} -- @property clients -- @param table +--- The maximum popup width. +-- +-- Some notifications have overlong message, cap them to this width. Note that +-- this is ignored by `naughty.list.notifications` because it delegate this +-- decision to the layout. +-- +-- @property[opt=500] max_width +-- @param number + +--- The application name specified by the notification. +-- +-- This can be anything. It is usually less relevant than the `clients` +-- property, but can sometime to specified for remote or headless notifications. +-- In these case, it helps to triage and detect the notification from the rules. +-- @property app_name +-- @param string + +--- The widget template used to represent the notification. +-- +-- Some notifications, such as chat messages or music applications are better +-- off with a specialized notification widget. +-- +-- @property widget_template +-- @param table + --FIXME remove the screen attribute, let the handlers decide -- document all handler extra properties @@ -526,7 +551,8 @@ local properties = { "shape" , "opacity" , "margin" , "ignore_suspend", "destroy" , "preset" , "callback", "actions" , "run" , "id" , "ignore" , "auto_reset_timeout", - "urgency" , "image" , "images" , + "urgency" , "image" , "images" , "widget_template", + "max_width", "app_name", } for _, prop in ipairs(properties) do From e524f93baa05db2233b47adc624b8b5671f1aa0a Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 17 Jul 2019 00:26:44 -0400 Subject: [PATCH 14/25] notification.action: Allow actions to be shared by multiple notification The reason is that if actions are provided by rules, only one instance exist. It was a mistake to couple actions with their notifications. It could not work reliably and has to be removed. The commit also change the notification action storage to be a copy instead of the original table. This allows to append actions (not part of this commit) without risking adding them to the wrong notification. **WARNING** This break an unreleased API by removing the `notification` property of an action. --- lib/naughty/action.lua | 56 +++++++++++++------------------- lib/naughty/layout/legacy.lua | 6 ++-- lib/naughty/list/actions.lua | 12 +++---- lib/naughty/notification.lua | 61 +++++++++++++++++++++++++++++------ 4 files changed, 81 insertions(+), 54 deletions(-) diff --git a/lib/naughty/action.lua b/lib/naughty/action.lua index dcd23409..3ed71114 100644 --- a/lib/naughty/action.lua +++ b/lib/naughty/action.lua @@ -2,7 +2,8 @@ --- A notification action. -- -- A notification can have multiple actions to chose from. This module allows --- to manage such actions. +-- to manage such actions. An action object can be shared by multiple +-- notifications. -- -- @author Emmanuel Lepage Vallee <elv1313@gmail.com> -- @copyright 2019 Emmanuel Lepage Vallee @@ -29,8 +30,8 @@ local action = {} -- If the action is selected. -- --- Only a single action can be selected per notification. It will be applied --- when `my_notification:apply()` is called. +-- Only a single action can be selected per notification. This is useful to +-- implement keyboard navigation. -- -- @property selected -- @param boolean @@ -50,10 +51,6 @@ local action = {} -- @property icon_only -- @param[opt=false] boolean ---- The notification. --- @property notification --- @tparam naughty.notification notification - --- When a notification is invoked. -- @signal invoked @@ -64,10 +61,7 @@ end function action:set_selected(value) self._private.selected = value self:emit_signal("property::selected", value) - - if self._private.notification then - self._private.notification:emit_signal("property::actions") - end + self:emit_signal("_changed") --TODO deselect other actions from the same notification end @@ -79,15 +73,12 @@ end function action:set_position(value) self._private.position = value self:emit_signal("property::position", value) - - if self._private.notification then - self._private.notification:emit_signal("property::actions") - end + self:emit_signal("_changed") --TODO make sure the position is unique end -for _, prop in ipairs { "name", "icon", "notification", "icon_only" } do +for _, prop in ipairs { "name", "icon", "icon_only" } do action["get_"..prop] = function(self) return self._private[prop] end @@ -95,22 +86,18 @@ for _, prop in ipairs { "name", "icon", "notification", "icon_only" } do action["set_"..prop] = function(self, value) self._private[prop] = value self:emit_signal("property::"..prop, value) - - -- Make sure widgets with as an actionlist is updated. - if self._private.notification then - self._private.notification:emit_signal("property::actions") - end + self:emit_signal("_changed") end end -local set_notif = action.set_notification - -function action.set_notification(self, value) - local old = self._private.notification - set_notif(self, value) - if old then - old:emit_signal("property::actions") - end +--TODO v4.5, remove this. +function action.set_notification() + -- It didn't work because it prevented actions defined in the rules to be + -- in multiple notifications at once. + assert( + false, + "Setting a notification object was a bad idea and is now forbidden" + ) end --- Execute this action. @@ -118,11 +105,12 @@ end -- This only emits the `invoked` signal. -- -- @method invoke -function action:invoke() - assert(self._private.notification, - "Cannot invoke an action without a notification") - - self:emit_signal("invoked") +-- @tparam[opt={}] naughty.notification notif A notification object on which +-- the action was invoked. If a notification is shared by many object (like +-- a "mute" or "snooze" action added to all notification), calling `:invoke()` +-- without adding the `notif` context will cause unexpected results. +function action:invoke(notif) + self:emit_signal("invoked", notif) end local function new(_, args) diff --git a/lib/naughty/layout/legacy.lua b/lib/naughty/layout/legacy.lua index 21e97444..ee99c083 100644 --- a/lib/naughty/layout/legacy.lua +++ b/lib/naughty/layout/legacy.lua @@ -430,12 +430,10 @@ function naughty.default_notification_handler(notification, args) actionmarginbox:buttons(gtable.join( button({ }, 1, function() - action:invoke() - notification:destroy() + action:invoke(notification) end), button({ }, 3, function() - action:invoke() - notification:destroy() + action:invoke(notification) end) )) actionslayout:add(actionmarginbox) diff --git a/lib/naughty/list/actions.lua b/lib/naughty/list/actions.lua index cc37d703..ff9c8e5e 100644 --- a/lib/naughty/list/actions.lua +++ b/lib/naughty/list/actions.lua @@ -117,10 +117,6 @@ local module = {} -- @tparam gears.surface|string action_bgimage_selected -- @see gears.surface -local default_buttons = gtable.join( - abutton({ }, 1, function(a) a:invoke() end) -) - local props = {"shape_border_color", "bg_image" , "fg", "shape_border_width", "underline", "bg", "shape", "icon_size", } @@ -176,7 +172,7 @@ local function update(self) awcommon.list_update( self._private.layout, - default_buttons, + self._private.default_buttons, function(o) return wb_label(o, self) end, self._private.data, self._private.notification.actions, @@ -273,7 +269,7 @@ end --- Create an action list. -- -- @tparam table args --- @tparam naughty.notification args.notification The notification/ +-- @tparam naughty.notification args.notification The notification. -- @tparam widget args.base_layout The action layout. -- @tparam table args.style Override the beautiful values. -- @tparam boolean args.style.underline_normal @@ -312,6 +308,10 @@ local function new(_, args) update_style(wdg) + wdg._private.default_buttons = gtable.join( + abutton({ }, 1, function(a) a:invoke(args.notification) end) + ) + return wdg end diff --git a/lib/naughty/notification.lua b/lib/naughty/notification.lua index 72ecab22..b1a89b08 100644 --- a/lib/naughty/notification.lua +++ b/lib/naughty/notification.lua @@ -641,6 +641,34 @@ function notification.get_clients(self) return self._private.clients end +function notification.set_actions(self, new_actions) + for _, a in ipairs(self._private.actions or {}) do + a:disconnect_signal("_changed", self._private.action_cb ) + a:disconnect_signal("invoked" , self._private.invoked_cb) + end + + -- Clone so `append_actions` doesn't add unwanted actions to other + -- notifications. + self._private.actions = gtable.clone(new_actions, false) + + for _, a in ipairs(self._private.actions or {}) do + a:connect_signal("_changed", self._private.action_cb ) + a:connect_signal("invoked" , self._private.invoked_cb) + end + + self:emit_signal("property::actions", new_actions) + + -- When a notification is updated over dbus or by setting a property, + -- it is usually convenient to reset the timeout. + local reset = ((not self.suspended) + and self.auto_reset_timeout ~= false + and naughty.auto_reset_timeout) + + if reset then + self:reset_timeout() + end +end + --TODO v6: remove this local function convert_actions(actions) gdebug.deprecate( @@ -808,21 +836,34 @@ local function create(args) private[k] = v end - -- notif.actions should not be nil to allow cheching if there is actions - -- using the shorthand `if #notif.actions > 0 then` - private.actions = private.actions or {} - - -- Make sure the action are for this notification. Sharing actions with - -- multiple notification is not supported. - for _, a in ipairs(private.actions) do - a.notification = n - end - -- It's an automatic property n.is_expired = false gtable.crush(n, notification, true) + -- Always emit property::actions when any of the action change to allow + -- some widgets to be updated without over complicated built-in tracking + -- of all options. + function n._private.action_cb() n:emit_signal("property::actions") end + + -- Listen to action press and destroy non-resident notifications. + function n._private.invoked_cb(a, notif) + if (not notif) or notif == n then + n:emit_signal("invoked", a) + + if not n.resident then + n:destroy(cst.notification_closed_reason.dismissed_by_user) + end + end + end + + -- notif.actions should not be nil to allow checking if there is actions + -- using the shorthand `if #notif.actions > 0 then` + private.actions = {} + if args.actions then + notification.set_actions(n, args.actions) + end + n.id = n.id or notification._gen_next_id() -- Register the notification before requesting a widget From 1bd2c1977e52f983e7319e47c69d557c077c492d Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 17 Jul 2019 00:40:40 -0400 Subject: [PATCH 15/25] notification: Add an `append_actions` method. The name is self explanatory, it adds more actions to a notification. One of the use case is adding a snooze/reming_me action. Another one is "mute similar notifications". --- lib/naughty/notification.lua | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/naughty/notification.lua b/lib/naughty/notification.lua index b1a89b08..985c1589 100644 --- a/lib/naughty/notification.lua +++ b/lib/naughty/notification.lua @@ -669,6 +669,21 @@ function notification.set_actions(self, new_actions) end end +--- Add more actions to the notification. +-- @method append_actions +-- @tparam table new_actions + +function notification:append_actions(new_actions) + self._private.actions = self._private.actions or {} + + for _, a in ipairs(new_actions or {}) do + a:connect_signal("_changed", self._private.action_cb ) + a:connect_signal("invoked" , self._private.invoked_cb) + table.insert(self._private.actions, a) + end + +end + --TODO v6: remove this local function convert_actions(actions) gdebug.deprecate( From 30b42905daa374a7588a80bc28d65c0c3dc7fa70 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 17 Jul 2019 01:50:57 -0400 Subject: [PATCH 16/25] notification: Fix a typo in the closed_reason API name. It wasn't part of a release and nobody uses that anyway. --- lib/naughty/constants.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/naughty/constants.lua b/lib/naughty/constants.lua index 6e8a1a52..def1f6eb 100644 --- a/lib/naughty/constants.lua +++ b/lib/naughty/constants.lua @@ -81,7 +81,7 @@ ret.notification_closed_reason = { dismissedByUser = 2, --TODO v5 remove this undocumented legacy constant dismissed_by_user = 2, dismissedByCommand = 3, --TODO v5 remove this undocumented legacy constant - dismissed_by_vommand = 3, + dismissed_by_command = 3, undefined = 4 } From 7d609fb69ba19c116c405e35769ffa1dcf6788ae Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 17 Jul 2019 01:53:56 -0400 Subject: [PATCH 17/25] tests: Test the notification spec v1.2 "resident notification" support. If a notification is resident, invoking an action should not destroy it. If an action is not resident, invoking an action destroys it. --- tests/test-naughty-legacy.lua | 63 +++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/test-naughty-legacy.lua b/tests/test-naughty-legacy.lua index 12968191..14875fe3 100644 --- a/tests/test-naughty-legacy.lua +++ b/tests/test-naughty-legacy.lua @@ -719,6 +719,69 @@ table.insert(steps, function() return true end) +-- Test adding actions, resident mode and action sharing. +table.insert(steps, function() + local n1 = naughty.notification { + title = "foo", + message = "bar", + timeout = 25000, + resident = true, + actions = { naughty.action { name = "a1" } } + } + + local n2 = naughty.notification { + title = "foo", + message = "bar", + resident = true, + timeout = 25000, + actions = { naughty.action { name = "a2" } } + } + + local is_called = {} + + n1:connect_signal("invoked", function() is_called[1] = true end) + n2:connect_signal("invoked", function() is_called[2] = true end) + + n1.actions[1]:invoke(n1) + n2.actions[1]:invoke(n2) + + assert(is_called[1]) + assert(is_called[2]) + assert(not n1._private.is_destroyed) + assert(not n2._private.is_destroyed) + + local shared_a = naughty.action { name = "a3" } + + n1:append_actions {shared_a} + n2:append_actions {shared_a} + + n1:connect_signal("invoked", function() is_called[3] = true end) + n2:connect_signal("invoked", function() is_called[4] = true end) + + assert(n1.actions[2] == shared_a) + assert(n2.actions[2] == shared_a) + + shared_a:invoke(n1) + + assert(is_called[3]) + assert(not is_called[4]) + assert(not n1._private.is_destroyed) + assert(not n2._private.is_destroyed) + + n1.resident = false + n2.resident = false + + shared_a:invoke(n1) + + assert(n1._private.is_destroyed) + assert(not n2._private.is_destroyed) + + shared_a:invoke(n2) + assert(n2._private.is_destroyed) + + return true +end) + -- Now check if the old deprecated (but still supported) APIs don't have errors. table.insert(steps, function() -- Tests are (by default) not allowed to call deprecated APIs From 4fe05a566d91cfb835d4370254f83638824e1ad7 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 17 Jul 2019 02:16:45 -0400 Subject: [PATCH 18/25] tests: Test exposing the notification persistence and animation props --- tests/test-naughty-legacy.lua | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/test-naughty-legacy.lua b/tests/test-naughty-legacy.lua index 14875fe3..07d626dd 100644 --- a/tests/test-naughty-legacy.lua +++ b/tests/test-naughty-legacy.lua @@ -3,6 +3,7 @@ local spawn = require("awful.spawn") local naughty = require("naughty" ) local gdebug = require("gears.debug") +local gtable = require("gears.table") local cairo = require("lgi" ).cairo local beautiful = require("beautiful") local Gio = require("lgi" ).Gio @@ -782,6 +783,38 @@ table.insert(steps, function() return true end) +-- Test that exposing support for animations and persistence are exposed to DBus. +table.insert(steps, function() + assert(not naughty.persistence_enabled) + assert(not naughty.image_animations_enabled) + + assert(gtable.hasitem(naughty.dbus._capabilities, "icon-static")) + assert(not gtable.hasitem(naughty.dbus._capabilities, "icon-multi")) + assert(not gtable.hasitem(naughty.dbus._capabilities, "persistence")) + + naughty.persistence_enabled = true + naughty.image_animations_enabled = true + + assert(naughty.persistence_enabled) + assert(naughty.image_animations_enabled) + + assert(gtable.hasitem(naughty.dbus._capabilities, "icon-multi")) + assert(gtable.hasitem(naughty.dbus._capabilities, "persistence")) + assert(not gtable.hasitem(naughty.dbus._capabilities, "icon-static")) + + naughty.persistence_enabled = false + naughty.image_animations_enabled = false + + assert(not naughty.persistence_enabled) + assert(not naughty.image_animations_enabled) + + assert( gtable.hasitem(naughty.dbus._capabilities, "icon-static")) + assert(not gtable.hasitem(naughty.dbus._capabilities, "icon-multi" )) + assert(not gtable.hasitem(naughty.dbus._capabilities, "persistence")) + + return true +end) + -- Now check if the old deprecated (but still supported) APIs don't have errors. table.insert(steps, function() -- Tests are (by default) not allowed to call deprecated APIs From 6761778830641959a908cba3627f37a3f7d6b69d Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 17 Jul 2019 03:09:06 -0400 Subject: [PATCH 19/25] tests: Test the notification icon theme request::icon support. --- tests/test-naughty-legacy.lua | 49 +++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/test-naughty-legacy.lua b/tests/test-naughty-legacy.lua index 07d626dd..640529f0 100644 --- a/tests/test-naughty-legacy.lua +++ b/tests/test-naughty-legacy.lua @@ -674,6 +674,9 @@ table.insert(steps, function() assert(n.actions[2].name == "five" ) assert(n.actions[3].name == "six" ) + n:destroy() + assert(#active == 0) + return true end) @@ -815,6 +818,52 @@ table.insert(steps, function() return true end) +local icon_requests = {} + +-- Check if the action icon support is detected. +table.insert(steps, function() + assert(#active == 0) + + naughty.connect_signal("request::icon", function(a, icon_name) + icon_requests[icon_name] = a + + a.icon = icon_name == "list-add" and small_icon or big_icon + end) + + local hints = { + ["action-icons"] = GLib.Variant("b", true), + } + + send_notify("Awesome test", 0, "", "title", "message body", + { "list-add", "add", "list-remove", "remove" }, hints, 25000) + + return true +end) + +table.insert(steps, function() + if #active ~= 1 then return end + + local n = active[1] + + assert(n._private.freedesktop_hints) + assert(n._private.freedesktop_hints["action-icons"] == true) + + assert(icon_requests["list-add" ] == n.actions[1]) + assert(icon_requests["list-remove"] == n.actions[2]) + + assert(n.actions[1].icon == small_icon) + assert(n.actions[2].icon == big_icon ) + + assert(type(n.actions[1].position) == "number") + assert(type(n.actions[2].position) == "number") + assert(n.actions[1].position == 1) + assert(n.actions[2].position == 2) + + n:destroy() + + return true +end) + -- Now check if the old deprecated (but still supported) APIs don't have errors. table.insert(steps, function() -- Tests are (by default) not allowed to call deprecated APIs From aed2af44b6ee81b5e5a930ed6667e515b53ca8bc Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 17 Jul 2019 03:26:17 -0400 Subject: [PATCH 20/25] tests: Check if using client icon doesn't exceed the size constraints. --- tests/test-naughty-legacy.lua | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test-naughty-legacy.lua b/tests/test-naughty-legacy.lua index 640529f0..f433c342 100644 --- a/tests/test-naughty-legacy.lua +++ b/tests/test-naughty-legacy.lua @@ -556,6 +556,16 @@ table.insert(steps, function() assert(n2.box.width +2*n2.box.border_width <= wa.width ) assert(n2.box.height+2*n2.box.border_width <= wa.height) + -- Check with client icons. + assert(not n1.icon) + + n1._private.clients = {{icon= big_icon, type = "normal"}} + assert(n1.icon == big_icon) + assert(n1.box.width +2*n1.box.border_width <= wa.width ) + assert(n1.box.height+2*n1.box.border_width <= wa.height) + assert(n2.box.width +2*n2.box.border_width <= wa.width ) + assert(n2.box.height+2*n2.box.border_width <= wa.height) + n1:destroy() n2:destroy() From 529f4be53e032b8662313bf4b12f5daca6d1fb13 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Wed, 17 Jul 2019 03:32:29 -0400 Subject: [PATCH 21/25] tests: Check that naughty emits "request::display_error" when needed. --- tests/test-naughty-legacy.lua | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test-naughty-legacy.lua b/tests/test-naughty-legacy.lua index f433c342..0940ca09 100644 --- a/tests/test-naughty-legacy.lua +++ b/tests/test-naughty-legacy.lua @@ -874,6 +874,21 @@ table.insert(steps, function() return true end) +-- Test the error popup. +table.insert(steps, function() + local got = nil + + naughty.connect_signal("request::display_error", function(err) + got = err + end) + + awesome.emit_signal("debug::error", "foo") + + assert(got == "foo") + + return true +end) + -- Now check if the old deprecated (but still supported) APIs don't have errors. table.insert(steps, function() -- Tests are (by default) not allowed to call deprecated APIs From 423aeebe8aeae1e0c3b8eab15f369015ac1ee42a Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 22 Jul 2019 00:10:22 -0400 Subject: [PATCH 22/25] naughty: Add more default values. Without this, some variables could accidently be set to `nil` by the dbus code. Ignore now also has a default. Ref #2829 --- lib/naughty/constants.lua | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/naughty/constants.lua b/lib/naughty/constants.lua index def1f6eb..ce237b13 100644 --- a/lib/naughty/constants.lua +++ b/lib/naughty/constants.lua @@ -72,6 +72,10 @@ ret.config.defaults = { border_width = dpi(1), position = "top_right", urgency = "normal", + message = "", + title = "", + app_name = "", + ignore = false, } ret.notification_closed_reason = { From c0ef0c880218fd7916dc893ba56933ef9a81c66c Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 22 Jul 2019 00:22:20 -0400 Subject: [PATCH 23/25] tests: Expose the previously private gears.protect_call error handlers. This will allow the test suits to intercept them instead of adding more and more exceptions to `run.sh`. --- lib/gears/protected_call.lua | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/gears/protected_call.lua b/lib/gears/protected_call.lua index e42bc707..fb967d14 100644 --- a/lib/gears/protected_call.lua +++ b/lib/gears/protected_call.lua @@ -12,11 +12,11 @@ local xpcall = xpcall local protected_call = {} -local function error_handler(err) +function protected_call._error_handler(err) gdebug.print_error(traceback("Error during a protected call: " .. tostring(err), 2)) end -local function handle_result(success, ...) +function protected_call._handle_result(success, ...) if success then return ... end @@ -27,13 +27,13 @@ if not select(2, xpcall(function(a) return a end, error, true)) then -- Lua 5.1 doesn't support arguments in xpcall :-( do_pcall = function(func, ...) local args = { ... } - return handle_result(xpcall(function() + return protected_call._handle_result(xpcall(function() return func(unpack(args)) - end, error_handler)) + end, protected_call._error_handler)) end else do_pcall = function(func, ...) - return handle_result(xpcall(func, error_handler, ...)) + return protected_call._handle_result(xpcall(func, protected_call._error_handler, ...)) end end From dcd034dcaca3bb760bab4edd8baa7c59c7872248 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 22 Jul 2019 00:33:34 -0400 Subject: [PATCH 24/25] naughty: Add 2 levels of fallback when the `widget_template` fails. * First, it will try the default widget template * If that fails, the `request::fallback` handler will use the legacy popup. Ref #2829 --- lib/naughty/layout/box.lua | 21 ++++++++++++++++++++- lib/naughty/layout/legacy.lua | 4 +++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/naughty/layout/box.lua b/lib/naughty/layout/box.lua index a4846794..5e80dd5c 100644 --- a/lib/naughty/layout/box.lua +++ b/lib/naughty/layout/box.lua @@ -17,6 +17,7 @@ local popup = require("awful.popup") local awcommon = require("awful.widget.common") local placement = require("awful.placement") local abutton = require("awful.button") +local gpcall = require("gears.protected_call") local dpi = require("beautiful").xresources.apply_dpi local default_widget = require("naughty.widget._default") @@ -136,10 +137,25 @@ end -- @param widget local function generate_widget(args, n) - local w = wibox.widget.base.make_widget_from_value( + local w = gpcall(wibox.widget.base.make_widget_from_value, args.widget_template or (n and n.widget_template) or default_widget ) + -- This will happen if the user-provided widget_template is invalid and/or + -- got unexpected notifications. + if not w then + w = gpcall(wibox.widget.base.make_widget_from_value, default_widget) + + -- In case this happens in an error message itself, make sure the + -- private error popup code knowns it and can revert to the fallback + -- popup. + if not w then + n._private.widget_template_failed = true + end + + return nil + end + if w.set_width then w:set_width(n.max_width or beautiful.notification_max_width or dpi(500)) end @@ -231,6 +247,9 @@ local function new(args) -- Generate the box before the popup is created to avoid the size changing new_args.widget = generate_widget(new_args, new_args.notification) + -- It failed, request::fallback will be used, there is nothing left to do. + if not new_args.widget then return nil end + local ret = popup(new_args) ret._private.args = new_args diff --git a/lib/naughty/layout/legacy.lua b/lib/naughty/layout/legacy.lua index ee99c083..f32af2da 100644 --- a/lib/naughty/layout/legacy.lua +++ b/lib/naughty/layout/legacy.lua @@ -308,7 +308,9 @@ end function naughty.default_notification_handler(notification, args) -- This is a fallback for users whose config doesn't have the newer -- `request::display` section. - if naughty.has_display_handler then return end + if naughty.has_display_handler and not notification._private.widget_template_failed then + return + end -- If request::display is called more than once, simply make sure the wibox -- is visible. From 6d58d7b4a2a2e3e5fa8515d55bc7c06a90becdc3 Mon Sep 17 00:00:00 2001 From: Emmanuel Lepage Vallee Date: Mon, 22 Jul 2019 01:14:47 -0400 Subject: [PATCH 25/25] tests: Test that the legacy naughty popup is used when the new API fails Displaying errors is important. If the notification popup caused the error, it was likely nothing could be displayed. --- tests/test-naughty-legacy.lua | 78 +++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/tests/test-naughty-legacy.lua b/tests/test-naughty-legacy.lua index 0940ca09..67fd80af 100644 --- a/tests/test-naughty-legacy.lua +++ b/tests/test-naughty-legacy.lua @@ -8,6 +8,8 @@ local cairo = require("lgi" ).cairo local beautiful = require("beautiful") local Gio = require("lgi" ).Gio local GLib = require("lgi" ).GLib +local gpcall = require("gears.protected_call") +local dwidget = require("naughty.widget._default") -- This module test deprecated APIs require("gears.debug").deprecate = function() end @@ -1000,6 +1002,82 @@ table.insert(steps, function() return true end) +-- Add a "new API" handler. +local current_template, had_error, handler_called = nil + +table.insert(steps, function() + assert(naughty.has_display_handler == false) + + naughty.connect_signal("request::display", function(n) + handler_called = true + + naughty.layout.box { + notification = n, + widget_template = current_template + } + end) + + return true +end) + +-- Make sure the legacy popup is used when the new APIs fails. +table.insert(steps, function() + assert(naughty.has_display_handler == true) + + function gpcall._error_handler() + had_error = true + end + + local n = naughty.notification { + title = nil, + message = nil, + timeout = 25000, + } + + assert(handler_called) + assert(not had_error) + assert(not n._private.widget_template_failed) + assert(not n.box) + + n:destroy() + handler_called = false + + -- Try with a broken template. + current_template = {widget = function() assert(false) end} + + n = naughty.notification { + title = "foo", + message = "bar", + timeout = 25000, + } + + assert(handler_called) + assert(had_error) + assert(not n.box) + + handler_called = false + had_error = false + + -- Break the default template + assert(dwidget.widget) + dwidget.widget = nil + dwidget.layout = function() assert(false) end + table.remove(dwidget, 1) + + n = naughty.notification { + title = "foo", + message = "bar", + timeout = 25000, + } + + assert(handler_called) + assert(n._private.widget_template_failed) + assert(had_error) + assert(n.box) + + return true +end) + -- Test many screens. require("_runner").run_steps(steps)