Posts Tagged Mercurial

v0.6 Release - Refactoring to Fix the Bitrotting Issue with Bug 459727

Posted by Sid in DPS911, Mercurial Project, Open Source on February 21st, 2009

I had mentioned in my previous blog post that hgpoller/pushlog-feed.py had bitrotted. One of my goals for this release was to make changes to the current version of pushlog-feed.py so that my patch is no longer broken for bug 459727. I’ve finally made those changes, which mainly occur in pushes_worker(). The following is what this method looks like with my changes:

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

def pushes_worker(query, repo):
    """Given a PushlogQuery, return a data structure mapping push IDs
    to a map of data about the push."""
    pushes = {}
    for id, user, date, node in query.entries:
        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:
            # we get the pushes in reverse order
            pushes[id]['changesets'].insert(0, 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

Basically I had to pass in repo (web.repo) so that I could have access to repo.changectx(node). This now allows me access to ctx.parents() which I need to retrieve merge changeset data. I also went through the whole file and changed every instance where pushes_worker() was called so that repo was being passed in as a paramater along with query.

These are all the changes I needed to make to the server side code. Now I’ll have to examine the changes that were made to the client side which caused my patch to bitrot.

, , , , ,

No Comments

v0.6 Release - Examining the Changes with hgpoller/pushlog-feed.py

Posted by Sid in DPS911, Mercurial Project, Open Source on February 20th, 2009

I had mentioned in my v0.6 goals blog post that my patch for bug 459727 had bitrotted. Unfortunately significant changes were made to hgpoller which apparently broke my patch. I need to remedy this situation because all my server side functionality for this patch is in the pushlog-feed.py file.

I downloaded the latest hgpoller source code and had a look at the changes that had been made. The file has changed in quite a few places. It seems that the function that my patch alters has been changed as well. I’m talking about pushes_worker(), which is repsonsible for passing the data from the server side to the client side.

1
2
3
4
5
6
7
8
9
10
11
12
13
14

def pushes_worker(query):
    """Given a PushlogQuery, return a data structure mapping push IDs
    to a map of data about the push."""
    pushes = {}
    for id, user, date, node in query.entries:
		if id in pushes:
			# we get the pushes in reverse order
			pushes[id]['changesets'].insert(0, node)
		else:
			pushes[id] = {'user': user,
						  'date': date,
						  'changesets': [node]
					     }
	return pushes

Now, the problem is that I need access to web.repo within pushes_worker() so that I can call repo.changectx(node) but right now, I don’t have access to repo within the method. I’ll have to figure out a way to do that somehow.

, , ,

No Comments

v0.4 Release Complete

Posted by Sid in DPS911, Mercurial Project, Open Source on January 23rd, 2009

As the title suggests my 0.4 release is complete! I’ve managed to fulfill all the goals (and more) that I set out to complete. I’ve posted a new patch for hg_templates containing all my changes. The following are all the related links:

View the project page for more details.

, , ,

No Comments

v0.4 Release - Merge Changesets Background Color Problems

Posted by Sid in DPS911, Mercurial Project, Open Source on January 22nd, 2009

I was testing bug 459727 again today and looks like there is a major problem that I didn’t notice before. Have a look at the screen shot below. Notice the problem?

No? Let me tell you then. The problem is that the grey rows and the white rows are not alternating properly. Also sometimes the email and date column are appearing at the bottom of the row instead of at the top. Why is that happening? Well, after reaching the point of being able to see the pushlog when I close my eyes I figured it out. I noticed that no merge changesets have grey backgrounds. That is why sometimes the text of the email and date column appears to be at the bottom of the row.

I had a look at how I was setting the background color of merge rows. I was using the following code:

var trScrollMerge = document.createElement("tr");
trScrollMerge.style.backgroundColor = trScroll.style.backgroundColor;

I commented it out and tested the pushlog again and found that there was no difference which means that the above code was doing nothing. To fix this issue I inserted the following code in its place:

var trScrollMerge = document.createElement("tr");
trScrollMerge.className = trScroll.className;

The above code gives me the following result which solves the issue:

This is another another bug fix that I can add to my v0.4 release.

, ,

No Comments

v0.4 Release - Loading More Entries onPageLoad

Posted by Sid in DPS911, Mercurial Project, Open Source on January 22nd, 2009

One of my goals for this release was to get the loading more entries onPageLoad functionality working properly concerning bug 459727. Currently I was getting an unresponsive script error. I’ve been struggling with this the last few days. I played around with this problem for a while in the hope of finding a viable solution.

The goal is to get more entries to load onPageLoad in order to fill out the entire page so that the scroll bar shows up. This will allow the user to scroll down and load further entries. However, the problem is that different users can have different screen sizes and/or resolutions and thus it is important that the number of entries loaded is dynamic.

To get rid of the unresponsive script error I tried using the following code:

var start;
function setup()
{
    start = 0;
    getMaxEntries();
 
    var docHeight = document.body.clientHeight;
    var desiredHeight = window.innerHeight;
 
    var i = Math.ceil(window.innerHeight / document.body.clientHeight);
 
    for(var counter = 0; counter < i; counter++) {
        if(start > 0) {
	    loadEntries();
       }
    }
}

The above code doesn’t work. Nothing happens onPageLoad, only the initial set of entries are loaded. I think this is because JavaScript doesn’t wait for getMaxEntries() to finish and moves on to the next line of code thus when the if(start > 0) statement is encountered the value of start is still 0 and loadEntries() is never called.

Somehow, I needed JavaScript to let getMaxEntries() execute completely before moving on to the rest of the code. The value of start is set in getMaxEntries() and loadEntries() will never get called unless the value of start is something greater than 0. I needed some sort of wait() or sleep() functionality. The closest thing I could find to wait() or sleep() in JavaScript was setTimeout(), which delays execution of the function for a set amount of time. The following is the code I came up with:

var start;
function setup()
{
    start = 0;
    getMaxEntries();
 
    var docHeight = document.body.clientHeight;
    var desiredHeight = window.innerHeight;
 
    setTimeout("if(start > 0) {loadEntries(Math.ceil(window.innerHeight / document.body.clientHeight)); }", 1000);
}

Now, the above code sets start to 0 and then executes getMaxEntries(). Then JavaScript moves onto executing the next line but setTimeout() delays the execution of loadEntries(). By the time loadEntires() gets executed the new value of start has been set and loadEntries() works properly.

I tested the above code and it works, more entries are loaded onPageLoad and the scroll bar appears. The only draw back being that Firefox hangs for a bit before the entries load. I don’t know if that is acceptable or not. I’ll put up the new patch tomorrow and lets see what ted and/or jorendorff have to say on this issue.

, , , ,

No Comments

v0.4 Release - XSS vulnerabilities and Loader.gif Issues

Posted by Sid in DPS911, Mercurial Project, Open Source on January 19th, 2009

I’ve decided to fix two extra features regarding my v0.4 release. During the course of figuring out how to implement other features I was able to fix these two issues as well so I decided to add them to the release. These fixes are concerning bug 459727.

XSS Vulnerability

In renderMorePushLogResults(), the main work horse that does all the work of adding new entries; I was creating an object from the JSON text received from the server:

1

var pushData = new Function("return " + pushCheckins.responseText) ();

Apparently the above makes the feature vulnerable to XSS attacks. I’m not an expert on security issues but this line has been pointed out to me on the bug page. So I decided to use the advice given to me and use JSON.parse() instead.

1

var pushData = JSON.parse(pushCheckins.responseText);

At first I thought that JSON was built into Javascript but I was wrong since I tried using JSON.parse() and got the error: “JSON is undefined”. After getting some help from IRC I found out that I needed a JSON library. Then, I headed over to good old Google and found the lib I was looking for here. Once I had the lib all I needed to do was add the following and everything worked again:

1

<script type="text/javascript" src="{url}static/json2.js"></script>

Loader.gif

I had already implemented the loader.gif last semester. I thought it was working and wasn’t showing up because I was testing on localhost and the load times were so fast that I wasn’t able to see the loader.gif

I was wrong. The loader.gif wasn’t showing up at all. To fix this all I needed to do was move the code that inserts the loader.gif to the very start of renderMorePushLogResults(). See below:

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

function renderMorePushLogResults() {
  var loader = document.getElementById("loader");
  loader.innerHTML = '<img src="{url}static/ajax-loader.gif" align="right" />';
 
  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 pushData = JSON.parse(pushCheckins.responseText);
        var counter = 0;
        for(var i = end; i > start; i--) {
          var trScroll = document.createElement("tr");
          if(counter == 0) {
            trScroll.className = "parity0"; 
            counter = 1;
          } else {
              counter = 0;
              trScroll.className = "parity1";
          }      
          var tdScrollUser = document.createElement("td");
          tdScrollUser.width = "184px";
          tdScrollUser.innerHTML += pushData[i].user + '<br />' + pushData[i].formattedDate;
          //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 bug link
		  var bugLink = createBuglink(pushData[i].desc);
 
          var tdScrollAuthorDesc = document.createElement("td");
          tdScrollAuthorDesc.innerHTML += '<strong>' + pushData[i].author + ' &mdash ' + bugLink + '</strong>';
          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>'; 
 
                    var merge_bugLink = createBuglink(mergeDesc);
 
                    var tdScroll_MergeAuthorDesc = document.createElement("td");
                    tdScroll_MergeAuthorDesc.innerHTML += '<strong>' + mergeUser + ' &mdash ' + merge_bugLink + '</strong>';
                    trScrollMerge.appendChild(tdScroll_MergeUser);
                    trScrollMerge.appendChild(tdScroll_MergeC);
                    trScrollMerge.appendChild(tdScroll_MergeAuthorDesc);
                    document.getElementById("titlePush").appendChild(trScrollMerge);
                  }
                }
              }
            }
          }
        }    
      } 
    }
  }
  pushCheckins.send(null);
}

, , ,

1 Comment

v0.4 Release - Linkifying all Bug Strings

Posted by Sid in DPS911, Mercurial Project, Open Source on January 18th, 2009

This post is again concerning my fixes for bug 459727. One of my goals for this release is to fix the bug link functionality. I’ve made some progress on that front in terms of creating correct bug links for the differnt types of patterns presented. This time I decided to tackle the other issue with bug links.

The problem is that for any given string that contains bug links only the very first bug string is converted into a link. The subsequent ones fail to get converted. Case in point below:

Both the first bug string “bug 437288″ and the second one, “bug 437288″ should be converted into buglinks but only the first one actually gets converted. This is what I’m looking to fix.

Now, this problem had me stumped for a while. I tried solving it using regexp functions in JavaScript but that was a dead end. I tried to think up other ways but none of them seemed to make sense to me.

Yesterday, while staring at my computer and thinking about this issue it just hit me, a perfect example of the light bulb effect. The answer to my problem was RECURSION!!! A concept I had come to dispise during my data structures and alogrithms class. I had never really used it in any of my programs before. This would be the first time.

Lets jump into the code as I explain this further. I have my main work horse function that creates the bug links:

function createBuglink(str) {
  //Create buglink
  var matchFound = new Boolean(false);
  var bugInDesc = 0;
  var pattern = -1;
  var bugLink = "";
 
  re = new RegExp("[bB][uU][gG]\\s\\d\{6\}");
  re2 = new RegExp("b=\\d\{6\}");
  re3 = new RegExp("\\d\{6\}\\W");
 
  bugInDesc = str.search(re);
  if(bugInDesc != -1) {
    matchFound = true;
	pattern = 1;
  } else {
      bugInDesc = str.search(re2);
	  if(bugInDesc != -1) {
	    matchFound = true;
	    pattern = 2;
	  } else {
	      bugInDesc = str.search(re3);
	      if(bugInDesc != -1) {
	        matchFound = true;
		    pattern = 3;
		  }
	  }
  }
 
  if(matchFound == true) {
    if(pattern == 1) {
	  bugLink = bugLinkify(bugInDesc, str, 4, 10);
	} else if(pattern == 2) {
	  bugLink = bugLinkify(bugInDesc, str, 2, 8);
    } else if(pattern == 3) {
	  bugLink = bugLinkify(bugInDesc, str, 0, 6);
    }	
  } else { //No bug provided
      bugLink = str;
    }
 
  return bugLink;
}

Then I have a function that does the actual string manipulation to insert the link code into the string which is called within createBugLink():

function bugLinkify(bugInDesc, str, start, end) {
  var bugLinkName = str.substring(bugInDesc, bugInDesc + end);
  var bugNumber = bugLinkName.substring(start, end);
  var bugLink = 
    str.substring(0, bugInDesc) + 
    '<a href=\"https://bugzilla.mozilla.org/show_bug.cgi?id=' + 
    bugNumber + 
    '\">' + 
    bugLinkName + 
    '</a>' + 
    createBuglink(str.substring(bugInDesc + end, str.length));
 
  return bugLink;
}

Now, in bugLinkify() the genius of recursion gets to take over. I just call the createBuglink() function again on the rest of the string that didn’t get converted into a buglink and keeps gettig called until there are no bug strings left to be converted.

The upside to this method is that a string could contain an unlimited amount of bug strings and they would all get converted into bug links. But the thing I love most about this method is that I didn’t have to add even a SINGLE line of code. That fact makes this implementation awesome. Gaining functionality without having to add more lines of code is always great.

The following are the results:

, , , , ,

1 Comment

v0.4 Release - Better regexps for Creating bugLinks

Posted by Sid in DPS911, Mercurial Project, Open Source on January 17th, 2009

This is concerning bug 459727. As one of my goals for this release I wanted to create better regexps in order to match all possible bugLink patterns i.e. bug 123455, b=134566, 345777 or BUG 293455

To do this I came up with 3 different regexps which match the above patterns and nothing else. 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
34
35
36
37
38
39
40
41
42
43

function createBuglink(str) {
  //Create buglink
  var matchFound = new Boolean(false);
  var bugInDesc = 0;
  var pattern = -1;
  var bugLink = "";
 
  re = new RegExp("[bB][uU][gG]\\s\\d\{6\}");
  re2 = new RegExp("b=\\d\{6\}");
  re3 = new RegExp("\\d\{6\}\\W");
 
  bugInDesc = str.search(re);
  if(bugInDesc != -1) {
    matchFound = true;
	pattern = 1;
  } else {
      bugInDesc = str.search(re2);
	  if(bugInDesc != -1) {
	    matchFound = true;
	    pattern = 2;
	  } else {
	      bugInDesc = str.search(re3);
	      if(bugInDesc != -1) {
	        matchFound = true;
		    pattern = 3;
		  }
	  }
  }
 
  if(matchFound == true) {
    if(pattern == 1) {
	  bugLink = bugLinkify(bugInDesc, str, 4, 10);
	} else if(pattern == 2) {
	  bugLink = bugLinkify(bugInDesc, str, 2, 8);
    } else if(pattern == 3) {
	  bugLink = bugLinkify(bugInDesc, str, 0, 6);
    }	
  } else { //No bug provided
      bugLink = str;
    }
 
  return bugLink;
}

As you can see once a match is found (bugInDesc is not -1) the pattern being used in the string is identified and then bugLinkify() is called which is responsible for actually creating the link.

1
2
3
4
5
6
7
8
9
10
11
12
13
14

function bugLinkify(bugInDesc, str, start, end) {
  var bugLinkName = str.substring(bugInDesc, bugInDesc + end);
  var bugNumber = bugLinkName.substring(start, end);
  var bugLink = 
    str.substring(0, bugInDesc) + 
    '<a href=\"https://bugzilla.mozilla.org/show_bug.cgi?id=' + 
    bugNumber + 
    '\">' + 
    bugLinkName + 
    '</a>' + 
    str.substring(bugInDesc + end, str.length);
 
  return bugLink;
}

Using this method the 3 different patterns are converted into bug links properly. See results below

, , , ,

3 Comments

v0.4 Release - Optimizing Client Side Code

Posted by Sid in DPS911, Mercurial Project, Open Source on January 17th, 2009

This is concerning bug 459727. As one of my goals for this release I’m trying to maintain/add functionality but reduce the amount of code. I started off by perusing the code in renderMorePushLogResults() found in the hg_templates/pushlog.tmpl file (see below).

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

function renderMorePushLogResults() {
  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) {
            trScroll.className = "parity0"; 
            counter = 1;
          } else {
              counter = 0;
              trScroll.className = "parity1";
          }      
          var tdScrollUser = document.createElement("td");
          tdScrollUser.width = "184px";
          tdScrollUser.innerHTML += pushData[i].user + '<br />' + pushData[i].formattedDate;
          //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
          re = new RegExp("[bB][uU][gG]\\s\\d\{6\}");
          var bugInDesc = (pushData[i].desc).search(re); 
          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 += '<strong>' + pushData[i].author + ' &mdash ' + bugLink + '</strong>';
          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
                    reg = new RegExp("[bB][uU][gG]\\s\\d\{6\}");
                    var merge_bugInDesc = mergeDesc.search(reg);
                    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 += '<strong>' + mergeUser + ' &mdash ' + merge_bugLink + '</strong>';
                    trScrollMerge.appendChild(tdScroll_MergeUser);
                    trScrollMerge.appendChild(tdScroll_MergeC);
                    trScrollMerge.appendChild(tdScroll_MergeAuthorDesc);
                    document.getElementById("titlePush").appendChild(trScrollMerge);
                  }
                }
              }
            }
          }
        }    
      } 
    }
  }
  pushCheckins.send(null);
}

I found that the bugLink code was being repeated. I was using it to create bugLinks for both normal and merge changesets. I decided to move the bugLink code to a function to stop the code repetition. By doing this I was able to reduce the number of lines of code by 27 lines (see below).

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

function renderMorePushLogResults() {
  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) {
            trScroll.className = "parity0"; 
            counter = 1;
          } else {
              counter = 0;
              trScroll.className = "parity1";
          }      
          var tdScrollUser = document.createElement("td");
          tdScrollUser.width = "184px";
          tdScrollUser.innerHTML += pushData[i].user + '<br />' + pushData[i].formattedDate;
          //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 bug link
          var bugLink = createBuglink(pushData[i].desc);
 
          var tdScrollAuthorDesc = document.createElement("td");
          tdScrollAuthorDesc.innerHTML += '<strong>' + pushData[i].author + ' &mdash ' + bugLink + '</strong>';
          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>'; 
 
                    var merge_bugLink = createBuglink(mergeDesc);
 
                    var tdScroll_MergeAuthorDesc = document.createElement("td");
                    tdScroll_MergeAuthorDesc.innerHTML += '<strong>' + mergeUser + ' &mdash ' + merge_bugLink + '</strong>';
                    trScrollMerge.appendChild(tdScroll_MergeUser);
                    trScrollMerge.appendChild(tdScroll_MergeC);
                    trScrollMerge.appendChild(tdScroll_MergeAuthorDesc);
                    document.getElementById("titlePush").appendChild(trScrollMerge);
                  }
                }
              }
            }
          }
        }    
      } 
    }
  }
  pushCheckins.send(null);
}

Reduced lines of code needed to achieve the same functionality also reduces the chance of bugs occurring, which is always a good thing.

, , ,

1 Comment

A New Beginning

Posted by Sid in DPS911, Mercurial Project, Open Source on January 15th, 2009

I’m finally back from my month long vacation and a new semester at Seneca College has begun. I spent most of the holidays trying to take it easy and get some rest. It was a welcome change from the frenzied quick pace of last semester.

This semester (my last at Seneca) I will be continuing my work with hgweb but in a new course setting, DPS911 the successor of my previous open source course. David Humphery will be mentoring us once again but the onus is on the students to get the work done.

For this course the goal is to make 7 releases so that the project goes from a version 0.3 to a version 1.0 in a matter of 3 months. We will be making a release at a rate of 1 every 2 weeks so the students will have to be constantly working on their projects to keep up.

I’ve created a project description and plan for this course which will help me visualize and keep track of what needs to be done. Lets hope things go well

Ready, set, GO!

, ,

No Comments