Edgewall Software
Modify

Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#10120 closed enhancement (fixed)

Add a `format` parameter to the RecentChanges macro

Reported by: Ryan J Ollos <ryano@…> Owned by: Ryan J Ollos <ryano@…>
Priority: normal Milestone: 1.0
Component: wiki system Version: 0.13dev
Severity: normal Keywords: bitesized, macro
Cc: Branch:
Release Notes:

RecentChanges can produce a simple list with group=none (default is group=date, pre-1.0 behavior of grouping by day)

API Changes:
Internal Changes:

Description

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.

Attachments (5)

macros.r10668.patch (2.4 KB ) - added by ryano@… 9 years ago.
First cut at a patch.
macros.r10671.patch (3.6 KB ) - added by Ryan J Ollos <ryano@…> 9 years ago.
macros.r10672.patch (3.4 KB ) - added by Ryan J Ollos <ryano@…> 9 years ago.
t10120-r11098-1.patch (8.1 KB ) - added by Ryan J Ollos <ryano@…> 8 years ago.
Patch described in comment:13.
t10120-r11098-2.patch (5.9 KB ) - added by Ryan J Ollos <ryano@…> 8 years ago.
Patch described in comment:13. Previous patch had some garbage in it.

Download all attachments as: .zip

Change History (28)

by ryano@…, 9 years ago

Attachment: macros.r10668.patch added

First cut at a patch.

comment:1 by anonymous, 9 years ago

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)]]

comment:2 by anonymous, 9 years ago

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.

comment:3 by Christian Boos, 9 years ago

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

comment:4 by Remy Blank, 9 years ago

Milestone: 0.13

in reply to:  3 ; comment:5 by anonymous, 9 years ago

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.

by Ryan J Ollos <ryano@…>, 9 years ago

Attachment: macros.r10671.patch added

in reply to:  5 ; comment:6 by Christian Boos, 9 years ago

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>.

by Ryan J Ollos <ryano@…>, 9 years ago

Attachment: macros.r10672.patch added

in reply to:  6 comment:7 by Ryan J Ollos <ryano@…>, 9 years ago

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.

comment:8 by Ryan J Ollos <ryano@…>, 9 years ago

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.

comment:9 by Remy Blank, 8 years ago

Milestone: 1.01.0-triage

Preparing for 1.0.

comment:10 by Christian Boos, 8 years ago

Milestone: next-stable-1.0.xnext-dev-1.1.x

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.

comment:11 by Ryan J Ollos <ryano@…>, 8 years ago

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.

comment:12 by Christian Boos, 8 years ago

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.

comment:13 by Ryan J Ollos <ryano@…>, 8 years ago

New patch against r11098 of the trunk:

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

by Ryan J Ollos <ryano@…>, 8 years ago

Attachment: t10120-r11098-1.patch added

Patch described in comment:13.

by Ryan J Ollos <ryano@…>, 8 years ago

Attachment: t10120-r11098-2.patch added

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

comment:14 by Christian Boos, 8 years ago

Milestone: next-dev-1.1.x1.0
Owner: set to Christian Boos

Looks good, thanks!

comment:15 by Christian Boos, 8 years ago

Resolution: fixed
Status: newclosed

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 ;-) ).

comment:16 by Christian Boos, 8 years ago

Owner: changed from Christian Boos to Ryan J Ollos <ryano@…>

comment:17 by Christian Boos, 8 years ago

Release Notes: modified (diff)

comment:18 by Ryan J Ollos <ryano@…>, 8 years ago

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

comment:19 by Remy Blank, 8 years ago

Resolution: fixed
Status: closedreopened

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)

comment:20 by Remy Blank, 8 years ago

Resolution: fixed
Status: reopenedclosed

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

comment:21 by Peter Suter, 8 years ago

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

    diff -r 5f17cf0919aa trac/wiki/tests/macros.py
    a b  
    361361</li></ul></div><p>
    362362</p>
    363363------------------------------
    364 """ % {'date': format_date(tzinfo=utc, locale=locale_en)}
     364"""
    365365
    366366def recentchanges_setup(tc):
    367367    def add_pages(tc, names):
     
    375375        'WikiMid',
    376376        'WikiEnd',
    377377        ])
     378    tc.correct = tc.correct % {'date': format_date(tzinfo=utc,
     379                                                   locale=locale_en)}
    378380
    379381def recentchanges_teardown(tc):
    380382    tc.env.reset_db()

in reply to:  21 comment:22 by Remy Blank, 8 years ago

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!

comment:23 by Peter Suter, 8 years ago

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos <ryano@…>.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos <ryano@…> to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.