v0.3 Release – Fix for my Patch for bug 445560

In a previous blog post I detailed problems with my previous patch for bug 445560. There were 2 problems that ted identified which need to be fixed:

  1. Should use ids instead of dates
  2. Should store unique identifiers in the class instead of the id of the tag

The Fix

I made a change to pushlog-feed.py to use ids of the changeset instead of the dates by adding the following code which passes the id to the client-side (line 270):

248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
def changelist(limit=0, **map):
        allentries = []
        lastid = None
        ch = None
        l = []
        mergehidden = ""
        p = 0
        currentpush = None
        for id, user, date, node in entries:
            ctx = web.repo.changectx(node)
            n = ctx.node()
            entry = {"author": ctx.user(),
                     "desc": ctx.description(),
                     "files": web.listfilediffs(tmpl, ctx.files(), n),
                     "rev": ctx.rev(),
                     "node": hex(n),
                     "tags": nodetagsdict(web.repo, n),
                     "branches": nodebranchdict(web.repo, ctx),
                     "inbranch": nodeinbranch(web.repo, ctx),
                     "hidden": "",
                     "push": [],
                     "mergerollup": [],
                     "Id": id
                     }

Then a change needed to be made to the hg_templatesgitweb_mozillamap file. The id needs to be stored in the class of the hidden rows and the expand row. So I made the following changes:

mergehidden = '<br/>← #count# hidden changesets <a class="expand hideidentifier#Id#" href="#">[Expand]</a>'
pushlogentry = '<tr class="parity#parity# #hidden# identifier#Id#">...

Now each expand row will have the class hideidentifier#id#, which is a unique identifier telling us which expand link is clicked. Then we can take this unique identifier and hide all the rows that have the same #Id#. To do this I needed to add some JavaScript code to hg_templatesgitweb_mozillapushlog.tmpl…

30
31
32
33
    var id = $(this).attr("class");
    id = '.' + id.substring(11, id.length);
    $(id).nextAll(id).toggle();
    return false;

So line 30 gets the id of the current object (the expand row whose [Expand] link is clicked by the user). So the id might be something like “hideidentifier2434″ after line 30 is executed. Next a ‘.’ character is added on the front and the id is substringed to give us something like “.identifier2434″. Now we have the class of the rows we want to hide/unhide and we can then execute line 32. Voila! Problem fixed.

Posted in Mercurial Project, Open Source | Tagged , , , , , , | Leave a comment

v0.3 Release – Problem with my Patch for bug 445560

I had a discussion with ted today relating to my patch for bug 445560 for the pushlog. There seems to be a slight problem with it. When there are 2 merges on one page, if one clicks the second expand link, it shows those changesets, but the row with the [Expand] link disappears (example of the problem). This is a problem that we don’t want happening.

Problem

Currently I’m using the date as a unique identifier (view here for full details) but if somehow (very unlikely, but possible) two rows have the same date then there will be a problem, which is the case with the example linked above.

Fix

This problem can be solved by using the id as the unique identifier instead of the date. This will work because each changeset has a different id and each set merge changesets have the same id so it will allow one to identify all the merge changesets to hide on a page and not hide the ones that shouldn’t be hidden.

I will be putting up a new patch to fix this issue.

Edit: I didn’t read the comments (comment #6) that ted put on bug 445560 for my patch before I made this post. There is another problem that he identified, which I need to fix. Apparently I’m not allowed to have multiple elements with the same id so I can’t be putting my unique identifiers in the id of the tag. ted recommended putting them in the class of the tag which is what I will be doing. Look out for a new blog post explaining the code.

Posted in Mercurial Project, Open Source | Tagged , , , , , , | Leave a comment

Looking Forward to v0.3 Release

The end of the semester is approaching fast and so the due of my  v0.3 release. I’ve had a talk with jorendorff about what I should do for my next release. We discussed working on the ability to filter by file/directory or a graph view for the pushlog. In the end we decided that working on a graph view for the pushlog would be a pretty amazing thing to work on.

Dirkjan Ochtman (djc) has already done some work on this graph view feature which you can see here. So now my job is to adapt and adjust the work done by djc for the pushlog. At this point I can’t really say how hard or easy this is going to be. All I know is that this feature would be quite awesome to have as far as the users are concerned.

I also will be looking to make fixes to the patches I submitted for my v0.2 release as the reviews come in.

With all the other assignments I have due the time to work on my v0.3 release isn’t too much so I need to be efficient with my work. Here’s hoping things go well…

Posted in Mercurial Project, Open Source | Tagged , , , , , , | Leave a comment

v0.2 Complete

“Takes a deep breath”

Well, I’ve finally completed my v0.2 release. I have to say that it has been tough and grueling. Throughout this period I have struggled to get things done countless times. I’ve hit the proverbial brick wall on so many occasions that I feel like I’m part of the wall now (the wall is like the annoying neighbor that won’t go away). Open source is definitely the course I have spent the most time on this semester. I’ve spent countless hours on this release (I’m guessing around 50-60hrs, at least it feels like that but I may be way off). But don’t get me wrong here. I’m not complaining. I’ve enjoyed myself too. These projects are hard and they will take time to complete. I know and accept that. I am getting better at working with my project too as with everyday that passes I feel I am more confident about my work.

I plan to release the patches for my 3 tasks (view here) tomorrow morning. They will all get reviewed by the Mozilla elders and I hope they get accepted. But, I am not naive. I don’t think they will get accepted immediately. This will be my first time providing a Mozilla patch and so I am sure that I probably have done something that they will not agree with. This is fine. It is all a learning process. Even people that have years of experience can get their patches rejected. Such a thing will not deter me if it happens.

ted, jorendorff and djc have been very helpful. Whenever I have had a question they have guided me onto the right track. Just have a look at my project page to see how many times I have discussed something with them.

We have less than a month left till the semester ends and I need to decide quickly on what I will be working on for my v0.3 release. At this moment I am too exhausted to decide. I will probably make my decision very soon so that I can get going on it. There isn’t much time left. Look out for a future blog post explaining what I will be doing.

Posted in Mercurial Project, Open Source | Tagged , , , , , | Leave a comment

v0.2 Release – Getting the Correct Chronological Order for bug 459727

Throughout my entire time working on a v0.2 release I haven’t been able to get the correct choronological order (it should be reverse chronological order) for the new entries that load OnScroll. This is an essential part of the pushlog as it shows users when the entries were added. I’m using a script called json-pushes to acquire the data I need to show. For each call to the script I need to provide a startID and endID. At first I started by calling the script like so:

  1. startID=0, endID=20
  2. startID=20, endID=40
  3. startID=40, endID=60
  4. startID=60, endID=80
  5. etc…

This didn’t work since since ID=1 is the very earliest entry in the pushlog when I should be starting with the later entries and working my way down to the earlier entries. So, what I needed to do was somehow acquire the ID of the latest entry in the pushlog. Thus, I wrote this new function in the server side file pushlog-feed.py

1
2
3
4
5
6
7
8
def getMaxEntries(repo):
  stmt = 'SELECT id from pushlog'
  max = 0
  conn = sqlite.connect(os.path.join(repo.path, 'pushlog2.db'))
  for id in conn.execute(stmt):
    max += 1
 
  return max

The above code provides me with the maximum ID in the pushlog. Now I need to provide this value to the client side so that I know which entry I should start off with when I make my first call to json-pushes. The following is the code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
function getMaxEntries() {
  var entries = new XMLHttpRequest();
  var max = 0;
  entries.open('GET', '/json-pushes?startID=0&endID=5', true);
  entries.onreadystatechange = function() {
  if(entries.readyState == 4)  {
    if(entries.status != 404) {
      var entryData = new Function("return " + entries.responseText) ();
      max = entryData[1].max;
 
      start = max - 20;
    }
  }
  else 
    return 0;
 } 
 entries.send(null);
}

So it turns out that I needed to make two XMLHttpRequest’s to first acquire the maximum ID (see above) and then the next one to actually acquire all the data to show OnScroll. Thus, now everytime the user reaches the end of the page json-pushes loads the IDs with the following pattern:

  1. startID=[MAX], endID=[MAX-20]
  2. startID=[MAX-20], endID=[MAX-40]
  3. startID=[MAX-40], endID=[MAX-60]
  4. startID=[MAX-60], endID=[MAX-80]
  5. etc…
Posted in Mercurial Project, Open Source | Tagged , , , , , , | Leave a comment

v0.2 Release Getting Merge Changesets to Display Properly for bug 459727

I’ve been working hard to make merge changesets display properly OnScrollDown for bug 459727. They weren’t appearing at all before since I didn’t implement any code for them. I started to work on doing so but I ran into a puzzling problem that jorendorff helped solve. I’ve managed to get merge changesets to display properly now. It wasn’t as hard as I had expected but the amount of code I had to write was quite long. There are probably ways to shrink it down a bit but I just don’t see how right now. Lets go over it…

The Code

The server side code can is explained here. The following is the client side code:

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
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
function renderMorePushLogResults() {
  //start += 20;
  //end = start + 19;
  var end = start;
  start = start - 20
  var pushCheckins = new XMLHttpRequest();
  pushCheckins.open('GET', '/json-pushes?startID=' + start + '&endID=' + end, true);
  pushCheckins.onreadystatechange = function() {
  if(pushCheckins.readyState == 4)  {
    if(pushCheckins.status != 404) {
      var loader = document.getElementById("loader");
      loader.innerHTML = '<img src="{url}static/ajax-loader.gif" align="right" />';
 
      var pushData = new Function("return " + pushCheckins.responseText) ();
      var counter = 0;
      for(var i = end; i > start; i--) {
        var trScroll = document.createElement("tr");
 
        if(counter == 0) {
          counter = 1;
        } else {
            trScroll.style.backgroundColor = "#f6f6f0";
            counter = 0;
        }      
 
        var tdScrollUser = document.createElement("td");
        tdScrollUser.width = "184px";
        tdScrollUser.innerHTML += '<i>' + pushData[i].user + '<br />' + pushData[i].formattedDate + '</i>';
 
        //Create changset link
        var tdScrollChangeset = document.createElement("td");
        tdScrollChangeset.innerHTML += 
         '<a href="/rev/' + 
         pushData[i].individualChangeset.substring(0, 12) + 
         '">' + 
         pushData[i].individualChangeset.substring(0, 12) + 
         '</a>';
 
        //Create buglink
        var bugInDesc = (pushData[i].desc).toLowerCase().indexOf("bug");
 
        if(bugInDesc != -1) {
          var bugLinkName = (pushData[i].desc).substring(bugInDesc, bugInDesc + 10);
          var bugNumber = bugLinkName.substring(4, 10);
          var bugLink = 
           (pushData[i].desc).substring(0, bugInDesc) + 
           '<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=' + 
           bugNumber + 
           '">' + 
           bugLinkName + 
           '</a>' + 
           (pushData[i].desc).substring(bugInDesc + 10, (pushData[i].desc).length);
        } else { //No bug provided
            var bugLink = pushData[i].desc;
        }
        var tdScrollAuthorDesc = document.createElement("td");
        tdScrollAuthorDesc.innerHTML += '<b>' + pushData[i].author + ' — ' + bugLink + '</b>';
 
        trScroll.appendChild(tdScrollUser);
        trScroll.appendChild(tdScrollChangeset);
        trScroll.appendChild(tdScrollAuthorDesc);
 
        loader.innerHTML = "";
        document.getElementById("titlePush").appendChild(trScroll);
 
        //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 mergeStr = pushData[i].mergeData[j];
              for(var k = 0; k < pushData[i].mergeData[j].length; k++) {
                var actualMergeStr = mergeStr[k].split('|-|');
 
                var mergeC = actualMergeStr[0];
                var mergeUser = actualMergeStr[1];
                var mergeDesc = actualMergeStr[2];
 
                if(mergeDesc != pushData[i].desc) {
 
                  var trScrollMerge = document.createElement("tr");
                  trScrollMerge.style.backgroundColor = trScroll.style.backgroundColor;
 
                  var tdScroll_MergeUser = document.createElement("td");
                  tdScroll_MergeUser.width = "184px";
 
                  //Create changset link
                  var tdScroll_MergeC = document.createElement("td");
                  tdScroll_MergeC.innerHTML +=
                   '<a href="/rev/' +
                   mergeC.substring(0, 12) + 
                   '">' + 
                   mergeC.substring(0, 12) +
                   '</a>';    
 
                  //Create buglink
                  var merge_bugInDesc = mergeDesc.toLowerCase().indexOf("bug");
                  if(merge_bugInDesc != -1) {
                    var merge_bugLinkName = mergeDesc.substring(merge_bugInDesc, merge_bugInDesc + 10);
                    var merge_bugNumber = merge_bugLinkName.substring(4, 10);
                    var merge_bugLink =
                    mergeDesc.substring(0, merge_bugInDesc) + 
                     '<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=' + 
                     merge_bugNumber + 
                     '">' + 
                     merge_bugLinkName + 
                     '</a>' + 
                     mergeDesc.substring(merge_bugInDesc + 10, mergeDesc.length);
                  } else { //No bug provided
                      var merge_bugLink = mergeDesc;
                  }
                  var tdScroll_MergeAuthorDesc = document.createElement("td");
                  tdScroll_MergeAuthorDesc.innerHTML += '<b>' + mergeUser + ' — ' + merge_bugLink + '</b>';
 
                  trScrollMerge.appendChild(tdScroll_MergeUser);
                  trScrollMerge.appendChild(tdScroll_MergeC);
                  trScrollMerge.appendChild(tdScroll_MergeAuthorDesc);
 
                  document.getElementById("titlePush").appendChild(trScrollMerge);
                }
              }
            }
          }
        }
 
      }    
    } 
   }
  }
  pushCheckins.send(null);
}

Everything before line 68 is for normal changesets that don’t have any merge changesets (in which case the whole section encapsulated within the if statement is skipped). Everything afterwords sets up the merge changesets.

Line 69 is a for loop that goes through pushData[i].mergeData which contains all the data for the merge changesets (changeset, user and description). Then on line 73 a split() is performed (the data is received as a string in the format: changeset|-|user|-|description) to get the values of the changeset, user and description. Instead of putting the data within a character delimited string I could have created a class but this implementation meant writing less lines of code which is almost always better.

On line 81 the row is created and the background color is set. The first column is then created (it will be empty) and the width is set to get the right alignment. Lines 87-95 the changeset link is created. Lines 97-114 the buglink is created via some cumbersome string manipulation. Lines 116-118 all three tds are added to the row and then on line 120 the row is added to the table.

The Result

The above code gives me the following result: http://sidkalra.com/files/mercurial/mergeChangsets.png

Posted in Mercurial Project, Open Source | Tagged , , , , , | Leave a comment

v0.2 Release – Concerning the Chronological Order for bug 459727

Currently the pushlog loads entries in reverse chronological order (the latest entries are shown on top instead of on the bottom). I want to maintain this functionality for the new entries that are loaded OnScroll. For my current implementation this is not occurring. Have a look at this example: http://sidkalra.com/files/mercurial/improvedPushlog2.png

Examine the date of the last entry before new items are loaded (Mon Sept 29 17:20:28 2008 – 0400) and then have a look at the date of the first new entry (Fri May 02 14:38:33 2008 – 0400). Even though this is reverse chronological order it is not correct since all the entries between May 02 and Sept 29 are missing.

Currently to load more entries OnScroll I am using a script that returns all the data I need. This script takes in two values, startID and endID. So this is what I’m doing currently:

  1. On first scroll: startID = 0, endID = 20
  2. On second scroll: startID = 21, endID = 40
  3. On third scroll: startID = 41, endID = 60
  4. and so on…

This obviously isn’t getting me the right data. I don’t understand which startID and endID I should start with. Having the startID = 0 obviously isn’t working. I’m kind of stuck on this issue.

From what I understand, the whole point of this feature is to mirror the functionality of pressing the Next link. When a user scrolls to the bottom of the page, the new entries should be identical to the entries that appear if one were to click on the next link.

Posted in Mercurial Project, Open Source | Tagged , , , , , | Leave a comment

v0.2 Release – On Initial PageLoad show 20 entries instead of 10 for bug 459727

Currently if you have a look at the pushlog on the initial PageLoad only 10 entries are shown. I’ve decided that I want the pushlog to initially show 20 entries instead of 10. There are 2 reasons for this, let me explain:

  • OnScroll Event doesn’t fire- If the code there are only 10 entries initially, the OnScroll event doesn’t fire since there is no scroll bar present. By making the initial number of entries 20, the scroll bar appears. Now, when the user attempts to scroll down the OnScroll Event is able to fire appropriately (see the below)

  • 1
    2
    3
    4
    5
    
    $(window).scroll(function() {
      if($(window).scrollTop() == $(document).height() - $(window).height()) {
        renderMorePushLogResults();
      }
    });

    renderMorePushLogResults(), which loads the new entries is fired when the user reaches the end of the page.

  • Maintaining Consistency – When the OnScroll event is fired (when the user reaches the end of the page) 20 more entries are loaded each time. To maintain consistency with this pattern the initial number of entries should also be the same number

The Code

To make this change I needed to make a couple of back-end changes to the pushlog-feed.py file. Thankfully, there wasn’t need to make any drastic changes. There is a global variable called PUSHES_PER_PAGE, which I set to 20. I also made the following change on line 144 in pushlogSetup()

144
e = getpushlogentries(conn, (page - 1) * PUSHES_PER_PAGE, 20, tipsonly)

That’s all the changes I needed to make to implement my change.

Posted in Mercurial Project, Open Source | Tagged , , , | Leave a comment

v0.2 Release – Showing the Correct Author Name for bug 459727

My v0.2 for the pushlog is coming along at a steady pace. There are still some problems but hopefully I will have them solved soon. Right now, the following is how new entries look like OnScrollDown:

http://sidkalra.com/files/mercurial/improvedPushlog2.png

Now have a look at the author column (in the above image). Sometimes the text under that column is a name while other times it is an email. This is not the functionality I want. It should always be a name, never the email (there is already a column for that). Currently I am using ctx.user() to populate the author column. This obviously doesn’t give me the right data. I can’t use the user column straight from the db (I already use it to populate the email column) since it doesn’t give me the author name either.

I need to find some other way of acquiring the author name. I was wondering if there is some other ctx function that I could use to acquire the right data. I tried guessing it by going ctx.author() or ctx.person() or ctx.name() but no luck so far.

I had a look at how it is done for the initially loaded data. The answer is in the map file. The author name is printed by going #author|person#. I don’t really understand what that means and how this variable gets the correct author name for each row. I’ll have to consult ted, djc or jorendorff on this issue.

EDIT: I’ve managed to fix this issue. It was actually quite simple in the end. I had a talk with ted and we discussed what #author|person# does. He said it was just a filter that takes whatever is in ctx.user() and tries to remove the email. I needed to do something similar for my solution so bsmedberg recommended importing mercurial.templatefilters.person which will allow me to perform person(ctx.user()) which returns the name every time. Problem solved! View the result

Posted in Mercurial Project, Open Source | Tagged , , , , | Leave a comment

v0.2 Release – Show Files Touched by a Changeset in the Pushlog

As part of my v0.2 release I wanted to fix bug 448707. It is aparently a very desired feature by the community. Adding this featue definitely makes it easier to immediately find out which files were touched while right now one has to click the changset link to do that.

Initial Thoughts

I talked to ted and jorendorff about how they thought I should go about implementing this feature. ted recommended using the http://hg.mozilla.org/mozilla-central/pushlog feed which returns the files touched. I thought I could use an XMLHttpRequest to get the data to the front-end and then format it. But after looking at the code I decided that that wasn’t the way to go. I decided to format the data on the back-end and then just print it through the map file exactly how everything else in the pushlog is rendered

Implementation

So, as I mentioned before I had to make some back-end changes (changelist function in pushlog-feed.py and map). The following is the code:

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
    def changelist(limit=0, **map):
        #pdb.set_trace()
        allentries = []
        lastid = None
        ch = None
        l = []
        mergehidden = ""
        p = 0
        currentpush = None
        for id, user, date, node in entries:
            filesTouched = ''
            ctx = web.repo.changectx(node)
            n = ctx.node()
            if len(ctx.files()) > 1:
              for f in ctx.files():
                filesTouched += f + '<br/>'
            else:
              for f in ctx.files():
                filesTouched += f
            entry = {"author": ctx.user(),
                     "desc": ctx.description(),
                     "files": web.listfilediffs(tmpl, ctx.files(), n),
                     "rev": ctx.rev(),
                     "node": hex(n),
                     "tags": nodetagsdict(web.repo, n),
                     "branches": nodebranchdict(web.repo, ctx),
                     "inbranch": nodeinbranch(web.repo, ctx),
                     "hidden": "",
                     "push": [],
                     "mergerollup": [],
                     "dateId": localdate(date),
                     "filesTouched": filesTouched
                     }

So basically, there is a function that gives me all the files touched by a changeset, ctx.files(). I just call the function append each file touched by a changeset into a string (filesTouched, in this case). Once I add it to the entry array I can then perform the following in the map file:

1
pushlogentry = '<tr id="date#dateId#" class="parity#parity# #hidden# #dateId#"><td>#push%pushinfo#</td><td class="age"><a href="{url}rev/#node|short#{sessionvars%urlparameter}">#node|short#</a></td><td id="#dateId#"><strong>#author|person# &mdash; #desc|strip|escape|buglink#</strong> <span class="logtags">{inbranch%inbranchtag}{branches%branchtag}{tags%tagtag}</span>#mergerollup%mergehidden#<br/><span style="font-size: x-small; color: #FF902B; font-weight: bold">#filesTouched#</span></td></tr>n'

I added:

1
<br/><span style="font-size: x-small; color: #FF902B; font-weight: bold">#filesTouched#</span>

The above line of code gets the filesTouched variable from the back-end and formats it using CSS styles to display within on the page.

The following is the result of the obove code: http://sidkalra.com/files/mercurial/filesTouched1.png

This way it is easy to tell which files a changeset touched. A users knows it right away, no need to click on any other links to find out. Only thing I am unsure about is the text color of the files touched. At first I had gone with red but that was a bit too bold, so I decided to go with orange but I don’t know if it fits the theme well. As far as I can tell it seems like an OK choice but I might (or might not) change it in the future.

EDIT: I’ve changed the color of the text to a dark brown so that it is easier to read. I felt that the previous orange color was a bit too harsh on the eyes. Have a look: http://sidkalra.com/files/mercurial/filesTouched2.png

Posted in Mercurial Project, Open Source | Tagged , , , , , | Leave a comment