From: Karl C. <quarl@NOSPAM.quarl.org> - 2006-09-22 02:02:46
|
I depend on using links within replacement text in my projects. The ReST at the bottom of this email worked in Docutils 0.3.7 but produces incorrect (IMO) output in Docutils 0.4. Docutils 0.3 gives me:: <p><a class="reference" href="http://example.org/">y</a></p> Docutils 0.4 gives me:: <p><a class="reference" href="#y">y</a></p> The "y" anchor is never defined, so I believe this behavior is unintentional. |x| .. |x| replace:: y_ .. _y: http://example.org/ -- Karl 2006-09-21 18:58 |
From: Felix W. <Fel...@os...> - 2006-09-22 03:00:33
|
Karl Chen wrote: > <p><a class="reference" href="#y">y</a></p> Yup, this is a known bug that we haven't gotten around to fixing yet -- sorry. See <http://docutils.sf.net/BUGS.html#hyperlink-references-in-substitutions> for a workaround, if that's OK for you. Best, Felix -- Felix Wiemann -- http://www.ososo.de/ |
From: Karl C. <qu...@cs...> - 2006-09-22 10:23:12
|
>>>>> On 2006-09-21 20:00 PDT, Felix Wiemann writes: Felix> Karl Chen wrote: >> <p><a class="reference" href="#y">y</a></p> Felix> Yup, this is a known bug that we haven't gotten around Felix> to fixing yet -- sorry. See Felix> <http://docutils.sf.net/BUGS.html#hyperlink-references-in-substitutions> Felix> for a workaround, if that's OK for you. Hi Felix, thanks for the pointer to the workaround. I believe the bug is due to this line in Substitutions.apply: ref.replace_self(subdef_copy.children) subdef_copy is a deep copy of subdef. At this point document.refnames only points to the substitution source text, not the nodes in the deep copies. So ExternalTargets.apply never sees them. I tried using replace_self(subdef.children) and that fixes this bug, but fails a bunch of test cases. I'm sure it was added for a reason. The deepcopy was introduced in this change: r3935 | fwiemann | 2005-10-11 12:34:52 -0700 (Tue, 11 Oct 2005) | 1 line added Node.deepcopy(); fixed bug with doubly-indirect substitutions The patch below against HEAD passes all test cases and works for me. However, it is kludgy and there are probably other things that either shouldn't be deep-copied or should have any links pointing to them updated. Index: references.py =================================================================== --- references.py (revision 4755) +++ references.py (working copy) @@ -730,6 +730,9 @@ ref.replace_self(prb) else: ref.replace_self(subdef_copy.children) + for node in subdef_copy.children: + if isinstance(node, nodes.reference) and node.hasattr('refname'): + self.document.note_refname(node) class TargetNotes(Transform): -- Karl 2006-09-22 03:13 |
From: Felix W. <Fel...@os...> - 2006-09-22 19:50:46
|
Karl Chen wrote: > I believe the bug is due to this line in Substitutions.apply: The substitution and reference transforms have like always been kindof a mess. Recent changes have broken in subtle ways (even though I think there were bugs before), but basically the transforms need to be cleaned up instead of fixed in particular places. > ref.replace_self(subdef_copy.children) > > subdef_copy is a deep copy of subdef. At this point > document.refnames only points to the substitution source text, not > the nodes in the deep copies. Yup, seems right. (ISTM that document.refnames is a source of evil and we want to get rid of it.) > So ExternalTargets.apply never sees > them. I tried using replace_self(subdef.children) and that fixes > this bug, but fails a bunch of test cases. You cannot do this because you would insert the same node in two different places in the tree, which probably causes extremely weird effects. So you actually do need to make a deep copy. > The patch below against HEAD passes all test cases and works for > me. However, it is kludgy and there are probably other things > that either shouldn't be deep-copied or should have any links > pointing to them updated. > > > > Index: references.py > =================================================================== > --- references.py (revision 4755) > +++ references.py (working copy) > @@ -730,6 +730,9 @@ > ref.replace_self(prb) > else: > ref.replace_self(subdef_copy.children) > + for node in subdef_copy.children: > + if isinstance(node, nodes.reference) and node.hasattr('refname'): > + self.document.note_refname(node) > > > class TargetNotes(Transform): Yes, it is kinda kludgy, but it looks like an OK fix for this particular problem. I'd like to have another pair of eyes looking at it, though, so David, do you think that we should apply this patch (and add a test case)? Best wishes, and thanks for the patch! Felix -- Felix Wiemann -- http://www.ososo.de/ |