Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#8344 closed Patch (fixed)

Focus the last directory when navigating to it's parent

Reported by: dudu Owned by:
Priority: normal Component: FileZilla Client
Keywords: Cc:
Component version: 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 (1)

reselect_item_we_are_coming_from_in_parent_directory.patch (3.9 KB) - added by dudu 7 years ago.
Patch implementing the ticket, version 3

Download all attachments as: .zip

Change History (7)

comment:1 Changed 7 years ago by Tim Kosse

Status: newmoreinfo

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.

comment:2 Changed 7 years 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.

comment:3 Changed 7 years ago by dudu

Summary: Select and Focus the last directory when navigating to it's parentFocus 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.

comment:4 Changed 7 years ago by Tim Kosse

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 7 years ago by dudu

Patch implementing the ticket, version 3

comment:5 Changed 7 years 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.

comment:6 Changed 7 years ago by Tim Kosse

Resolution: fixed
Status: moreinfoclosed

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.