So, as I had mentioned in my goals blog post, I would be updating my files touched patch to work with the new code now found in the repo. First of all lets have a look at my old code:
248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 | def changelist(limit=0, **map): allentries = [] lastid = None ch = None l = [] for id, user, date, node in entries: filesTouched = '' ctx = web.repo.changectx(node) if len(ctx.files()) > 1: for f in ctx.files(): filesTouched += f + '<br/>' else: for f in ctx.files(): filesTouched += f if id != lastid: lastid = id l.append({"parity": parity.next(), "user": user, "date": localdate(date), 'numchanges': 0, "changes": []}) ch = l[-1]['changes'] ctx = web.repo.changectx(node) n = ctx.node() ch.append({"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), "parity": l[-1]["parity"], "filesTouched": filesTouched }) l[-1]['numchanges'] += 1 if limit > 0: l = l[:limit] for e in l: yield e |
pushlogchange = '<td class="age"><a href="{url}rev/#node|short#{sessionvars%urlparameter}">#node|short#</a></td><td><strong>#author|person# — #desc|strip|escape|buglink#</strong><br/><span style="font-size: x-small; color: #996633; font-weight: bold">#filesTouched#</span> <span class="logtags">{inbranch%inbranchtag}{branches%branchtag}{tags%tagtag}</span></td></tr><tr class="parity#parity#">' |
pushlog-feed.py has changed quite a bit since I submitted this patch a few months back. Thus, I’ll have to change my code a bit. Even the map file has changed but my code itself (for the map file) won’t be changing much. I’ll just be placing it in a different area. Also, as you can see above I have the CSS within the span tag. That has to be moved to the a stylesheet.
Lets look at my solution:
365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426 427 | def changelist(limit=0, **map): # useless fallback listfilediffs = lambda a,b,c: [] if hasattr(webutil, 'listfilediffs'): listfilediffs = lambda a,b,c: webutil.listfilediffs(a,b,c, len(b)) elif hasattr(web, 'listfilediffs'): listfilediffs = web.listfilediffs allentries = [] lastid = None ch = None l = [] mergehidden = "" filesTouched = "" p = 0 currentpush = None for id, user, date, node in query.entries: ctx = web.repo.changectx(node) n = ctx.node() entry = {"author": ctx.user(), "desc": ctx.description(), "files": 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": [], "filesTouched": filesTouched, "id": id } if id != lastid: lastid = id p = parity.next() entry["push"] = [{"user": user, "date": localdate(date)}] if len([c for c in ctx.parents() if c.node() != nullid]) > 1: mergehidden = "hidden" entry["mergerollup"] = [{"count": 0}] else: mergehidden = "" currentpush = entry else: entry["hidden"] = mergehidden if mergehidden: currentpush["mergerollup"][0]["count"] += 1 entry["parity"] = p for f in ctx.files(): entry["filesTouched"] += '<br/>' + f l.append(entry) if limit > 0: l = l[:limit] for e in l: yield e |
pushlogentry = '<tr class="parity#parity# #hidden# id#id#"><td>#push%pushinfo#</td><td class="age"><a href="{url}rev/#node|short#{sessionvars%urlparameter}">#node|short#</a></td><td><strong>#author|person# — #desc|strip|escape|buglink#</strong><span class="filetouch">{filesTouched}</span> <span class="logtags">{inbranch%inbranchtag}{branches%branchtag}{tags%tagtag}</span>#mergerollup%mergehidden#</td></tr>n' |
span.filetouch { font-family: sans-serif; color: #996633; } |
I have reduced the server side code a little but I’ve managed to retain the same functionality, which is always great. I moved the declaration of the filesTouched variable to the top where the rest of the variable declarations occur.
So, the entry dictionary holds the filesTouched key/value. I set the value in my for loop at line 415. In my previous code I had an if/else statement, which I have now removed. This is because before I had a static line break in the map file right before the files are displayed. I removed that and just decided to add a line break before each file is displayed (line 416). Now I don’t have to worry about whether there is only one file touched or more than one and add a line break accordingly (which is what the old if/else statement was for).
Also, as you can see the CSS now cleanly resides in the style-gitweb.css stylesheet. I still don’t know about the text color though. Does it match the color scheme of the gitweb_mozilla template? Not really but I couldn’t find any other color that was both readable and matched the theme. I guess I could have left the text black but I wanted to differentiate it somehow. I guess this is the difference between a programmer and a designer eh?
In the future (maybe my next release) I’ll look to add expand/collapse functionality for this bug since many times just one push can touch a large amount of files. For now, I hope this gets reviewed and approved soon so that people can start reaping its benefits.