Opened 10 years ago

Closed 5 years ago

#5009 closed Patch (outdated)

Patch for feature request #4993

Reported by: Xishi Pan Owned by:
Priority: normal Component: FileZilla Client
Keywords: Cc:
Component version: Operating system type: Windows
Operating system version: XP SP3

Description

Hi,

Here is my patch for feature request #4993 - default download folder.
Please help review

Attachments (3)

Patch4993.diff (8.4 KB) - added by Xishi Pan 10 years ago.
Source patches.
Patch4993.2.diff (7.4 KB) - added by Xishi Pan 10 years ago.
Revision to previous patch but with same patch name. Add an option to enable/disable
Patch4993_5009.diff (15.9 KB) - added by Xishi Pan 10 years ago.
Patch for #4993 and #5009

Download all attachments as: .zip

Change History (19)

Changed 10 years ago by Xishi Pan

Attachment: Patch4993.diff added

Source patches.

comment:1 in reply to:  description Changed 10 years ago by Xishi Pan

Oops... sorry, just forgot to add some descriptions.
In the "Settings->Transfer" dialog, a text box and button for choosing a folder have been added.
The code, including the updated "settings.xrc" works in my dev environment but an error of "invalid resource file" was reported after installing with NSIS script... have to figure out why...

:)

comment:2 Changed 10 years ago by Tim Kosse

Thanks, two suggestions:

  • Please make it an option to choose between current behavior (remember last used) and a fixed default directory.
  • Don't include the START/END comments in the patch

comment:3 in reply to:  2 Changed 10 years ago by Xishi Pan

Thanks for your input. I would like to add that option and submit the patch again.
Hope I can fix the "invalid resource file" issue... :)

comment:4 Changed 10 years ago by Tim Kosse

Possibly the flex grid sizer. It's set to 4 items in a 2x2 grid yet only a single item in the sizer item in the file.

comment:5 in reply to:  4 Changed 10 years ago by Xishi Pan

Thanks. I'm trying but it doesn't work so far.
What's the best practice of updating the UI?
I'm using wxFormBuilder v3.0 to change the form design, export xrc and then manually merge the changes...

comment:6 Changed 10 years ago by Tim Kosse

I'm using XRCed to edit the .xrc files. Bit hard to install under Windows, but on several Linux distributions such as Ubuntu, packages are readily available.

comment:7 in reply to:  6 Changed 10 years ago by Xishi Pan

Just figured out the invalid resource file issue...
I've added some backup xrc files in the source folder and the NSIS script copied all of them to the installation folder... then... they were all loaded by FileZilla...

I'm submitting the patch again ...

Changed 10 years ago by Xishi Pan

Attachment: Patch4993.2.diff added

Revision to previous patch but with same patch name. Add an option to enable/disable

comment:8 Changed 10 years ago by Tim Kosse

Status: newmoreinfo

Thanks, making nice progress.

Two more things:

  • Have a look at the code that currently restores the last used local directory when starting FileZilla. Should probably be moved to the tab creation code as well.
  • In most dialogs of FileZilla, controls for some options are disabled if a corresponding checkbox/radiobutton isn't selected. The default directory input box and browse button should probably be disabled if the checkbox isn't set.

Changed 10 years ago by Xishi Pan

Attachment: Patch4993_5009.diff added

Patch for #4993 and #5009

comment:9 in reply to:  8 Changed 10 years ago by Xishi Pan

Status: moreinfonew

Thanks for the input.
I've updated the code accordingly. Please help review.

comment:10 Changed 10 years ago by Xishi Pan

By the way...in recent SVN revision, the usage of site manager has been changed to show the site list by right-click. right? I think there is a bug: when connected with a site, choose a remote folder and copy the url into clipboard, then click site manager. Filezilla will crash definitely. And sometimes, after copying a url into clipboard, any other operations on context-menu crash the app. easily...

comment:11 Changed 10 years ago by Tim Kosse

Status: newmoreinfo

1) Please use an individual file for each unrelated change. Patches that change multiple things make it hard to review and apply the changes. In future please attach the patch to the original ticket for the problem it fixes, but for now you can keep using this ticket.

2) Please adhere to the project's coding style. FileZilla uses tabs for indentation in most of its files, not spaces.

3) Why do you call it default download directory? In behavior it actually the default local directory for all operations, many of which aren't downloads or don't even involve transfers. Default local directory would also be in line to the terminology used in the sitemanager and bookmarks dialog.

4) The checkbox "Enable default download (new: local) directory" is confusing. What about two radiobuttons, one labeled Remember last used local directory and the other Use custom directory. All controls wrapped inside a static box labelled Default local directory.

5) Is the transfer page really the correct location? The default local directory affects more than just transfers. Furthermore, the transfers page is full already, it would have to be split into multiple pages to fit on smaller screens (think netbook users). Therefore I think the interface page would be more fitting.

6) There's a control with the unused name "m_staticline3" in the xrc file wasting resources

comment:12 Changed 10 years ago by Tim Kosse

Are you still working on this? There have been some architectural changes in FileZilla. Please update your patch so that it applies to current trunk.

comment:13 in reply to:  12 Changed 10 years ago by Xishi Pan

Status: moreinfonew

Replying to codesquid:

Are you still working on this? There have been some architectural changes in FileZilla. Please update your patch so that it applies to current trunk.

Yes. But i got too busy these weeks...
I will submit new patch for the new architecture

comment:14 Changed 10 years ago by Tim Kosse

Status: newmoreinfo

Any progress?

comment:15 Changed 8 years ago by Tim Kosse

Status: newmoreinfo

comment:16 Changed 5 years ago by Tim Kosse

Resolution: Noneoutdated
Status: newclosed
Note: See TracTickets for help on using tickets.