patchmldonkey, a multi-networks file-sharing client - Patches: patch #5239, GTK2 GUI: Fix result sorting,...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #5239: GTK2 GUI: Fix result sorting, update search results instead of double display

Submitter:  su_blanc <su_blanc>
Submitted:  Fri 14 Jul 2006 05:08:40 PM UTC
   
 
Category:  Bug fixes Priority:  5 - Normal
Status:  Done Assigned to:  None
Open/Closed:  Closed

Sat 15 Jul 2006 11:47:26 AM UTC, comment #3: 

Applied 2006/07/15

spiralvoice <spiralvoice>
Group administrator
Sat 15 Jul 2006 10:07:07 AM UTC, comment #2: 

patch updated, according to the quote.
Now GUI updates the results....

Thus sorting should be fixed also.
results were deleted from the query when the max results was exceeded without checking it could be already displayed in the results list of the query.
In this case, the sorting function raised an exception because it couldn't retrieve the result.

su_blanc <su_blanc>
Group Member
Fri 14 Jul 2006 07:45:58 PM UTC, comment #1: 

Yes, the change you observed is correct.
Here is a copy of an IRC conversation about this topic between pango and zet, zet coded the changes in patch #4810 responsible for the changes.

May 26 13:49:51 <spiralvoice> pango: have investigated it further, some patch from z some months ago sent those results (same file from several servers I think) to the GUI
May 26 13:51:03 <spiralvoice> kmldonkey prints each filename as one line although hash is the same (3 filenames, 1 hash -> kmldonkey 3 search result lines, sancho 1 line with popup where the other filenames are printed)
May 26 13:51:33 <spiralvoice> this kmldonkey behaviour was already reported as a bug somewhere
May 26 13:53:40 <pango> spiralvoice: I think mlgui suffers from this too
May 26 13:54:08 <spiralvoice> pango: reason is IMHO the changing of a GUI opcode instead of introducing a new one
May 26 13:58:28 <pango> http://mldonkey.sourceforge.net/phpBB2/viewtopic.php?p=22600#22600
May 26 14:01:11 <spiralvoice> patches 4810 or 5088 should be responsible for that
May 26 14:01:30 <spiralvoice> maybe this new line in commonResults.ml, IndexedResults.update_result rs rr; - have to test it
May 26 14:01:59 <spiralvoice> 4810 was first in 2.7.3
May 26 14:02:15 <spiralvoice> 2.7.3 and 2.7.5 for sure - contains bug in searching
May 26 14:04:55 <pango> well, it's not a bug, it's a feature ;) but GUIs need modifications
May 26 14:05:54 <spiralvoice> no, current opcodes need to be changed back and new opcodes introduced, GUI protocol assures backward compatibility which is broken in this case
May 26 14:16:18 <pango> or, as I wrote, a change of protocol version (and only use new behavior if version is high enough); Because I think it's a change of behavior, rather than packets contents
May 26 15:15:06 <z|away> GUI protocol was not broken.  It is the design of the gui protocol to send a structure again when it is updated. Files. Clients, Options, etc.  Now Results do the same. Adding another opcode would be incorrect. 
May 26 15:16:22 <pango> but do you agree that protocol version should have been incremented ?
May 26 15:17:59 <pango> (not that this was not forgotten for some other protocol changes, I remember when chunks states increased from 3 to 4,... it's too easy to forget)
May 26 15:18:53 <z|away> Not really. You could set it to not send duplicate results unless gui prot is the current ver(40) to help, but "should have" means it was in error.  The error is in guis that didn't take this into account from the beginning. (sancho was lazy too)
May 26 15:19:42 <pango> not a reason for breaking them if it can be avoided
May 26 15:21:34 <z|away> As it said, if it helps and doesn't hurt it can be done easily.  But there is no reason to write code specifically to avoid bugs in other software.
May 26 15:22:15 <pango> no reason ?
May 26 15:24:28 <pango> btw I saw some post from su_blanc on the forum, maybe he can help with mlgui
May 26 15:24:38 <z|away> No reason = No reason for a new opcode = No reason to design it specifically for broken software.  Hacks like only sending dups if guiprot > 39 aren't redesigning anything.
May 26 15:25:45 <pango> that's how backward compatibility is handled by this protocol scheme
May 26 15:27:18 <z|away> Again, if every gui handled dup options, just like it handles dup files, clients, etc, it wouldn't be noticed. It isn't a backwards compatibility issue like changing the format of an opcode, (from a design perspective).  It is one from a more "hacky" perspective.
May 26 15:27:48 <pango> I think you're often right, but in that case you're just arrogant
May 26 15:28:28 <z|away> Thanks.  But there remains a difference IMHO.  Maybe the word hacky bothers you, I don't know..
May 26 15:28:47 <pango> other GUIs exists, they may have their flaws, but we should try to support them, specially when the cost it's high
May 26 15:28:53 <pango> s/it's/isn't/
May 26 15:29:05 <z|away> Of course.  Where was I arguing against this?
May 26 15:29:39 <pango> you deny the fact there's any reason to increase protocol version
May 26 15:30:51 <pango> I don't think a new opcode was called for either
May 26 15:31:40 <z|away> Nope.  Now you're putting words into my mouth.  I explained what I meant by "No reason" above.
May 26 15:34:29 <pango> yes, that in theory they're wrong so it's ok to break them. I agree with the first part of the sentense only
May 26 15:35:05 <z|away> That in theory they are wrong, so there is No Reason to redesign what a Result is just for them. 
May 26 15:35:33 <pango> but does supporting them require a redesign ?
May 26 15:36:50 <z|away> I've already said this.. (no).  This is what I am arguing against: a new opcode.
May 26 15:37:44 <pango> and I'm arguing about a protocol version increment, is the conclusion that we're in violent agreement ? :)
May 26 15:39:14 <z|away> Depends.  I'm in support of a retroactive increment to only send dups if gui prot is == to the current version, not to increment the current one to an even higher number.
May 26 15:40:25 <pango> the version number must be higher than the one "supported" by the GUIs that don't understand search result updates
May 26 15:40:33 <z|away> (or whatever version it was at the time I suppose.. )
May 26 15:40:38 <pango> I don't know when protocol version was last modified
May 26 15:43:49 * pango writing to su_blanc_ atm
May 26 15:48:23 <pango> done
May 26 16:55:25 <Amorphous> man we increase the proto version all the time. the version is there to be increased...
May 26 16:56:02 <Amorphous> z|away: just changing the behaviour for the current version is flawed (or not) in the same way it is for older versions.
May 26 16:56:18 <z|away> I don't know what you are arguing.
May 26 16:57:18 <Amorphous> z|away: for inceasing the version and only sending dups for the newer versions
May 26 16:57:51 <z|away> And? I've said that already.
May 26 16:58:19 <Amorphous> z|away: no you said: "not to increment the current one to an even higher number."
May 26 16:58:29 <Amorphous> or did i missunderstood you?
May 26 17:03:44 <z|away> Probably.  The version # at which dups are sent should match the version#+1 at the time the change was made (a while ago). Incrementing the current one doesn't make sense.
May 26 17:05:27 <Amorphous> oh mh it seems i missunderstood the whole problem then...
May 26 17:05:52 <pango> I thought it was a recent change too
May 26 17:08:40 <z|away> Maybe it is.. I don't know what the version# was. If it was the same as the current, then current can change.  The point is that it depends on what it was at the time, not what it is now.
May 26 17:09:28 <pango> patch #4810 was applied in January it seems
May 26 17:12:03 <pango> the 19th... proto version changed just before, then 16th ? http://cvs.savannah.nongnu.org/viewcvs/mldonkey/mldonkey/src/daemon/common/guiProto.ml?only_with_tag=HEAD&r2=1.24&r1=1.23
May 26 17:13:56 <pango> sounds good enough, what about only sending updates only when negociated proto >= 40 ?
May 26 17:14:12 <SchAmane> huh, thats not so easy if you dont anderstand even basic of ocaml0
May 26 17:14:12 <z|away> Sure, go for it. :)



spiralvoice <spiralvoice>
Group administrator
Fri 14 Jul 2006 05:08:40 PM UTC, original submission:  

should fix bug #16994

The core is sending the same result for the same query several times. Is there any change done recently?
Is the core updating the result (like availability, ....)?
Before it was not the case.

regards

su_blanc <su_blanc>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

CC list is empty

 

There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

 

Follow 7 latest changes.

Date Changed by Updated Field Previous Value => Replaced by
2006-07-15 spiralvoice StatusReady For Test Done
    Open/ClosedOpen Closed
2006-07-15 spiralvoice Summaryfix bug #16994 GTK2 GUI: Fix result sorting, update search results instead of double display
2006-07-15 su_blanc StatusWorks For Me Ready For Test
2006-07-15 su_blanc Attached File#10352 Removed
2006-07-15 su_blanc Attached File- Added GTK2_gui_sorting_results_2.patch, #10356
2006-07-14 su_blanc Attached File- Added GTK2_gui_sorting_results.patch, #10352

Back to the top

Powered by Savane 3.13-f8d8.
Corresponding source code