Skip to content

Store "sender-pid" and fix Clippy lints#142

Open
joshuamegnauth54 wants to merge 1 commit intopop-os:masterfrom
joshuamegnauth54:handle-sender-pid-plus-clippy
Open

Store "sender-pid" and fix Clippy lints#142
joshuamegnauth54 wants to merge 1 commit intopop-os:masterfrom
joshuamegnauth54:handle-sender-pid-plus-clippy

Conversation

@joshuamegnauth54
Copy link

I bundled the Clippy fixes with this commit since they're small and don't warrant its own PR.

"sender-pid" is a hint set by some apps, such as Electron apps or "notify-send." It's useful as a more granular indicator than app ID because it's the PID of the sending app.

For COSMIC Notifications, this commit fixes a warning but does not use "sender-pid" directly. It's stored for further use like the other hints.


  • I have disclosed use of any AI generated code in my commit messages.
    • If you are using an LLM, and do not fully understand the changes it is making to the code base, do not create a PR.
    • In our experience, AI generated code often results in overly complex code that lacks enough context for a proper fix or feature inclusion. This results in considerably longer code reviews. Due to this, AI authored or partially authored PRs may be closed without comment.
  • I understand these changes in full and will be able to respond to review comments.
  • My change is accurately described in the commit message.
  • My contribution is tested and working as described.
  • I have read the Developer Certificate of Origin and certify my contribution under its conditions.

I bundled the Clippy fixes with this commit since they're small and
don't warrant its own PR.

"sender-pid" is a hint set by some apps, such as Electron apps or
"notify-send." It's useful as a more granular indicator than app ID
because it's the PID of the sending app.

For COSMIC Notifications, this commit fixes a warning but does not use
"sender-pid" directly. It's stored for further use like the other hints.
Hint::SenderPid(pid) => Some(*pid),
_ => None,
})
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this usable by cosmic-notifications itself? It would help to have something to test this with.

Copy link
Author

@joshuamegnauth54 joshuamegnauth54 Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I left it unused because I think System76 would have to decide what to do with it. For example, the group_notifications() function can use it to group by both PID and App ID instead of just PID. This PR only silences a warning that's printed to the system logs fairly often because sender-pid is a common hint.

I wrote the function you're referencing because some of the other hints have convenience functions like that. 😁

A quick way to test this before and after is to use notify-send which also sets the sender-pid hint. Run journalctl --user --follow in a separate tab then post some notifications with notify-send.

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

Successfully merging this pull request may close these issues.

2 participants