v0.3 Release – Fixing the Problems with my Patch for bug 459727

I’ve been busy trying to fix problems that had arised regarding the patch I submitted for bug 459727. All that is being done right now is fixing various small problems but the overall functionality remains the same. So the following are the solutions to the problems that jorendorff identified:

To calculate the max SELECT COUNT(*) FROM … should be used

Changed my implementation to use gettotalpushlogentries(conn) which performs a select statement. I’ve gotten rid of getMaxEntries() since it is no longer required.

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

def pushes_worker(repo, startID=0, endID=None):
    stmt = 'SELECT id, user, date, rev, node from pushlog INNER JOIN changesets ON id = pushid WHERE id > ? %s ORDER BY id ASC, rev ASC'
 
    args = (startID,)
    if endID is not None:
        stmt = stmt % 'and id <= ?'
        args = (startID, endID)
    else:
        stmt = stmt % ""
    if os.path.basename(repo.path) != '.hg':
        repo.path = os.path.join(repo.path, '.hg')
    conn = sqlite.connect(os.path.join(repo.path, 'pushlog2.db'))
    pushes = {}
    for id, user, date, rev, node in conn.execute(stmt, args):
        mergeData = []
        ctx = repo.changectx(node)
        if len(ctx.parents()) > 1:
          for cs in ctx.parents():
            mergeData.append(hex(cs.node()) + '|-|' + clean(person(cs.user())) + '|-|' + clean(cs.description()))
        if id in pushes:
            pushes[id]['changesets'].append(node)
            pushes[id]['mergeData'].append(mergeData)
        else:
            pushes[id] = {'user': user,
                          'date': date,
                          'changesets': [node],
                          'formattedDate': util.datestr(localdate(date)),
                          'individualChangeset': hex(ctx.node()),
                          'author': clean(person(ctx.user())),
                          'desc': clean(ctx.description()),
                          'mergeData': mergeData,
                          'max': gettotalpushlogentries(conn)
                          }
    return pushes

`new Function(“return ” + entries.responseText) ()` is saying
`eval(entries.responseText)`, perhaps use `JSON.parse (entries.responseText)`

I haven’t implemented this yet because I want more information on why the way I have done is not the appropriate way. I’ve asked on the bug page (comment 12)but haven’t heard back yet

The page loads more results when the user scrolls.  What if the initial results don’t fill up the window?

I added the following CSS to always show a scrollbar no matter what. This should allow the user to scroll even if the initial entries don’t fill up the page.

html { 
  overflow: scroll; overflow-x: auto; 
}

getMaxEntriesis called, then ‘start’ is used once the user scrolls.  But the user might scroll before the first result comes back, while start is still zero — a race condition

I changed the implementation to check the value of the start variable before more entries are allowed to load onScroll. If the value of start is still 0 then no entries are allowed to load. The following is the code:

$(window).scroll(function() {
  if(start > 0) {
    if($(window).scrollTop() == $(document).height() - $(window).height()) {
      renderMorePushLogResults();
    }
  }
});

Better to move CSS and formatting to stylesheets where possible

I removed explicitly setting the styles of the rows. Now I am just setting the class name of the row so that it uses the preexisting styles for that class name. View lines 37-43 for the code

The JS code isn’t consistently indented

It should be correctly formatted now. View here

This patch seems to contain some non-ASCII characters.  Bugzilla doesn’t render it properly

Used &mdash in the places where non-ASCII characters were being used. View lines 73 and 121

In a few places the b tag is used where the server uses the strong tag

Changed the b tags to use strong. View lines 73 and 121

I will be putting up a new patch very soon…

EDIT:

The new patches have been posted here