[Aptitude-devel] aptitude-qt VCS usage and code review

[Piotr Galiszewski]

2010/7/10 Daniel Burrows <dburrows at debian.org>:
> On Sat, Jul 10, 2010 at 12:24:41AM +0200, Piotr Galiszewski <piotr at galiszewski.pl> was heard to say:
>> 2010/7/9 Daniel Burrows <dburrows at google.com>:
>> >   Item (2) confuses me -- what is the name for, exactly?  If it's just the
>> > name the user gave, why is it immutable?  I'm going to assume that
>> > was an oversight.
>>
>> Name  is shown in filters_list on the right side of packages tab. It
>> is immutable, because I have forgotten to add setter. I planned to do
>> this when creating manage filters window. As  name is confusing
>> probably display or display_name would be better.
>
>  Ok.  The user can edit that, I assume?
>
>> >   Item (3) can be handled at the same time as item (2), by creating
>> > a separate "holder" class that represents the user's definition of the
>> > filter and is modified when the definition is modified.  This has the
>> > extra advantage of letting you switch to any other filter, not just one
>> > implemented by the same concrete class.
>> >   i.e., you would have "package_filter", which is an interface for
>> > predicates
>> > on packages, and "package_filter_definition", which contains a name and a
>> > package_filter (pointer), both of which have mutators, and which has two
>> > signals corresponding to those fields, emitted when they change.
>> >   Daniel
>>
>> If I understand correctly package_filter contains
>> accept_package(package) function and for example lists of accepted
>> tags/sections or other mutators. Later every package_filter_definition
>> sets proper values of this mutators? I have read it several times and
>> it is still not entirely clear for me. Some sample code will be really
>> nice :)
>
>  package_filter is a pure virtual interface with "bool package_matches()"
> defined somewhere in it.  package_filter objects are immutable.
>

Thanks for the code! I am working on this now (will be finished
tomorrow) and I have one question. You write that package_filter
should be immutable. What about search filter? It will not be added to
any view, so name is not not necessary. It looks for me that
implementation of package_filter will be sufficient. The only problem
is that searched text could change and in this situation signal should
be emitted to proxy_model. How should this works? Currently I am
planning to add

	virtual sigc::connection connect_filter_invalidated(const
sigc::slot<void> &slot) = 0;

and signal to package_filter. When filter is registered in proxy
model, model will connect to this signal.

In this situation filter will not be immutable.

Is it an acceptable solution?

>  package_filter_definition contains a package_filter and a name.  The
> package_filter can be set to a different package_filter and the name
> can be set to a different name.  Setting either of these fields triggers
> a signal, so interested parties can notice when the filter has changed.
>
>  Actually, I'm not even 100% sure that package_filter_definition needs
> to be mutable; I just assumed there was a good reason for that. :)
>
>
>  I got partway through your second patch, but I think I need to break
> for the night now.  Sorry.
>
>  Daniel
>



-- 
Regards
Piotr Galiszewski