Ticket #8344 (closed Patch: fixed)

Opened 17 months ago

Last modified 5 months ago

Focus the last directory when navigating to it's parent

Reported by: dudu Owned by:
Priority: normal Component: FileZilla Client
Keywords: Cc:
Operating system type: Operating system version:

Description

Now when filezilla goes up to a parent directory it selects the ".." entry instead of the directory we are coming from.

It should select and focus the directory its coming from and make sure it's visible.

This ticker is related to #1878

I've coded up a solution that implements this behavior. Patch attached against latest svn trunk (r4847)

Attachments

reselect_item_we_are_coming_from_in_parent_directory.patch Download (3.9 KB) - added by dudu 17 months ago.
Patch implementing the ticket, version 3

Change History

Changed 17 months ago by codesquid

  • status changed from new to moreinfo

Three issues:

1) The distinction between selected and focused is important. With your patch, extra directories are being selected when a listing is being refreshed.

To reproduce:
* Select last item in a file list
* Press Ctrl+Up
* Press F5

Before: Second-last item focused but not selected
After: Second-last item selected as well.

2) Catching exception once as copy, once as non-const reference. Somewhat inconsistent. Perhaps use const ref for both?

3) Coding style. Please follow the conventions of the surrounding code, e.g. position of opening brackets.

Other remarks:

I wonder how this affects memory usage in very long running sessions. Compared to some other caches, this one probably is negligible though.

Changed 17 months ago by dudu

Thanks for the review, I've updated the patch. This version should address all three issues and the memory usage remark too.

Changed 17 months ago by dudu

  • summary changed from Select and Focus the last directory when navigating to it's parent to Focus the last directory when navigating to it's parent

Removed mention to focus from the summary since the patch does not concerned with focusing items.

Changed 17 months ago by codesquid

1) state.cpp:311: Condition seems wrong. Going from /foo/bar to /foo/baz would focus bar.

2) Parents are recursive. Both /foo/ and /foo/bar/ are a parent of /foo/bar/baz, yet when going from /foo/bar/baz to /foo, bar should be focused, not baz.

Changed 17 months ago by dudu

Patch implementing the ticket, version 3

Changed 17 months ago by dudu

Please see the (once again) updated patch, I've replaced the conditions to only set refocus information if the the user navigates to up to the direct parent of a current directory.

If the navigation skip levels of subdirectory then no focus will be remembered. I'm not sure what the correct behaviour should be, total-commander seem to be only remember one level changes (at least when using the alt+down history list to jump more than one level) as this patch does.

Changed 17 months ago by codesquid

  • status changed from moreinfo to closed
  • resolution set to fixed

Thanks, it's in with some small cleanup. I've renamed the variables and functions in CState, as that class shouldn't refer to GUI-specific concepts if it can be avoided.

Note: See TracTickets for help on using tickets.