Re: [PATCH] Sparse checkout: items selection dialog

On 04.09.2013 19:22, Sergey Azarkevich wrote:
> Hello,
>
> Disclaimer: I attach patch, but it is only DRAFT. If it will be agreed
> to apply, then I test it more heavily.
>
>
> I try improve behavior of 'Choose items' dialog by:
>
> 1. When tree item inserted into tree, it inherit parent check state
>
> I think it is logical - expand checked item and see all children
> checked. Expand unchecked item and see all items unchecked
>
> As side effect all items initially checked (inherit root check state).

Use this instead:
// inherit parents check state
if (m_RepoTree.GetRootItem() == hParent)
     m_RepoTree.SetCheck(hNewItem, false);
else
     m_RepoTree.SetCheck(hNewItem, m_RepoTree.GetCheck(hParent));

>
> 2. Introduce pseudo tri-state behavior (but not look):
>
> If user click unchecked item, then only this item checked (depth - 'Only
> this item')
> If user click checked item and all children unchecked - check whole subtree
> If user click checked item and not all children unchecked - uncheck
> whole subtree
>

Looks good!

> This rule not concern when:
>
> * several items selected in tree.
> it can confuse user when click can lead to different effects: some items
> will be checked, some unchecked. check/uncheck will be applied
> recursevily to all selected items and theirs children
>
> * item collapsed
> it will confuse user when check will be reset only from second try
> without other side effects.
> also if item still not loaded, then we will not remember special
> semi-checked mode.

I think this looks very good. Almost ready to commit.
Just a few nitpicks:

CheckTreeItemParents /rename/to/ CheckParentsOfTreeItem
CheckTreeItemRecurse /rename/to/ CheckTreeItemRecursive

Also the line
m_RepoTree.SetCheck(hChild, bCheck);
is not necessary:

void CRepositoryBrowser::CheckTreeItemRecurse( HTREEITEM hItem, bool
bCheck )
{
     // process item
     m_RepoTree.SetCheck(hItem, bCheck);

     // process all children
     HTREEITEM hChild = m_RepoTree.GetChildItem(hItem);
     while (hChild)
     {
// the following line is not necessary!
         m_RepoTree.SetCheck(hChild, bCheck);
         CheckTreeItemRecurse(hChild, bCheck);
         hChild = m_RepoTree.GetNextItem(hChild, TVGN_NEXT);
     }
}

IsAllChildrenUnchecked /rename/to/ AreAllChildrenUnchecked
Or maybe even rename to
HaveAllChildrenSameCheckState(HTREEITEM hItem, bool bChecked = false)
and check if all items are checked or unchecked, depending on the
bChecked parameter.

Also looking at your patch I realize that I should maybe change the
m_blockEvents from a bool to an int and treat it as a counter: that way
we can avoid problem with recursion.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest interface to (Sub)version control
    /_/   \_\     http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=3063928
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].