Add option to remove obsolete drafts #1627
Conversation
| # set body text | ||
| mailcontent = self.message.get_body_text() | ||
| envelope = Envelope(bodytext=mailcontent, tags=tags) | ||
| envelope = Envelope(bodytext=mailcontent, tags=tags, | ||
| previous_draft=draft) |
There was a problem hiding this comment.
I don't quite see how this is picket up by my editor then in envelope mode?
Am I missing something or is then envelope -> edit command simply ignoring this?
There was a problem hiding this comment.
Your $EDITOR? Is that necessary? SendCommand and SaveCommand simply pick it up from this field in the envelope.
There was a problem hiding this comment.
Sorry, I was confused and only now understood the logic behind this patch.
Question: What if I answer "no" to the prompt to remove the draft, then create another draft and end up with two drafts, then send the last and say "yes" to the prompt.
As far as I can see, the current logic would only remove the very last draft, not the first one?
If so, is that intentional?
I could not test this myself at the minute because I have problems installing the dev version with poetry.. :D
There was a problem hiding this comment.
Yes, that's how it currently works. Removing every revision would require persisting their id's somehow, and that is not how @stevenengler built it. I'm not sure how I would implement that (maybe a custom header?). Currently it remembers the saved draft at runtime in envelope mode, and when it is not deleted, there is no relationship between the old and the new. I'd think that would even be the least surprising behavior.
I tested it by installing by sudo ./setup.py install which immediately says I shouldn't do it that way ;)
|
By the way, I just remembered I saw a pull request today that I think was merged which made the tags to indicate a draft configurable by the user. That is not yet incorporated in this commit. |
|
I took a look at adding support for the draft_tags stuff here and it seems there are other places in the current code base that needs the same type of fix already (see the output of |
|
Sorry for the late response but what is the status of this? |
|
No worries. I'm busy with some IRL stuff right now, but have an idea on how to fix the |
Hi all!
This is an updated version of #1313 (which aimed to fix #1044). I rebased the commit, and deduplicated the code in
SaveCommandandSendCommand. I think that was the most obvious feedback then, but I can imagine there's more.