Re: [PATCH] Keep Selection when refreshing

Tobias Schäfer wrote:
> Hi Stefan,
>
> whenever a SVNStatusList gets refreshed (F5, Drag and Drop in the commit
> dialog), the current selection gets lost, i.e. manually deselected versioned
> items or manually selected non-versioned items.
> This is rather annoying when you have chosen single items to commit and need
> to refresh.
>
> This patch solves this. Manually de-/selected items are restored in their
> previous state. If new items appear (new non-versioned item, versioned item
> gets modified/deleted etc) they get the selection state they would get if the
> dialog was closed and reopened.
>
> I don't like removing the "const" from AddEntry(...) but it makes the
> implementation so much easier. Could you please review. Ok to commit?

Idea is good. Patch needs some tweaking. Just a few comments:

> -void CSVNStatusListCtrl::AddEntry(const FileEntry * entry, WORD langID, int listIndex)
> +void CSVNStatusListCtrl::AddEntry(FileEntry * entry, WORD langID, int listIndex)
> {
> static CString ponly(MAKEINTRESOURCE(IDS_STATUSLIST_PROPONLY));
> static HINSTANCE hResourceHandle(AfxGetResourceHandle());
>
> + CString path = entry->GetPath().GetSVNPathString();
> + bool checked;
> + if (m_mapFilenameToChecked.Lookup(path,checked))
> + {
> + // The user manually de-/selected an item. We now restore this status
> + // when refreshing.
> + entry->checked = checked;
> + }
> +

This is very expensive. Imagine a working copy with thousands of files.
For every file added, you do a lookup in the map.
Maybe a new boolean which indicates if it's a refresh or the first time
filling? I haven't checked the code for the Lookup() method, but maybe a
check if the map is empty first might be faster?

> +
> + CMap<CString,LPCTSTR,bool,bool> m_mapFilenameToChecked; ///< Remember manually de-/selected items
> };

Why do you use the MFC map instead of the std::map here? Is there any
special reason? In my experience, the std::map is somewhat faster.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org