Re: [Evolution-hackers] Re: [evolution-patches] patch for bug 48466

[Jeffrey Stedfast <fejj ximian com>]

What happens if the order that get_selection() returns is not in
ascending order? What if it returns: 0, 1, 3, 2, 4, 7, 3 ?

Your solution will also double-remove items... your solution only works
if the items are in ascending order. Perhaps we can assume that, but I'm
not sure we can. If what you are saying is true, that the way items are
selected (and re-selected?) has an effect on how they are added to the
linked list, then I think this is a real possibility also.

Jeff

On Mon, 2003-09-15 at 10:32, Charles Zhang wrote:
> Thank you, Not Zed.
> I think it's necessary to do so.
> 
> First, in the existing code structure, it often do remove an item more
> than once.
> Because in the result of function gnome_iconlist_get_selection(), the
> return GList contains the index of the selected item.
> But in this index list, the item index information is duplicated if
> you pressed 'CTRL' while selecting item (from unselected state) in the
> iconlist widget.
> For example, you insert three attachments, select the first one, then
> multi-select the second one(PRESS CTRL WHILE DOING IT).
> Thus you get the all three selected, and if you right click one of the
> two selected icons and use the "remove" submenu, then these three all
> disappear because what the function gnome_iconlist_get_selection()
> return is 011, but not 01.
> I don't know if I have showed it clearly enough. My libgnomeui's
> version is 2.2.2-6.
> 
> Secondly, I think the existing code is less efficient, because it
> makes another list, this is not necessary.
> Why it makes this list, I'll coming to that in the third reason.
> 
> The third, in the existing code, the author uses two loop parts to
> complete the remove action, he cannot do this remove action with one
> loop because what the function gnome_iconlist_get_selection() return
> is the index of the selected icon. So if he removes the item, the
> other icon's index is not the order it has been. Thus he convert the
> index to the pointer of the real object in the first loop part, and
> remove them in the second loop part. If we use this list_reverse()
> function, we don't have to consider the order at all.
> 
> The fourth,  because the  gnome_iconlist_get_selection() function
> return the duplicated information of the selected item' index (maybe
> it has been updated or fixed in libgnomeui's new version, but we
> should have all versions work.), we should check the item index in the
> loop part to see if it has been removed.
> 
> Finally, we can easily get the dialog struct, but I cannot get it's
> wrapper struct and free it. Then I have to use this method to get us
> free the DialogData struct defined in line
> 283(e-msg-composer-attachment.c).
> 
> I must have some problem unsolved, thank you for your any advice and
> permanent responsible work.
> 
> Best Regards
> Charles Zhang
> 
> Not Zed wrote:
> > Hi,
> > 
> > I'd be happier if you left the existing code structure in place - why
> > exactly do you say it can remove an item more than once?
> > 
> > With you're changes you're actually changing the selection on the fly,
> > and only by reversing the index list does it 'work' - although it is
> > less transparent as to why it works.
> > 
> > I suggest you just change the last loop to be something like:
> >        
> >        for (p = attachment_list; p != NULL; p = p->next) {
> >                 EMsgComposerAttachment *attachment = p->data;
> >                 if (attachment->editor_gui != NULL) {
> >                      
> > gtk_widget_destroy(glade_xml_get_widget(attachment->editor_gui,
> > "dialog"));
> >                 }    
> >                remove_attachment (bar, attachment);
> >         }
> > 
> >        
> > 
> > On Sun, 2003-09-14 at 15:49 +0800, Charles Zhang wrote:
> > 
> >   
> > > Hello, all.
> > >        In this patch, I fix bug 48466.
> > >        It's about the attatchment-bar in the composer.
> > >        I've modified most of the code of function "remove_selected" in 
> > > the file "e-msg-composer-attatchment-bar.c" under dir "composer", 
> > > because it is less efficient and dangerous. Following the origin code,  
> > > attatchment will be removed more than one times.
> > >        Please review it.
> > > 
> > > Best Regards.
> > > Charles
> > >     
> >   
-- 
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
fejj ximian com  - www.ximian.com