Add a `format` parameter to the RecentChanges macro

[Ryan J Ollos <ryano@…>]

Add a format parameter to the RecentChanges macro with the following allowed values:

• groupbydate (default) - the current display format
• list - list each page on a separate line, a bullet ed list, same as the current display but without the date shown.

For reference, two macros accept a format parameter: WikiMacros#TitleIndex-macro and WikiMacros#TicketQuery-macro.

A patch will be provided shortly.

[ryano@…]

First cut at a patch.

[anonymous]

There is an extra whitespace in the help text that should be corrected before committing the patch. The current behavior is that any value for format other than format=groupbydate

Here are the test cases I used:

{{{
[[RecentChanges]]
}}}
[[RecentChanges]]

{{{
[[RecentChanges(,3)]]
}}}
[[RecentChanges(,3)]]

{{{
[[RecentChanges(SandBox,3)]]
}}}
[[RecentChanges(SandBox,3)]]

{{{
[[RecentChanges(SandBox)]]
}}}
[[RecentChanges(SandBox)]]

{{{
[[RecentChanges(SandBox,format=list)]]
}}}
[[RecentChanges(SandBox,format=list)]]

{{{
[[RecentChanges(format=list)]]
}}}
[[RecentChanges(format=list)]]


{{{
[[RecentChanges(, 5, format=list)]]
}}}
[[RecentChanges(, 5, format=list)]]


{{{
[[RecentChanges(SandBox, 5, format=list)]]
}}}
[[RecentChanges(SandBox, 5, format=list)]]

{{{
[[RecentChanges(SandBox, 5, format=groupbydate)]]
}}}
[[RecentChanges(SandBox, 5, format=groupbydate)]]

{{{
[[RecentChanges(SandBox, format=groupbydate)]]
}}}
[[RecentChanges(SandBox, format=groupbydate)]]

{{{
[[RecentChanges(format=groupbydate)]]
}}}
[[RecentChanges(format=groupbydate)]]

{{{
[[RecentChanges(format=groupbydate, , 3)]]
}}}
[[RecentChanges(format=groupbydate, , 3)]]

{{{
[[RecentChanges(format=groupbydate, SandBox, 3)]]
}}}
[[RecentChanges(format=groupbydate, SandBox, 3)]]

[anonymous]

Submitted my previous comment before finishing what I was writing …

The current behavior is that any value for format other than format=groupbydate results in the list format. I was thinking this might be expanded on in the future to add a format=compact option.

[Christian Boos]

Nice proposal, thanks!

Two remarks:

• as this is 0.13 material, please use the x if b else y style instead of b and x or y (see #9536)
• for format=list, I think it would be more appropriate to generate a single list; even if this will probably make the patch a bit longer, it more important to have a semantically correct output

[anonymous]

Replying to cboos:

Nice proposal, thanks!

Two remarks:

• as this is 0.13 material, please use the x if b else y style instead of b and x or y (see #9536)
• for format=list, I think it would be more appropriate to generate a single list; even if this will probably make the patch a bit longer, it more important to have a semantically correct output

I've incorporated those changes. I'm not super happy with the list generation … perhaps someone will be more clever than I have been in coding this. However, it seems to work correctly at least.

[Christian Boos]

Replying to anonymous:

I've incorporated those changes.

Nice, thanks!

I'm not super happy with the list generation …

For example you could add an extra transformation step to entries_per_date and factor out the part:

tag.li(tag.a(page, href=formatter.href.wiki(name)), ' ',  ...
... for date, entries in entries_per_date

into:

items_per_date = [(date, [tag.li(...) 
                         for ...])
                  for ... in entries_per_date]

and from that produce either the (<h3>, <ul>) sequence or a single <ul>.

[Ryan J Ollos <ryano@…>]

Replying to cboos:

For example you could add an extra transformation step to entries_per_date and factor out the part: …

Thank you for the pointers! I attempted to incorporate those changes into the latest patch.

[Ryan J Ollos <ryano@…>]

I've been looking closely at the syntax of the TicketQuery macro and am thinking about some ways that we can make the new arguments to this macro more like the format and group arguments of the TicketQuery macro, for consistency. I'll post some more comments about this shortly.

[Remy Blank]

Preparing for 1.0.

[Christian Boos]

Last patch looks good, I suppose it was not applied because in comment:8 you were talking about follow-up changes. But that was quite some time ago, so I suppose we can consider the patch as is for the next dev release.

[Ryan J Ollos <ryano@…>]

One of many things I've unfortunately lost track of.

If I remember correctly, I was thinking the notation would be more consistent with the TicketQuery macro if we added a group option, rather than adding a format option. The default for the group option would be date (or perhaps we want to use day, to allow for grouping by week, month, etc. in the future). We could add a none option for the grouping, and in the future also allow for grouping by author, alphabetically, etc.

I'm not sure which way is the better way to go. But if anyone has suggestions, I can quickly rework the patch.

[Christian Boos]

Yes, group=date|none (where date is interpreted as day, as a first step) sounds better and leaves the door opened for more grouping possibilities, as you said.

[Ryan J Ollos <ryano@…>]

New patch against r11098 of the trunk:

• option added is group=date|none.
• added unit tests.

[Ryan J Ollos <ryano@…>]

Patch described in comment:13.

[Ryan J Ollos <ryano@…>]

Patch described in comment:13. Previous patch had some garbage in it.

[Christian Boos]

Looks good, thanks!

[Christian Boos]

I've taken the occasion of reviewing and testing the patch to do a few more clean-ups in macros.py.

Detailed steps in repos:cboos.git:ticket10120/t10120-r11098-2.patch-review, committed as a whole in r11100 (+ there's a hidden bonus in that changeset ;-) ).

[Ryan J Ollos <ryano@…>]

Thanks! Looking forward to doing more work on this project :)

[Remy Blank]

The unit test in [11100] fails here, but strangely only if functional tests are run. If they aren't, it passes. It's obviously locale-related, but I don't understand what's happening.

======================================================================
FAIL: Test RecentChanges, group option
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/joe/src/trac/trunk/trac/wiki/tests/formatter.py", line 213, in test
    % (msg, self.file, self.line, self.title, formatter.flavor))
AssertionError: 
========== expected: ==========
<p>
</p><div><h3>07/13/12</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><h3>07/13/12</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li></ul></div><p>
</p>


========== actual: ==========
<p>
</p><div><h3>2012-07-13</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><h3>2012-07-13</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li><li><a href="/wiki/WikiMid">WikiMid</a>
</li><li><a href="/wiki/WikiStart">WikiStart</a>
</li></ul></div><p>
</p><div><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
</li></ul></div><p>
</p>


========== wiki: ==========
u'
[[RecentChanges()]]
[[RecentChanges(group=date)]]
[[RecentChanges(group=none)]]
[[RecentChanges(,2,group=none)]]
[[RecentChanges(Wiki,group=none)]]
[[RecentChanges(Wiki,1,group=none)]]
'
========== diff: ==========
--- expected 
+++ actual 
@@ -1,9 +1,9 @@
 <p>
-</p><div><h3>07/13/12</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
+</p><div><h3>2012-07-13</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
 </li><li><a href="/wiki/WikiMid">WikiMid</a>
 </li><li><a href="/wiki/WikiStart">WikiStart</a>
 </li></ul></div><p>
-</p><div><h3>07/13/12</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
+</p><div><h3>2012-07-13</h3><ul><li><a href="/wiki/WikiEnd">WikiEnd</a>
 </li><li><a href="/wiki/WikiMid">WikiMid</a>
 </li><li><a href="/wiki/WikiStart">WikiStart</a>
 </li></ul></div><p>


/home/joe/src/trac/trunk/trac/wiki/tests/macros.py:1: "RecentChanges, group option" (default flavor)

----------------------------------------------------------------------
Ran 1392 tests in 257.481s

FAILED (failures=1)

[Remy Blank]

Unit test breakage fixed in [11120], and some simplifications in [11123].

[Peter Suter]

I still get that unit test failure when the functional tests are run. (It also seems to have popped up in comment:57:ticket:10779.)

The problem can be described like this:

>>> import datetime
>>> import locale
>>> locale.getlocale()
(None, None)

>>> datetime.datetime.now().strftime("%x")
'09/14/12'

>>> locale.setlocale(locale.LC_ALL, "")
'German_Switzerland.1252'
>>> locale.getlocale()
('German_Switzerland', '1252')

>>> datetime.datetime.now().strftime("%x")
'14.09.2012'

The strftime("%x") calls are in trac.util.datefmt.format_time. (At least without Babel.) The setlocale(locale.LC_ALL, "") call is in trac/tests/functional/testenv.py.

I'm not sure what the best way to fix it is. Somehow remove the setlocale call somehow? Call setlocale earlier? Call format_date later?

I only tried a somewhat crude trick to call format_date later (and it seems to avoid the problem):

trac/wiki/tests/macros.py

@@ -361 +361 @@
 </li></ul></div><p>
 </p>
 ------------------------------
-""" % {'date': format_date(tzinfo=utc, locale=locale_en)}
+"""
 
 def recentchanges_setup(tc):
     def add_pages(tc, names):
@@ -375 +375 @@
         'WikiMid',
         'WikiEnd',
         ])
+    tc.correct = tc.correct % {'date': format_date(tzinfo=utc,
+                                                   locale=locale_en)}
 
 def recentchanges_teardown(tc):
     tc.env.reset_db()

[Remy Blank]

Replying to psuter:

I still get that unit test failure when the functional tests are run. (It also seems to have popped up in comment:57:ticket:10779.)

Yes, at the time I thought it was due to my strange locale on Windows (I use an en_US locale but change the date and time formats back to Swiss ones).

I only tried a somewhat crude trick to call format_date later (and it seems to avoid the problem):

That actually sounds like the proper fix :) Let me check… Yes, that fixes the issue for me, too. Please apply!

[Peter Suter]

OK, thanks for confirming! Applied in [11328].