tests/expected_output/en/sitemap - Issue 29741581: Fixes 6546 - Swap the contents of default.tmpl and empty.tmpl in the test suite

Side by Side Diff: tests/expected_output/en/sitemap

Issue 29741581: Fixes 6546 - Swap the contents of default.tmpl and empty.tmpl in the test suite (Closed)

Patch Set: Remove unnecessary template=default metadata and adjust the expected output for sitemap accordingly Created April 3, 2018, 4:34 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View unified diff | Download patch

OLD	NEW
1	1

2	2

3	3

4	4

5 <h1> Unfiltered </h1>	5 <h1> Unfiltered </h1>

6 <ul>	6 <ul>

7 <li>title : filter </li>

8 <li>title : get_page_url </li>

9 <li>title : global </li>

10 <li>title : includer </li>	7 <li>title : includer </li>

11 <li>title : translate </li>

12 <li>title : translate-html </li>

13 <li>title : translate-include </li>

14 <li>title : translate-not-enough </li>

15 <li>title : translate-partial </li>

16 <li>title : translate-template </li>	8 <li>title : translate-template </li>

17 <li>title : translate-tmpl </li>
Sebastian Noack 2018/04/03 22:32:51 So these pages showed up here before because they So these pages showed up here before because they defined any metadata (i.e. the template), but since they didn't define any other metadata they are no longer considered by get_pages_metadata(). I still fail to see why these semantics make any sense. But not my business, and somewhat unrelated of this change anyway. Vasily Kuznetsov 2018/04/03 22:47:04 I pretty much see it the same way as you do, but t Show quoted text On 2018/04/03 22:32:51, Sebastian Noack wrote: > So these pages showed up here before because they defined any metadata (i.e. the > template), but since they didn't define any other metadata they are no longer > considered by get_pages_metadata(). I still fail to see why these semantics make > any sense. But not my business, and somewhat unrelated of this change anyway. I pretty much see it the same way as you do, but there's another possible point of view: `get_pages_metadata` returns user-defined metadata dicts from the pages that have user-defined metadata. Arguably it's less consistent, but in practice (and for real use cases of this function) it makes no difference because all the pages that people would ever want to get out if it would have user-defined metadata. Julian (who actually uses CMS and this function) prefers second interpretation and I think this is not important enough to have debates about, so I'll just let it be. rosie 2018/04/04 16:22:17 Should there be a comment somewhere in this file t Show quoted text On 2018/04/03 22:47:04, Vasily Kuznetsov wrote: > On 2018/04/03 22:32:51, Sebastian Noack wrote: > > So these pages showed up here before because they defined any metadata (i.e. > the > > template), but since they didn't define any other metadata they are no longer > > considered by get_pages_metadata(). I still fail to see why these semantics > make > > any sense. But not my business, and somewhat unrelated of this change anyway. > > I pretty much see it the same way as you do, but there's another possible point > of view: `get_pages_metadata` returns user-defined metadata dicts from the pages > that have user-defined metadata. Arguably it's less consistent, but in practice > (and for real use cases of this function) it makes no difference because all the > pages that people would ever want to get out if it would have user-defined > metadata. Julian (who actually uses CMS and this function) prefers second > interpretation and I think this is not important enough to have debates about, > so I'll just let it be. Should there be a comment somewhere in this file that makes the expected behavior clearer? Sebastian Noack 2018/04/04 17:42:57 As I pointed out in the other review, there is a c Show quoted text On 2018/04/04 16:29:37, Vasily Kuznetsov wrote: > It would be good to document this behavior (probably the docstring of > `get_pages_metadata` is the best place), but this is not related to this change, > so it probably should be a separate noissue review. As I pointed out in the other review, there is a comment in the source code: https://hg.adblockplus.org/cms/file/b683e321a9d0/cms/converters.py#l477 But its merely indicating that pages without custom metadata are skipped, but fails to explain why. Furthermore, it seems there is out-of-source documentation for provided template functions in docs/api/functions.md, which however is missing any mention of get_pages_metadata() at all.
18 <li>title : sitemap </li>	9 <li>title : sitemap </li>

19 </ul>	10 </ul>

20	11

21 <h1> Filtered </h1>	12 <h1> Filtered </h1>

22 <ul><li>title : sitemap </li>	13 <ul><li>title : sitemap </li>

23 </ul>	14 </ul>

24	15

25 <h1> String Filter For Listy Option </h1>	16 <h1> String Filter For Listy Option </h1>

26 <ul><li>title : sitemap </li>	17 <ul><li>title : sitemap </li>

27 </ul>	18 </ul>

OLD	NEW

« no previous file with comments | « no previous file | tests/test_additional_paths.py » ('j') | no next file with comments »