Fix 380: Try to auto-detect cyclic references in header fields

Fix 380: Try to auto-detect cyclic references in header fields

Inside \fromproperty, simply change the props so that all references to the
currently interpreted property point to a function that prints out
a warning and returns a null markup, while interpreting the contents
of the property.

[pkx166h, 2011-09-10 10:04:22]

Passes make but fails reg test check:

see http://code.google.com/p/lilypond/issues/detail?id=380#c15

for details (keeps the noise down here)

James

[pkx166h, 2011-09-10 10:39:44]

patch 2 passes make and reg tests

[hanwenn, 2011-09-13 18:53:55]

have you thought of fixing this generically instead?

You could the hare/tortoise algorithm to detect cycles in any markup, and could
run that on the entry point (not the recursive function) for evaluating markups
to stencils.

[joeneeman, 2011-09-13 19:46:52]

http://codereview.appspot.com/4951073/diff/3001/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):

http://codereview.appspot.com/4951073/diff/3001/scm/define-markup-commands.sc...
scm/define-markup-commands.scm:1897: (interpret-markup layout (cons  (list (list
symbol `(,property-recursive-markup symbol))) props) m)
props is an alist, right? In that case, I think you want
(interpret-markup layout (cons (cons symbol `(,...)) props) m)

[Reinhold, 2011-09-14 22:20:42]

On 2011/09/13 19:46:52, joeneeman wrote:
> http://codereview.appspot.com/4951073/diff/3001/scm/define-markup-commands.scm
> File scm/define-markup-commands.scm (right):
> 
>
http://codereview.appspot.com/4951073/diff/3001/scm/define-markup-commands.sc...
> scm/define-markup-commands.scm:1897: (interpret-markup layout (cons  (list
(list
> symbol `(,property-recursive-markup symbol))) props) m)
> props is an alist, right?

No, it's a list of alists (for chain-assoc-get), so:

> In that case, I think you want
> (interpret-markup layout (cons (cons symbol `(,...)) props) m)

The correct code is (notice also the , before "symbol"):

(interpret-markup layout (cons (list (cons symbol `(,property-recursive-markup
,symbol))) props) m)

The latest patch fixes this, and now I correctly get the warning messages:

Vorverarbeitung der grafischen Elemente...
Warnung: Recursive definition of property header:title detected!
Ideale Seitenanzahl wird gefunden...

[Reinhold, 2011-09-14 23:28:09]

On 2011/09/13 18:53:55, hanwenn wrote:
> have you thought of fixing this generically instead?
> 
> You could the hare/tortoise algorithm to detect cycles in any markup, and
could
> run that on the entry point (not the recursive function) for evaluating
markups
> to stencils.

Actually, I fail to see how I can use the algorithm to detect cycles in markups.
First, a markup is a tree and a recursive function rather tan a chained function
application, so the algorithm would have to run on each branch. And second, the
markup => stencil conversion is done in Text_interface::interpret_markup by
simply calling the first markup function, which then recursively evaluates all
its subexpressions.
We never actually get the intermediate values to run the algorithm. We can only
get the intermediate values by evaluating the markup, but then we are already
inside the loop.

[Reinhold, 2011-09-15 13:00:46]

On 2011/09/14 23:28:09, Reinhold wrote:
> On 2011/09/13 18:53:55, hanwenn wrote:
> > have you thought of fixing this generically instead?
> > 
> > You could the hare/tortoise algorithm to detect cycles in any markup, and
> could
> > run that on the entry point (not the recursive function) for evaluating
> markups
> > to stencils.
> 
> Actually, I fail to see how I can use the algorithm to detect cycles in
markups.
> First, a markup is a tree and a recursive function rather tan a chained
function
> application, so the algorithm would have to run on each branch. And second,
the
> markup => stencil conversion is done in Text_interface::interpret_markup by
> simply calling the first markup function, which then recursively evaluates all
> its subexpressions.
> We never actually get the intermediate values to run the algorithm. We can
only
> get the intermediate values by evaluating the markup, but then we are already
> inside the loop.

I updated this patch to correctly print out the warning, and also implemented
the hare+tortoise algorithm in patch http://codereview.appspot.com/5027042/

I'd like to apply both, because the general cycle detection is not able to print
useful error messages, while this patch does, although it works only for cyclic
\fromproperty.