Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

notification_command should be renamed #279

Closed
raffaem opened this issue Jan 24, 2024 · 6 comments
Closed

notification_command should be renamed #279

raffaem opened this issue Jan 24, 2024 · 6 comments

Comments

@raffaem
Copy link

raffaem commented Jan 24, 2024

Even with notify: false in ~/.config/udiskie/config.yml, udiskie still shows notifications when a USB drive is connected/disconnected.

For example, these are the notifications send when a device is removed (obtained from makoctl history).

We have 3 with summaries device_removed, and another one with Device removed.

In general, I propose to remove the first three notifications, as I don't find them useful.

Otherwise, don't show them when notify: false.

{
	"type": "aa{sv}",
	"data": [
		[
			{
				"app-name": {
					"type": "s",
					"data": "udiskie"
				},
				"app-icon": {
					"type": "s",
					"data": "usb-creator"
				},
				"category": {
					"type": "s",
					"data": ""
				},
				"desktop-entry": {
					"type": "s",
					"data": ""
				},
				"summary": {
					"type": "s",
					"data": "device_removed"
				},
				"body": {
					"type": "s",
					"data": ""
				},
				"id": {
					"type": "u",
					"data": 4
				},
				"urgency": {
					"type": "y",
					"data": 1
				},
				"actions": {
					"type": "a{ss}",
					"data": {}
				}
			},
			{
				"app-name": {
					"type": "s",
					"data": "udiskie"
				},
				"app-icon": {
					"type": "s",
					"data": "usb-creator"
				},
				"category": {
					"type": "s",
					"data": ""
				},
				"desktop-entry": {
					"type": "s",
					"data": ""
				},
				"summary": {
					"type": "s",
					"data": "device_removed"
				},
				"body": {
					"type": "s",
					"data": "/dev/sda"
				},
				"id": {
					"type": "u",
					"data": 3
				},
				"urgency": {
					"type": "y",
					"data": 1
				},
				"actions": {
					"type": "a{ss}",
					"data": {}
				}
			},
			{
				"app-name": {
					"type": "s",
					"data": "udiskie"
				},
				"app-icon": {
					"type": "s",
					"data": "usb-creator"
				},
				"category": {
					"type": "s",
					"data": ""
				},
				"desktop-entry": {
					"type": "s",
					"data": ""
				},
				"summary": {
					"type": "s",
					"data": "device_removed"
				},
				"body": {
					"type": "s",
					"data": "/dev/sda1"
				},
				"id": {
					"type": "u",
					"data": 2
				},
				"urgency": {
					"type": "y",
					"data": 1
				},
				"actions": {
					"type": "a{ss}",
					"data": {}
				}
			},
			{
				"app-name": {
					"type": "s",
					"data": "udiskie"
				},
				"app-icon": {
					"type": "s",
					"data": "drive-removable-media"
				},
				"category": {
					"type": "s",
					"data": ""
				},
				"desktop-entry": {
					"type": "s",
					"data": ""
				},
				"summary": {
					"type": "s",
					"data": "Device removed"
				},
				"body": {
					"type": "s",
					"data": "device disappeared on /dev/sda"
				},
				"id": {
					"type": "u",
					"data": 1
				},
				"urgency": {
					"type": "y",
					"data": 255
				},
				"actions": {
					"type": "a{ss}",
					"data": {}
				}
			}
		]
	]
}
@coldfix
Copy link
Owner

coldfix commented Jan 24, 2024

Hi,

For example, these are the notifications send when a device is removed (obtained from makoctl history).

We have 3 with summaries device_removed, and another one with Device removed.

In general, I propose to remove the first three notifications, as I don't find them useful.

That should normally be just one, or maybe two, notification for device removal. Looks like this might have been triggered by some special case of how udisks reports status updates to clients. The notification system needs a rework.

Even with notify: false in ~/.config/udiskie/config.yml, udiskie still shows notifications when a USB drive is connected/disconnected.
[...]
Otherwise, don't show them when notify: false.

Can you post the config.yml including whitespace? It should read:

program_options:
    notify: false

(indentation is relevant in .yml!)

Notifications can also be disabled individually:

notifications:
    device_removed: false

What does echo $XDG_CONFIG_HOME show? Depending on that value, your udiskie config might not have been recognized, because udiskie looks in $XDG_CONFIG_HOME/udiskie/config.yml if that variable is defined.

@raffaem
Copy link
Author

raffaem commented Jan 24, 2024

Sorry for making it unclear.

With notify: false, the last notification, the one with summary="Device Removed", is not triggered.

The other three notifications, the ones with app-icon="usb-creator", are.

So I think it is reading the correct config file which is correctly formatted, otherwise it would not have any effect.

Anyway here is my ~/.config/udiskie/config.yml:

program_options:
  # Configure defaults for command line options

  tray:             auto    # [bool] Enable the tray icon. "auto"
                            # means auto-hide the tray icon when
                            # there are no handled devices.

  menu:             flat    # ["flat" | "nested"] Set the
                            # systray menu behaviour.

  automount:        true   # [bool] Enable automatic mounting.

  notify:           true    # [bool] Enable notifications.

  password_cache:   30      # [int] Password cache in minutes. Caching is
                            # disabled by default. It can be disabled
                            # explicitly by setting it to false

  file_manager: "dolphin"
    # [string] Set program to open directories. It will be invoked
    # with the folder path as its last command line argument.

  terminal: "kitty"
    # [string] Set terminal command line to open directories. It will be
    # invoked with the folder path as its last command line argument.

  password_prompt: "builtin:gui"
    # [string|list] Set command to retrieve passwords. If specified
    # as a list it defines the ARGV array for the program call. If
    # specified as a string, it will be expanded in a shell-like
    # manner. Each string will be formatted using `str.format`. For a
    # list of device attributes, see below. The two special string values
    # "builtin:gui" and "builtin:tty" signify to use udiskie's
    # builtin password prompt.

  notify: true

  notify_command: "notify-send --icon=usb-creator --app-name=udiskie '{event}' '{device_presentation}'"
    # [string|list] Set command to be executed on any device event.
    # This is specified like `password_prompt`.

  icon_names:
    # Customize the icon set used by the tray widget. Each entry
    # specifies a list of icon names. The first installed icon from
    # that list will be used.
    media:   [drive-removable-media-usb-panel]

Also:

➜ echo $XDG_CONFIG_HOME
/home/MYUSERNAME/.config

@coldfix
Copy link
Owner

coldfix commented Jan 25, 2024

Okay, that explains it I think. You might want to disable out your notify_command as that is executed in addition to normal notifications (it can be used to either replace builtin notification, or trigger custom commands upon actions). It is not as "smart" and will be called independently of the notify: false setting or other conditions that the builtin notification takes into account (allowing you to implement your own logic). Apparently, this is where your usb-creator notifications originate from:

  notify_command: "notify-send --icon=usb-creator --app-name=udiskie '{event}' '{device_presentation}'"

Also, note that in your config the setting notify: true appears twice. Changing just one of them might not always achieve the expected effect. I recommend to remove one of these lines.

@coldfix
Copy link
Owner

coldfix commented Jan 25, 2024

I admit that the name for notify_command is chosen poorly and I have to improve the description. It should have been named event_hook or similar to distinguish it from the notification system.

@raffaem
Copy link
Author

raffaem commented Jan 25, 2024

Oh sorry, I'm reusing udiskie since a long time and forgot why I put that line there. Then I just assumed it was the command to send the notifications, not the command that gets called on new event happening :)

Maybe we can rename that option event_hook, retaining the old name for backward compatibility, but having the old name almost disappear from the documentation.

@raffaem raffaem changed the title Notifications still show up even when "notification: false" in config notification_command should be renamed Jan 25, 2024
@coldfix
Copy link
Owner

coldfix commented Jan 26, 2024

Maybe we can rename that option event_hook, retaining the old name for backward compatibility, but having the old name almost disappear from the documentation.

Agreed, I think changing the name will be an improvement.

coldfix added a commit that referenced this issue Jan 26, 2024
- drop external dependency on distutils (#278)
- rename ``--notify-command`` to ``--event-hook`` to prevent misunderstandings (#279)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants