Added file exist actions

[paolozambotti]

Hi,

I've added a couple of options that I find sometime useful to the file exist actions.
The new options are:

• overwrite if size is different
• overwrite if size is different or source is newer

I also made some changes around the code in order to allow future new option easy to add; I changed all (all the ones I found) refernces to fie exist actions as integer in actions as "enum CFileExistsNotification::OverwriteAction"

I tested the changes compiling the code under mingw, they seem to work fine.

Cheers,
Paolo.

[paolozambotti]

Patch file more file exist actions based on rev. 2390

[Tim Kosse]

Thanks for your patch.

I have applied the part of the patch that changes the action from integers into the enum and also found a few more places where it was wrong. A reminder for future patches: Please split janitorial work into a separate patch, makes it easier to review your changes.

I cannot apply the second half yet, it needs to be improved first:

1. You cannot do things like "&o - Overwrite if different size or source newer", not all platforms have keyboard mnemonics in dialogs, e.g. they just do not exist under OS X. I think we have to live with some options not having a mnemonic in radio button groups, especially since cursor keys allow changing the selection quickly.

2. The case where both local and remote filesizes are unknown needs to be handled for overwriteSize. Your code incorrectly skips the transfer. This case should rarely (if ever) happen, but we cannot ignore it.

Please fix these two issues and attach another patch against current SVN trunk.

[paolozambotti]

some more actions int to enum changes

[paolozambotti]

Thanks for your notices.

As you suggested I split the new patch in janitorial work and new fuctionalities.
First of all I attached a new patch (FileZilla_actions_int_to_enum_rev2413.patch) that fixes some more places where actions are still present as integer. This patch was included in my previous one so I don't know if you missed this changes or you didn't want to merge them but, in any case, I resubmit to you.
About the second part of the patch with the new actions (size and sizeornewer) a new patch will follow but before I need to ask you a couple or things.
I don't catch your point (1): why doing "&o - Overwrite if different size or source newer" is different from "Rena&me"? Ok, this is just to let my understand, no problem to leave new actions without mnemonics.
About point (2), you are right this case must to be handled. Just to be sure, unknown size is represented by pData->local/remoteFileSize as -1 value, isn't it?

Cheers,
Paolo.
File Added: FileZilla_actions_int_to_enum_rev2413.patch

[Tim Kosse]

I did already commit janitorial part yesterday.

You cannot use "&o - Overwrite if different size
or source newer" because some platforms have no mnemonics. Users of those platforms would see the actual labels prefixed with a totally useless "o - ". If you use a mnemonic, it has to be part of the actual label, don't just add some random letters.

Yes, unknown size is -1

[paolozambotti]

Two more file exist action patch on rev 2413

[paolozambotti]

Yes I know you already commit janitorial part but, after doing that (rev 2413), there are still a couple of files where file exist action are managed with integers and not with enum. The file FileZilla_actions_int_to_enum_rev2413.patch fixes also those files.

About mnemonics, ok I understand your point, thanks.
Anyway, I attaced the new patch (filezilla_more_fileexist_actions_rev2413.patch) with the two new option (size and sizeornewer) with the two fixes you suggest:

• no mnemonics at all for the two new option (I prefer to not assign mnemonics at all in place of assign them in one window and not in the other cause a lack of letters)
• both file of unknown size is now managed (overwritten)

Just one note filezilla_more_fileexist_actions_rev2413.patch is based on the source already patched with FileZilla_actions_int_to_enum_rev2413.patch

Cheers,
Paolo.
File Added: filezilla_more_fileexist_actions_rev2413.patch

[Tim Kosse]

I've applied the 2nd enum patch with a few small changes. You can't cast an int from an untrusted source directly into an enum, the result is undefined (actually implementation-dependent).
In some implementations, an enum's size is just one byte. Both 1 and 257 would both be casted to "overwrite". In other implementations where an enum is bigger, the 257 would fall into the default case of the switch statement.

Next I'll be reviewing the third patch.

[Tim Kosse]

I've applied the new actions. Fixed a few mistakes:

• unknown size comparison incorrectly used != instead of ==
• compile error due to CFileExistsNotification::CFileExistsNotification::overwriteSize
• moved new actions to the end of the action enum, else they would have interfered with saved actions from previous versions of FileZilla