-
-
Notifications
You must be signed in to change notification settings - Fork 442
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
Attachments should honor overflow=truncate setting (and not send more then 1) #1091
Comments
I think you really stumbled onto somthing here. I'd like you to get credit for the fix but i don't quite think what you submitted is what we need. It's still a bit too complicated. Attached is a gunzip /path/to/apprise-truncate-attachment.patch.gz
git apply /path/to/apprise-truncate-attachment.patch It's based on our discussion, but a more clean lightweight version. I also added some unit tests to verify it works correctly. Please make a PR with this and i'll gladly accept it. It's a great find on your part. This will enforce that |
Thanks so much for helping me out with the PR! I'm still not too versed in all of this. |
I would roll back and just apply the attached patch as it will solve your problem. Or just close your PR and start a fresh one. While the is nothing wrong with your solution, it's a bit more code then what is needed to solve the problem unless there is something I'm missing? |
Obviously your solution is much cleaner. I just thought that maybe you were keen on also having the overflow=split option respected when handling the attachments. Personally, your patch is totally sufficient for me. |
Closing this off as i think we nailed this! Thank you again for your issue and help! Don't ever hesitate to reach out if you find anything else! 🙂 |
💡 The Idea
Pushover does not support messages with multiple attachments. As a workaround, the Apprise plugin currently splits a given notification into multiple messages when passed a list of attachments. This is a great way to circumvent this limitation, but sometimes results in confusion when receiving multiple messages with more than one attachment in a short period of time.
There should be a new, optional parameter turning this behavior off. This would enhance user experience especially when using the Pushover plugin in parallel to another notification service that natively supports multiple attachments.
🔨 Breaking Feature
No, as the new parameter should be optional.
The text was updated successfully, but these errors were encountered: