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

Added PCRE notification filtering/blocking #364

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

akoerner
Copy link

@akoerner akoerner commented Nov 20, 2023

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.

@palinek
Copy link
Contributor

palinek commented Nov 21, 2023

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

@akoerner1
Copy link

akoerner1 commented Nov 21, 2023

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.

@palinek
Copy link
Contributor

palinek commented Nov 21, 2023

using QRegularExpression adds yet another external dependency

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

@akoerner
Copy link
Author

akoerner commented Nov 21, 2023

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.

@tsujan
Copy link
Member

tsujan commented Nov 21, 2023

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

@akoerner
Copy link
Author

akoerner commented Nov 22, 2023

Sorry, at this point I am very confused. All of the branches refer to Qt5:

set(QT_MINIMUM_VERSION "5.15.0")

find_package(Qt5LinguistTools ${QT_MINIMUM_VERSION} REQUIRED)

find_package(Qt5Widgets ${QT_MINIMUM_VERSION} REQUIRED)

..and don't compile regexps on each filtering invocation.
@palinek
Copy link
Contributor

palinek commented Nov 22, 2023

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.

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.

4 participants