lxqt-notificationd icon indicating copy to clipboard operation
lxqt-notificationd copied to clipboard

Added PCRE notification filtering/blocking

Open akoerner opened this issue 1 year ago • 7 comments

Background

This PR adds PCRE message filtering and blocking.

Many applications offer no ability or limited ability to configure notifications. This is annoying when a particular application is sending too many notifications. There may be some interesting notifications that an application offers and some annoying ones with no way to configure fine grain control of this.

Why does this need to exist?

Some applications do not allow you to configure notifications or do not support granular notification settings. The black list feature does not work as expected or desired. Some notifications are absolutely incessant where you constantly have to click them away or interact with them. This PR offers a simple and powerful solution to filter dbus notifications based off of PCREs; this feature could be rolled into the black list if desired.

This PR offers a simple and powerful way to filter/block messages with a PCRE no matter how the notifications are generated.

Messages can be filtered on: application, body or summary by setting either application_pcre_filter, body_pcre_filter and/or summary_pcre_filter in the lxqt/notifications.conf.

If the application string is captured by the PCRE set by application_pcre_filter then the notification will not be shown, same goes for the body and summary. View the readme for more details on configuring PCRE filters.

Changes

  1. 3 new configuration parameters were added to the QSettings con fig file: application_pcre_filter, body_pcre_filter, and summary_pcre_filter
  2. Exception handling was enabled in src/CMakeLists.txt for the lxqt-notificationd target.
  • Compiling PCREs at runtime can generate exceptions. If the user provides a PCRE containing a syntax error then lxqt-notificationd should simply ignore it instead of crashing.
  1. Added the function boolean NotificationLayout::filter(std::string string, std::string pcre)
  • This function checks if the supplied string is captured by the provided pcre. This function returns true if the string is captured by the pcre and false if there is a syntax error in the pcre or it is not captured.

akoerner avatar Nov 20 '23 19:11 akoerner

Please, what is the reason to not use the Qt's facility for regular expressions and do this std <-> qt translations?

palinek avatar Nov 21 '23 08:11 palinek

For the first comment regarding std::string I have no excuse other then it is what I am familiar with. For the second point using QRegularExpression adds yet another external dependency when std::regex is more then sufficient to accomplish the task.

akoerner1 avatar Nov 21 '23 09:11 akoerner1

using QRegularExpression adds yet another external dependency

What additional dependency are you referring to? It is in Qt core, isn't it?

palinek avatar Nov 21 '23 09:11 palinek

using QRegularExpression adds yet another external dependency

What additional dependency are you referring to? It is in Qt core, isn't it?

Yes you would have to add Qt core as a dependency. This is already an argument with sticking with std::regex. Between major versions of Qt you would need to modify the cmake list and refactor the code because the interface for regular expressions changed between Qt versions.

For Qt5:

...
find_package(Qt6 REQUIRED COMPONENTS Core5Compat)
target_link_libraries(mytarget PRIVATE Qt6::Core5Compat)
...

and for Qt6:

...
find_package(Qt6 REQUIRED COMPONENTS Core)
target_link_libraries(mytarget PRIVATE Qt6::Core)
...

Even briefly looking at the docs for QRegExp in Qt5 and QRegularExpression in Qt6 there are significant interface changes between qt5 and qt6. This would add another thing that would need to be majorly refactored and tested between qt versions. I think sticking with the C++ standard libraries wherever possible is prudent.

akoerner avatar Nov 21 '23 10:11 akoerner

Nothing in Qt is an extra dependency, let alone Qt core.

QRegularExpression has been in Qt5 for years, and QRegExp shouldn't be used anymore (old codes should/will be updated if existing).

Qt6::Core5Compat isn't used in our Qt6 branches (which don't include lxqt-notificationd yet).

tsujan avatar Nov 21 '23 10:11 tsujan

Sorry, at this point I am very confused. All of the branches refer to Qt5: https://github.com/lxqt/lxqt-notificationd/blob/01699d85bb48411e80877610f6d11b457f57cc17/CMakeLists.txt#L21 https://github.com/lxqt/lxqt-notificationd/blob/01699d85bb48411e80877610f6d11b457f57cc17/CMakeLists.txt#L24 https://github.com/lxqt/lxqt-notificationd/blob/01699d85bb48411e80877610f6d11b457f57cc17/CMakeLists.txt#L25

akoerner avatar Nov 22 '23 09:11 akoerner

Sorry, at this point I am very confused.

You're confusing us. There is no additional dependency needed for using Qt regular expressions -> see the PR for your code.

palinek avatar Nov 22 '23 11:11 palinek