lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Introduce markup-list-command table (issue 289980043 by address@hidd


From: thomasmorley65
Subject: Re: Introduce markup-list-command table (issue 289980043 by address@hidden)
Date: Sun, 31 Jan 2016 11:18:39 +0000


https://codereview.appspot.com/289980043/diff/1/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):

https://codereview.appspot.com/289980043/diff/1/scm/define-markup-commands.scm#newcode4724
scm/define-markup-commands.scm:4724: The amount of columns is specifies
by @var{columns}.  Each column may be aligned
On 2016/01/30 23:02:10, Carl wrote:
Should read "The number of columns is specified"

Done.

https://codereview.appspot.com/289980043/diff/1/scm/define-markup-commands.scm#newcode4767
scm/define-markup-commands.scm:4767: ;; -> (list 0 1 3 6 10)
On 2016/01/30 23:32:17, dak wrote:
It's not clear why rl is even a list if it always has to have exactly
one
member.  And the (reverse rl) indicates that this function does
something else.

Also the comment does not mention the function of "padding".  So the
function
description tries to be sort-of abstract by avoiding to mention what
the
function is actually being used for, but there are lots of missing
pieces which
can only be guessed if you actually know what is being done here.

Bad combination.  Try being more complete.  I suspect that this will
yield to a
call of fold, but it's hard to say with the documentation not really
matching
the code.

Changed it using fold.
Better description (hopefully)

https://codereview.appspot.com/289980043/diff/1/scm/define-markup-commands.scm#newcode4783
scm/define-markup-commands.scm:4783: (split-lst lst columns '())))
On 2016/01/30 23:32:17, dak wrote:
We've had this a few times.  A markup-list? argument is not
necessarily a list
of markups. It can also be a markup list command call.

Aargh, I keep forgetting about it.

The only thing you can
reliably do before starting list processing is to call
interpret-markup-list on
the _whole_ markup-list argument.  Only _then_ can you start splitting
the
resulting list of stencils.

Done.

https://codereview.appspot.com/289980043/diff/1/scm/define-markup-commands.scm#newcode4814
scm/define-markup-commands.scm:4814: ;;              aligned, (-1, 0, 1)
means (LEFT, RIGHT, CENTER)
On 2016/01/30 23:02:10, Carl wrote:
Surely you mean (LEFT, CENTER, RIGHT), don't you?

Done.

https://codereview.appspot.com/289980043/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]