rgladwell / imap-upload

Python script for uploading a local mbox file to IMAP4 server.
Other
130 stars 30 forks source link

Fixed empty box bug #32

Closed sraedler closed 2 years ago

sraedler commented 2 years ago

In the given line, the application crashes for me because of an empty box variable.

My parameters: -r $FileDir$ --host imap.strato.de --port 993 --ssl --user ... --password ...

In the FileDir, multiple mbox files are available (INBOX, ....) I could solve the issue by passing the current mbox name as the new box to the upload.

I couldn't see (within a few minutes) why the box should be empty in this line. Therefore, I opened up this pull request.

Nevertheless, many thanks for the great script! It helped me to sync from one server to another!

rgladwell commented 2 years ago

Thanks for the PR, I really appreciate all the contributions users have been making.

Just before I merge this change, I'd like to understand the values of file.split(".")[0] vs box. For both cases, where box == null and box != null.

Feels like this might break other cases. Wouldn't this lose the recursiveness of this function, by dropping the subbox parameter?

sraedler commented 2 years ago

Feels like this might break other cases. That might be the case, yes. My code review was not exhaustive. I just checked my issue.

Wouldn't this lose the recursiveness of this function, by dropping the subbox parameter? Somehow also true.

IMHO, the problem is that the recursion is only working if there is a file tree with more than one level. In my case, the mbox files (e.g. INBOX.mbox, Sent Items.mbox,...) are in the same dictionary as the python file and there is no additional nested mbox-file. Therefore, the initial call of the recursion is with box == '' and the empty string is also propagated in the upload function, which breaks.

If you like, I can change it to: target_box = file.split(".")[0] if (box == null or box == '') else box This could solve the problem correctly mentioned by you.

But that's just a guess. I haven't debugged it yet.

rgladwell commented 2 years ago

Your suggested change seems wiser. Push it and I'll merge.

sraedler commented 2 years ago

DONE.

Thanks!