Apache Allura™

• labels: support --> support, p3

Originally by: ligc

HI,

We are running into the same issue with the TOC supporting included pages, see https://sourceforge.net/p/forge/site-support/8032/.

This is very important for us, we are using included pages everywhere, without the TOC for these included pages, our documentation seems quite messed up.

Any idea on when this bug will fixed? Thank you.

• labels: support, p3 --> support, p3, 42cc
• status: open --> in-progress
• assigned_to: Igor Bondarenko

• Milestone: forge-backlog --> forge-sep-5

• Size: --> 2

• status: in-progress --> code-review

Phew! This was a tough one. Make sure you test it against different wiki pages with different markup, macros, etc, since it changes markdown pipeline a bit.

Closed #635. je/42cc_4771

• Milestone: forge-sep-5 --> forge-sep-19

• Milestone: forge-sep-19 --> forge-oct-3

• Milestone: forge-oct-3 --> forge-oct-17

• QA: Dave Brondsema

Can you comment (here or in the code) about what the change in ForgeMacroPattern really does? The inline comment seems to be about the strip() and join() but it's not obvious to me what calling etree.fromstring() does in this context.

The custom slugify function took me a bit to get (maybe just because its a friday afternoon) but looks like its the same as the original but with better unicode handling.

The change in ForgeMacroPattern is essentially about making included page's content available to TOC extension. We need to include processed page contents (html) into parent page, so that TOC will find it. Old code just put macro result onto html stash, which is processed after all the extensions (and there's no way around it), so it was hidden from TOC extension.

Returning html as a string does not work in this context. Actually, it includes page contents as needed, but TOC extension still can't find it (don't sure why). If we return html as a etree, then TOC can find it and do the job.

The except might happen if macro returned error, which is just text and can't be parsed to html, so it's ok to return that text, since there will not be any headers that we need to include in TOC anyway.

Regarding slugify you're completely right, it is only for better unicode handling. I was getting some "TypeError: must be unicode, not str" errors from within markdown, bacause etree that I return from ForgeMacroPattern is always returns a bytestring, when coerced into string, and slugify needed unicode.

Hope, this clarifies it.

• status: code-review --> in-progress

If the include macro is used to include a repo file, the line numbers get squashed together. HTML changed from

<div class="linenodiv"><pre>1
2</pre></div>

to:

<div class="linenodiv">
<pre>12</pre>
</div>

• Milestone: forge-oct-17 --> forge-oct-31

• Labels: support, p3, 42cc, sf-2 --> p3, support, 42cc, sf-current, sf-2

Closed #672. ib/4771

I have added comments and fixed the case when page includes file from a repo directly, but there's still a problem if you include a wiki page, which, in turn, includes a file from a repo. I managed to fix issues with the line numbers in that case, but not with file contents (code). It gets extra <br>'s which break layout. I've tried a lot of things to fix that, but nothing works.

Entire approach is very fragile... Any ideas on how to tackle it in more robust way and fix the issue with multi-level includes?

Latest commits cause text to be omitted sometimes. Specifically it seems like a child page with a header will only include the text from the first line after the header.

Test case improvement:

diff --git Allura/allura/tests/test_globals.py Allura/allura/tests/test_globals.py
index a68096d..5631026 100644
--- Allura/allura/tests/test_globals.py
+++ Allura/allura/tests/test_globals.py
@@ -256,34 +256,32 @@ def test_macro_project_admins_one_br():
 @td.with_wiki
 def test_macro_include_no_extra_br():
     p_nbhd = M.Neighborhood.query.get(name='Projects')
     p_test = M.Project.query.get(shortname='test', neighborhood_id=p_nbhd._id)
     wiki = p_test.app_instance('wiki')
     with h.push_context(p_test._id, app_config_id=wiki.config._id):
         p = WM.Page.upsert(title='Include_1')
-        p.text = 'included page 1'
+        p.text = '# page 1 title\nincluded page 1 line 1\nincluded page 1 line 2\nincluded page 1 line 3'
         p.commit()
         p = WM.Page.upsert(title='Include_2')
         p.text = 'included page 2'
         p.commit()
         p = WM.Page.upsert(title='Include_3')
         p.text = 'included page 3'
         p.commit()
         ThreadLocalORMSession.flush_all()
         md = '[[include ref=Include_1]]\n[[include ref=Include_2]]\n[[include ref=Include_3]]'
         html = g.markdown_wiki.convert(md)

     expected_html = '''
 <div class="markdown_content">
-<p>
-<div><div class="markdown_content"><p>included page 1</p></div></div>
+<div><div class="markdown_content"><h1 id="page-1-title">page 1 title</h1>
+<p>included page 1 line 1<br />included page 1 line 2<br />included page 1 line 3</p></div></div>
 <div><div class="markdown_content"><p>included page 2</p></div></div>
 <div><div class="markdown_content"><p>included page 3</p></div></div>
-</p>
-<p></p>
 </div>
 '''.strip().replace('\n', '')
     assert html.strip().replace('\n', '') == expected_html, html

 @with_setup(setUp, tearDown)
 @td.with_wiki
 @td.with_tool('test', 'Wiki', 'wiki2')

I agree this approach is fragile, and getting quite messy. I don't really have a complete picture of how all the pieces fit together, so I can't really help with a specific recommendation. What it looks like to me though is the challenge is getting the includes to run before the TOC. And you've worked on moving includes earlier in the process but that is causing problems with how they get rendered. What about moving the TOC to later in the process, and leave the includes how they originally were? Would that be any better?