Re: Patch for issue #1116 (one stencil in fill-line) (issue1689041)

[n . puttock]

Hi Alexander,

LGTM.

It just needs some regression tests; the examples from #1116 and #382
should suffice.

Cheers,
Neil


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

http://codereview.appspot.com/1689041/diff/2001/3001#newcode845
scm/define-markup-commands.scm:845: (set! line-stencils empty-stencil)
Hmm, line-stencils isn't appropriate from this point, since it's no
longer a list of stencils; perhaps it would be clearer to bind to
another variable?

http://codereview.appspot.com/1689041/diff/2001/3001#newcode847
scm/define-markup-commands.scm:847: (stack-stencils-padding-list
indent

http://codereview.appspot.com/1689041/diff/2001/3001#newcode848
scm/define-markup-commands.scm:848: X RIGHT fill-space line-stencils)))
On 2010/06/30 18:34:35, joeneeman wrote:
> These two set!s could be written more concisely as
> (set! line-stencils
>    (stack-stencils-padding-list
>     X RIGHT fill-space empty-stenc

No, they're part of the `if' form above.

http://codereview.appspot.com/1689041/show