Skip to content

Implement a command line control (dunstctl)#651

Merged
tsipinakis merged 21 commits into
dunst-project:masterfrom
bebehei:dunstctl
May 1, 2020
Merged

Implement a command line control (dunstctl)#651
tsipinakis merged 21 commits into
dunst-project:masterfrom
bebehei:dunstctl

Conversation

@bebehei

@bebehei bebehei commented Aug 11, 2019

Copy link
Copy Markdown
Member

@bebehei bebehei added this to the 1.5 milestone Aug 11, 2019
@bebehei bebehei force-pushed the dunstctl branch 2 times, most recently from d079bad to ad848be Compare August 23, 2019 00:15
@bebehei bebehei changed the title Implement a command line control Implement a command line control (dunstctl) Aug 23, 2019

@Stunkymonkey Stunkymonkey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

testing the cl-interface everything works fine.

Comment thread dunstctl Outdated
method_call "${DBUS_IFAC_DUNST}.NotificationShow" >/dev/null
;;
"status")
property_get running | ( read _ _ status; printf "%s\n" "${status}"; )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can produce the output if dunst is not running:

$ dunstctl status
Error org.freedesktop.DBus.Error.NoReply: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken.

is this wanted?

Comment thread dunstctl
|| die "No status parameter specified. Please give either 'true' or 'false' as running parameter."
[ "${2}" == "true" ] || [ "${2}" == "false" ] \
|| die "Please give either 'true' or 'false' as running parameter."
property_set running variant:boolean:"${2}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the same applies here from above

Comment thread src/dbus.c Outdated
" <interface name=\""DUNST_IFAC"\">"

" <method name=\"ContextMenuCall\" />"
// TODO: add an optional parmater definining the action of notification number X to invoke

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i skipped reading through this file, because there are too much TODOs and FIXMEs.
but testing wise, everything works perfectly fine for me.

@tsipinakis

Copy link
Copy Markdown
Member

@bebehei Looking at the TODOs in the first message this looks almost done, will you work on it or maybe I should take over? I'd like to finish this and make a release soon-ish.

@Stunkymonkey

Copy link
Copy Markdown
Contributor

when continuing, also look at this: bebehei#2

@bebehei

bebehei commented Apr 4, 2020

Copy link
Copy Markdown
Member Author

@tsipinakis Well, sad news. @Stunkymonkey and I started a new project together and our goal is to rewrite dunst in Rust. You may have seen it already in your GitHub timeline, it's called durst (german for thirst and a funny combination of dunst and rust).

Beginning with the lockdown for the corona pandemic, I've got more time to contribute to OSS projects again. In relation to the durst notification manager, I'll continue to help here in dunst. But my long term goal will be the switch to durst.

So yes, I won't finish this PR here in its full intention. Feel free to clone the branch and continue (or start over and rewrite commits). I won't have the necessary power and motivation to finish this PR.

@bebehei

bebehei commented Apr 4, 2020

Copy link
Copy Markdown
Member Author

PS: If you want to join the project, we'd be happy to have you there.

@tsipinakis

Copy link
Copy Markdown
Member

Well, sad news. @Stunkymonkey and I started a new project together and our goal is to rewrite dunst in Rust

I saw you creating the repo, but at the time it was just a hello world. I'll post some of my thoughts in an issue in that repo later today :)

@tsipinakis tsipinakis marked this pull request as ready for review April 9, 2020 18:03
@tsipinakis

Copy link
Copy Markdown
Member

@bebehei Mind giving this a review? I completed pretty much everything still in progress I could find, and scrapped the monitor for status changes features for now. It will probably take some effort and I'll return to it later.
For now my goal is to get at least a replacement for the keyboard shortcuts merged ASAP.

@codecov-io

codecov-io commented Apr 10, 2020

Copy link
Copy Markdown

Codecov Report

Merging #651 into master will decrease coverage by 0.10%.
The diff coverage is 15.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #651      +/-   ##
==========================================
- Coverage   66.40%   66.29%   -0.11%     
==========================================
  Files          29       29              
  Lines        4899     5076     +177     
==========================================
+ Hits         3253     3365     +112     
- Misses       1646     1711      +65     
Flag Coverage Δ
#unittests 66.29% <15.18%> (-0.11%) ⬇️
Impacted Files Coverage Δ
test/dunst.c 78.57% <0.00%> (-21.43%) ⬇️
src/dbus.c 66.54% <13.51%> (-19.37%) ⬇️
test/dbus.c 98.67% <100.00%> (+<0.01%) ⬆️
src/x11/x.c 2.33% <0.00%> (-0.02%) ⬇️
src/option_parser.c 86.66% <0.00%> (+0.38%) ⬆️
test/option_parser.c 98.25% <0.00%> (+0.53%) ⬆️
src/utils.c 91.59% <0.00%> (+0.94%) ⬆️
src/settings.c 57.89% <0.00%> (+1.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d03802...c89898b. Read the comment docs.

@bebehei

bebehei commented Apr 10, 2020

Copy link
Copy Markdown
Member Author

I'll review this now.

@bebehei bebehei left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Shall I review my own code, too?

Comment thread src/dbus.c
Comment thread dunstctl Outdated
Comment thread dunstctl Outdated
Comment thread dunstctl Outdated
Comment thread dunstctl Outdated
@tsipinakis

Copy link
Copy Markdown
Member

Fixed.

Shall I review my own code, too?

Of course 😉

evenbrenden added a commit to evenbrenden/dotfiles that referenced this pull request Apr 14, 2020
As suggested in dunst-project/dunst#216 (comment).

/tmp should be cleared on boot. Note that notifications is still turned on when starting i3.
That way we override whatever the default behaviour of dunst is (it defaults to on) and we
force the state to appear in i3status by creating the state file.

This is fine while waiting for dunstctl (dunst-project/dunst#651).
@tsipinakis

Copy link
Copy Markdown
Member

I'm basically done here I think.
@bebehei Tell me if you want to focus on durst rather than reviewing PRs here, but mind a final review?

@tsipinakis

Copy link
Copy Markdown
Member

I timed out waiting for @bebehei, I'll just merge this for now to have something usable in master. Any other fixes can be done later.

I'd like to see this tested for some weeks before making a release, if you do use it please report your experience with it and any bugs you find.

@AkashKrDutta

Copy link
Copy Markdown

Hi. This PR doesn't resolve #308 right? The history-pop command just re-displays the notifications from history and they again go back to the history stack after click/timeout. let me know if there's a command to remove a notification from history.

@fwsmit

fwsmit commented May 16, 2021

Copy link
Copy Markdown
Member

No you're right. I do not plan on implementing that feature any time soon, but you could implement it yourself. Otherwise, subscribe to the issue and you'll hear when it's implemented

@BachoSeven

Copy link
Copy Markdown

@fwsmit So if I understand correctly now all keybindings from [shortcuts], which was just deprecated by your PR, need to be implemented manually by users through their WMs by using dunstctl commands(?)

@fwsmit

fwsmit commented Jul 25, 2021

Copy link
Copy Markdown
Member

@fwsmit So if I understand correctly now all keybindings from [shortcuts], which was just deprecated by your PR, need to be implemented manually by users through their WMs by using dunstctl commands(?)

Yes. There was already a warning in the documentation about the shortcuts section being deprecated for a while now. All settings in the [shortcuts] section should have dunstctl equivalents.

@BachoSeven

Copy link
Copy Markdown

I see, thanks for the clarification. I ran dunst via .xinitrc and didn't notice the deprecation messages until today when the shortcuts stopped working :'D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants