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

Custom indicators for actions and URLs #1216

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

elamperti
Copy link

@elamperti elamperti commented Oct 24, 2023

What's new?

Continuing the work of #1150, this PR resolves #509. It adds three new settings:

  • action_indicator and url_indicator, which replace the values of %A and %U respectively in the format string whenever there are actions or URLs in the notification
  • show_action_count, which adds the action count to the default indicator. E.g.: if there are 3 actions for a notification, instead of showing "(A)", with show_action_count enabled you will see "(3A)". Users can use the action count in their own format strings with %C (which will be replaced by the number of actions). Requested by @monkz and @bynect.

And also:

  • Using %D in the format string will print how many duplicates a given notification has, if any.

Other things included in the PR

  • A test for the default indicator and another one for custom indicators
  • Small refactor in notification_update_text_to_render() to simplify how we create the default dup/action/url indicators

Pending question

After implementing the changes to have %C, I'm wondering if going forward we really need to ditch show_indicators. Or if we need to have a more powerful solution such as this:

action_count_strings="no actions|%C action|%C actions"

so users can set e.g. format="%s %C" and dunst picks which part of action_count_strings to show in %C accordingly.

I think this would be a bit convoluted. What do you think?

@bynect
Copy link
Member

bynect commented Oct 24, 2023

Regarding your last question, probably adding a action_many_indicator would suffice. When no action is added we do not even consider action_indicators at all. Maybe we can introduce a base_indicator or something like that that gets added when no other indicator is present.

Also, correct me if I am wrong, does the show_indicators now work like this?
url_indicator = "(U)"
action_indicator = "(A)"

But then, how do we recreate the current indicator behavior with this? We should probably try to avoid breaking backwards compatibility for what it's worth.

Ps: do %U and %A only print when the notification has an action/url?

@bynect
Copy link
Member

bynect commented Oct 24, 2023

Also we could add a xmore_indicator or something like that for formatting the notification you get with indicate_hidden

@elamperti
Copy link
Author

Regarding your last question, probably adding a action_many_indicator would suffice. When no action is added we do not even consider action_indicators at all. Maybe we can introduce a base_indicator or something like that that gets added when no other indicator is present.

As someone who has no interest in seeing the action count, I'd still need the action_indicator. But this brings me back to my original question, because in my case I could achieve the same behavior (following the example above, this is not current behavior) with something like action_count_strings="|A|A", which would show an empty string when there are no actions, and "A" if there's one or more. I don't think this would be intuitive for someone who is configuring dunst for the first time.

Also, correct me if I am wrong, does the show_indicators now work like this? url_indicator = "(U)" action_indicator = "(A)"

They are "A" and "U" (no parenthesis) in the default config, but you will be able to customize them however you want (e.g. "(U)", "Click here", "🔗", etc). When show_indicators is enabled (it is by default), a notification with one URL and one action will show "(UA)". Check the changes in notification.c.

If you want them to be separated (i.e. "(U) (A)"), it could be achieved with a config like this:

show_indicators = no
url_indicator = "(U)"
action_indicator = "(A)"
format = "%s %U %A"

But then, how do we recreate the current indicator behavior with this? We should probably try to avoid breaking backwards compatibility for what it's worth.

This change shouldn't affect current configs, with no url_indicator or action_indicator defined in your dunstrc. Defaults are set in settings_data.h.

Ps: do %U and %A only print when the notification has an action/url?

Correct, please see notification.c.

Also we could add a xmore_indicator or something like that for formatting the notification you get with indicate_hidden

I don't understand this, could you explain your idea with more detail? (there's no indicate_hidden either)

@bynect
Copy link
Member

bynect commented Oct 24, 2023

As someone who has no interest in seeing the action count, I'd still need the action_indicator

Yes, I meant that we can do something like this:

  • action_indicator when we have a single action
  • action_many_indicator when we have multiple action (so context)
  • url_indicator for urls

Alternatively to action_many_indicator we can have a context_indicator that is placed when either multiple actions or url+action are present. (Probably even better)

The idea of base_indicator or something similar for notifications without any action/url still stands.

@fwsmit
Copy link
Member

fwsmit commented Oct 24, 2023

This seems a little hard to make configurable and still make sure everything works as expected (like using plural wording when there are more actions). I would suggest to allow the user to choose between a few pre-selected styles, instead of letting them configure their own strings.

@elamperti
Copy link
Author

That sounds reasonable. I've opened #1218 so we can discuss the action count indicator there.

Is it ok if I remove %C and show_action_count from this PR? They'd become obsolete pretty quick if we implement some kind of action count indicator and I'd rather keep it small.

I'm trying to figure out the memory leaks and errors in the automated builds. If you can identify quick solutions they'd be welcome, I'm not familiar enough with C to spot them right away.

@fwsmit
Copy link
Member

fwsmit commented Nov 10, 2023

I've left my preference in the thread you mentioned. If you could implement that, then I'll look at the memory leaks afterwards.

@fwsmit
Copy link
Member

fwsmit commented Dec 31, 2023

Added a waiting-for-info label, since I'm waiting for a different implementation as mentioned in #1218

@fwsmit
Copy link
Member

fwsmit commented Jan 20, 2024

@elamperti are you still planning to finish this PR?

@elamperti
Copy link
Author

Hi, I'm busy with other things at the moment and the refactor needed to implement #1218 would take me a considerable effort (I'm not used to C and I was aiming to just have a simple way to show icons/indicators). If someone else can take it from here, it'll be more than welcome.

@fwsmit
Copy link
Member

fwsmit commented Jan 25, 2024

Okay, no problem. I'll add a help wanted tag.

@bynect bynect marked this pull request as draft April 6, 2024 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom indicators for URL and Actions
3 participants