caronc / apprise

Apprise - Push Notifications that work with just about every platform!
https://hub.docker.com/r/caronc/apprise
BSD 2-Clause "Simplified" License
10.9k stars 384 forks source link

Apply overflow options to attachments #1093

Closed pomeloy closed 2 months ago

pomeloy commented 2 months ago

Description:

Related issue (if applicable): #1091

Checklist

Testing

Anyone can help test this source code as follows:

# Create a virtual environment to work in as follows:
python3 -m venv apprise

# Change into our new directory
cd apprise

# Activate our virtual environment
source bin/activate

# Install the branch
pip install git+https://github.com/caronc/apprise.git@<this.branch-name>

# Test out the changes with the following command:
apprise -t "Test Title" -b "Test Message" \
  <apprise url related to ticket>
caronc commented 2 months ago

I think only 1 line of change is needed in the if TRUNCATE

# inside if overflow is set to truncate
If attach and len(attach) > 1:
  # log entry that attachments were truncated
  t_attach = AppriseAttachments()
  # store first attachment only
  t_attach.add(attach[0])
  # swap in new attachment placeholder
  # garbage collector will handle previous attach module now dereferenced
  attach = t_attach

I'm traveling right now, so replying with my phone. But it looks like this is all that is needed sheet reviewing your PR. All of the other things you changed (while clever) makes the code a wee bit more complicated then it needs to be (or already is 🙂)

Now all endpoints impacted (new and old) are updated/supported.

Thoughts?

pomeloy commented 2 months ago

Thank you so much for your input! I would have totally missed out on the garbage collection and logging part without it. I had a brain fart and pasted the code almost in its entirety but I think I got it now.