Re: add stencil-whiteout-outline function (issue 236480043 by address@hidden)

[paulwmorris]

Just uploaded patch set 4.

It includes improved convert-ly rules.  I ran
scripts/auxiliar/update-with-convert-ly.sh
with these rules on both a clean branch and on the branch where I'm
working on this functionality.  It worked as expected for both.

This patch set also includes a changes entry and addresses Keith's
comments from the last set.

I had to update one snippet manually ("Using the whiteout property") to
update the doc string to "whiteout-box". I then copied it to the
snippets/new directory and ran the snippet update script, all as
described in the CG.  It seems like that went as it should.

Thanks for reviewing,
-Paul


https://codereview.appspot.com/236480043/diff/40001/python/convertrules.py
File python/convertrules.py (right):

https://codereview.appspot.com/236480043/diff/40001/python/convertrules.py#newcode3751
python/convertrules.py:3751: str = re.sub (r'\bwhiteout\b',
'whiteout-box', str)
On 2015/06/02 05:00:30, Keith wrote:

> Separate rules or a pattern that catches the prefixes-- whatever works
to update
> the LilyPond input without changing comments about the whiteout.

> And be a bit defensive against cases that you can imagine.  You do not
want to
> convert whiteout-box -> whiteout-box-box, and to be thorough you might
want to
> catch uses with no spaces like \markup\whiteout1 , although with the
new
> \whiteout being an improvement over the old you can err on the side of
being
> conservative.

> Scheme functions that do not appear in the docs, such as
'stencil-whiteout',
> probably do not need convert-ly rules, but if the conversion is easy
to do
> without false-positives it should be fine.

Ok, thanks, fixed in next patch set (4th).

https://codereview.appspot.com/236480043/diff/40001/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):

https://codereview.appspot.com/236480043/diff/40001/scm/define-grob-properties.scm#newcode1147
scm/define-grob-properties.scm:1147: equivalent to a value of 3.
Usually #f by default.")
On 2015/06/02 05:00:30, Keith wrote:
> The 'boolean-or-number' parameter made sense for getting two types of
behavior
> through one parameter, but it is unusual to have #t be a synonym for
3.  '3' is
> easier to type than '##t'.

> If this is a backward-compatibility help for people whose fingers know
\override
> Xx #'whiteout = ##t , it would be less mysterious to say "For
compatibility with
> former behavior (now available with @code{whiteout-box}) the value #t
is treated
> as 3.0."

Ok, I've fixed the wording in the next patch set (4th).  I was thinking
about backwards compatibility like you describe and also that it's
easier on users if they can get a sensible default with ##t without
having to remember a number and what the units are.  Also for parity
with whiteout-box which accepts ##t for its default (and may take a
number in the future to customize its outline thickness).

> Isn't it just "multiple of the staff-line thickness"? That is, if you
> #(set-global-staff-size 5) doesn't the white-out stay relatively
thicker, to
> match the relatively thicker staff-lines?

Yep, done.

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

https://codereview.appspot.com/236480043/diff/40001/scm/define-markup-commands.scm#newcode710
scm/define-markup-commands.scm:710: #:properties ((thickness 4))
On 2015/06/02 05:00:30, Keith wrote:
> I just guessed the number 3 staffline thicknesses, and apparently
typo'd it as
> 4.  If you like that default, fine.

Ok, I tried both and went with 3 in the next patch set (4th).

https://codereview.appspot.com/236480043/diff/40001/scm/define-markup-commands.scm#newcode720
scm/define-markup-commands.scm:720: \\whiteout whiteout
On 2015/06/02 05:00:30, Keith wrote:
>    \\override #'(thickness . 1.5) \\whiteout whiteout
> otherwise there is no indication of what thickness does,
> and the default thickness around 'whiteout' looks jagged on the black
> background.

Done.

https://codereview.appspot.com/236480043/