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

Allow notifications for systems without graphical capabilities #81

Merged

Conversation

ammgws
Copy link
Contributor

@ammgws ammgws commented Apr 22, 2020

Solves #80.

@frederickjh Would you like to test this?

@frederickjh
Copy link

frederickjh commented Apr 24, 2020

@ammgws Thanks for your work on this. I set this up on a remote server to test.

I copied done.fish from your pull repository and installed fishpkg/fish-humanize-duration. I then set:

# Set up done for remote notitfication via Telegram
set -U __done_allow_nongraphical 1
set -U __done_notification_command "ta \$title \$message"

Running a command longer than the default 5 seconds would generate a notification via my ta.fish function using Telegram.

I then increased the time before done sends a message to one minute with:

# in milliseconds time before done sends a message
set -U __done_min_cmd_duration 60000

This works as we discussed in #80.

I also added:

## Commands not to send done messages for. Accepts a regex
# all git commands, except push and pull.
# nano
set -U __done_exclude 'git (?!push|pull)|nano'

As who wants to be notified when they finish a long edit on a document?

Consider adding something like the following to the documentation section Prevent specific commands from triggering notifications. Accepts a regex. something like:

Useful with set -U __done_allow_nongraphical 1 to prevent notifications for commands normally run interactively that you do not want to get done notifications for.

All in all a 👍, thanks!

@ammgws
Copy link
Contributor Author

ammgws commented May 7, 2020

Good to know it's working!

I'll leave docs up to @franciscolourenco as I think it's fine

@frederickjh
Copy link

I'll leave docs up to @franciscolourenco as I think it's fine

I found this blog post that says with X11 forwarding turned on for a SSH connection that that notify-send should send the alert on the local machine from the remote ssh session.

I have not yet had time to test this with done but the blog post says the code they are using ,which was added to fish_prompt.fish, should be added to the Fish shell on the remote host for this to work.

### Notify on long command completion

# If commands runs >= 10 seconds, notify user on completion
if test $CMD_DURATION
    if test $CMD_DURATION -gt (math "1000 * 10")
        set secs (math "$CMD_DURATION / 1000")
        notify-send "$history[1]" "Returned $status, took $secs seconds"
    end
end

If this works with done I think that it would be a good idea to add this to the documentation.

@ammgws
Copy link
Contributor Author

ammgws commented May 20, 2020

If it works you should open another PR for adding it to the docs

@frederickjh
Copy link

If it works you should open another PR for adding it to the docs

@ammgws If it was only that simple. I could not get this to work between two Ubuntu machines. One 16.04 and the other 18.04. I tried a number of settings locally in the ~/.ssh/config file and the remote /etc/ssh/sshd_config but could not get this working even thought X11 forwarding is working to start a GUI program on the remote server and have it appear locally in a window to work on.

The notifications are sent via the DBUS. I tried setting the DBUS_SESSION_BUS_ADDRESS environmental variable but this too did not work. I have gone back to using done with my custom command that sends the notifications via a Telegram chat.

If someone smarter that me can figure this out so that it can be documented, so be it. 😉 😎 👼

@ammgws
Copy link
Contributor Author

ammgws commented Nov 6, 2020

Closing due to inactivity
Sorry @frederickjh !

@ammgws ammgws closed this Nov 6, 2020
@franciscolourenco
Copy link
Owner

@ammgws What is the status of this? Is it working, reliable?

@ammgws
Copy link
Contributor Author

ammgws commented Nov 6, 2020

I made this for @frederickjh, so will need his input

@frederickjh
Copy link

@franciscolourenco and @ammgws Sorry I didn't realize that more input was needed from me. I thought that my comments above were clear as to the status of this pull request.

To summarize what I stated above:

@frederickjh
Copy link

Actually there was one thing I thought should be added to the documentation. From above:

Consider adding something like the following to the documentation section Prevent specific commands from triggering notifications. Accepts a regex. something like:

Useful with set -U __done_allow_nongraphical 1 to prevent notifications for commands normally run interactively that you do not want to get done notifications for.

@franciscolourenco
Copy link
Owner

@ammgws I've had a look at the PR and it looks good, so I would be up for merging this. Do you think it makes sense to add what @frederickjh to the docs, and if so would you mind doing it? Thanks!

@ammgws
Copy link
Contributor Author

ammgws commented Nov 18, 2020

@franciscolourenco done 🐟

@frederickjh
Copy link

Any reason @franciscolourenco this has not yet been merged in? Looks like this has everything to close #80.

@franciscolourenco
Copy link
Owner

Sorry, only was able to get to this.

@franciscolourenco franciscolourenco merged commit fb38ef3 into franciscolourenco:master Jan 24, 2021
@frederickjh
Copy link

No problem @franciscolourenco. Sometimes life happens.

@ammgws ammgws deleted the remotenotifications branch January 24, 2021 22:50
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.

3 participants