Today ted unexpectedly reviewed my onScroll patch. Now, suddenly I have a ton of changes that I have to make and my release is due tomorrow (technically today since the clock just hit midnight).
One of the problems that ted focused on was how I passed merge changesets from the server-side to the client-side. I was using a weird and ugly string which is delimited by a random separator. I’ve known all along that this was a very ugly solution. When I had come up with this implementation I didn’t know python very well and thus did not use the dictionary data structure, which is an integral part of the language. I don’t know why I haven’t changed this implementation until now. I guess I decided to give other things more priority because although the solution was ugly, it worked.
Nonetheless, I have made the appropriate changes and the following is what my server-side solution looks like now:
1 2 3 4 5 | def getMergeData(ctx, mergeData): for cs in ctx.parents(): mergeData.append({'node': ctx.hex(), 'user': clean(person(cs.user())), 'desc': clean(cs.description())}) if len(cs.parents()) > 1: getMergeData(cs, mergeData) |
The client side implementation:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 | function loadEntries(iterations) { for(var t = 0; t < iterations; t++) { var end = start; $('.loader').html('<img src="static/ajax-loader.gif" align="right" />'); start -= 20; $.get('/json-pushes?startID=' + start + '&endID=' + end, function(responseText) { var pushData = JSON.parse(responseText), counter = 0; for(var i = end; i > start; i--) { var row = $(document.createElement('tr')); row.addClass('parity' + counter % 2); counter++; // user column $('<td>' + pushData[i].user + '<br /><span class="date">' + pushData[i].formattedDate + '</span></td>') .appendTo(row) .width(184); // rev column $('<td>' + createRevTd(pushData[i].individualChangeset) + '</td>').appendTo(row); // author and description column $('<td>' + '<strong>' + pushData[i].author + ' &mdash ' + createBuglink(pushData[i].desc) + '</strong>' + '</td>').appendTo(row); $('.loader').html(""); $('.titlePush').append(row); //Check whether it is a merge changeset or not if(pushData[i].mergeData != []) { for(var j = 0; j < pushData[i].mergeData.length; j++) { if(pushData[i].mergeData[j] != "") { var node = pushData[i].mergeData[j]['node']; var user = pushData[i].mergeData[j]['user']; var desc = pushData[i].mergeData[j]['desc']; createMergeRows(node, user, desc, row); // nested lists if(pushData[i].mergeData[j] instanceof Array) { for(var k = 0; k < pushData[i].mergeData[j].length; k++) { node = pushData[i].mergeData[j][k]['node']; user = pushData[i].mergeData[j][k]['user']; desc = pushData[i].mergeData[j][k]['desc']; createMergeRows(node, user, desc, row); } } } } } } }); } } |
So, as you can see now I’m using a list that holds a bunch of dictionaries. Each dictionary has 3 keys: node, user and desc. I pass the the list, mergeData over the to the client side via JSON. Then on the client side I go through the list and retrieve the values of each key (lines 29-32)
However, you must have noticed that the client-side implementation has a nested for loop (line 38). Ted was wondering why it takes a maze of nested loops to display merges (although now it is down to only one nested loop). The reason I have to do it this way is because sometimes mergeData can contain nested lists. Especially when dealing with large merges that tracemonkey usually has.
So there you have it. I’ve replaced the long and ugly string that previously held merge information with a nice and beautiful dictionary.