Issue 4504: Revert interface changes for whiteout (issue 4418)

Issue 4504/5 update changes.tely


Issue 4504/4 edits to tests and docs


Issue 4504/3 run scripts/auxiliar/update-with-convert-ly.sh


Issue 4504/2 convert-ly rule: whiteout-box --> whiteout


Issue 4504/1 add whiteout-style, drop whiteout-box

Introduce whiteout-style grob property, with
options of 'box and 'outline, to be used with
the whiteout grob property, and an equivalent
style property for the whiteout markup command.

Remove the whiteout-box grob property and
markup command, and use the new properties
instead.

Make the box style of whiteout the default
style again, as it was before issue 4418.

[pwm, 2015-11-03 04:23:06]

Please review. This patch set implements the "whiteout and whiteout-style" grob
property approach.  

Another approach ("whiteout and whiteout-thickness") has also been proposed. See
issue tracker:

https://sourceforge.net/p/testlilyissues/issues/4504/

Thanks,
-Paul

[pkx166h, 2015-11-03 11:09:00]

This fails make/make doc

(see comments inline)

https://codereview.appspot.com/275770043/diff/1/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/275770043/diff/1/Documentation/changes.tely#ne...
Documentation/changes.tely:72: \\markup {
You don't need to escape '\'. @lilypond[] (this will now literally print two '\'
characters).

see:
http://lilypond.org/doc/v2.19/Documentation/contributor-big-page#lilypond-for...

You'll notice the examples there do not have double slashes

but also see:

http://lilypond.org/doc/v2.19/Documentation/contributor-big-page.html#special...

Which is when you 'do' need to escape certain characters in TexInfo (but the
escape character is not a '\')

[Trevor Daniels, 2015-11-03 11:52:28]

I raise a couple of questions - maybe I'm not understanding something.

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

https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.sc...
scm/define-grob-properties.scm:1143: (whiteout ,boolean-or-symbol? "If a number
or true, the grob is
Shouldn't this be boolean-or-number>

https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.sc...
scm/define-grob-properties.scm:1149: (whiteout-style ,number? "Determines the
shape of the
Shouldn't this be ,symbol?

https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.sc...
scm/define-grob-properties.scm:1150: @code{whiteout} background.  Available are
@code{outline} and the
'outline

https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.sc...
scm/define-grob-properties.scm:1151: default @code{box}.")
'box

[pwm, 2015-11-03 15:28:57]

fix problems in patch set 1

[pkx166h, 2015-11-03 15:33:19]

https://codereview.appspot.com/275770043/diff/20001/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/275770043/diff/20001/Documentation/changes.tel...
Documentation/changes.tely:90: @end lilypond"
Paul, sorry - I did comment on this previously but it didn't seem to get saved -
there are unnecessary double quote marks here ('@end lilypond"') that need
removing to allow make doc to succeed.

[pwm, 2015-11-03 15:46:09]

Thanks for the feedback.  All fixed in patch set 2.
-Paul

https://codereview.appspot.com/275770043/diff/1/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/275770043/diff/1/Documentation/changes.tely#ne...
Documentation/changes.tely:72: \\markup {
Ah, thanks, and thanks for those links.  I got tripped up by copying the example
from define-markup-commands.scm where the double '\' is needed.  Fixed in patch
set 2.

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

https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.sc...
scm/define-grob-properties.scm:1143: (whiteout ,boolean-or-symbol? "If a number
or true, the grob is
On 2015/11/03 11:52:28, Trevor Daniels wrote:
> Shouldn't this be boolean-or-number>

Yes, good catch, thanks.  Fixed in patch set 2.  (I had started to change to the
"whiteout and whiteout-thickness" approach, but then changed it back, and in the
process forgot to change these predicates back as well.)

https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.sc...
scm/define-grob-properties.scm:1149: (whiteout-style ,number? "Determines the
shape of the
Yes, good catch, thanks. Fixed in patch set 2.

https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.sc...
scm/define-grob-properties.scm:1150: @code{whiteout} background.  Available are
@code{outline} and the
Fixed in patch set 2.

https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.sc...
scm/define-grob-properties.scm:1151: default @code{box}.")
Fixed in patch set 2.

[pwm, 2015-11-03 15:50:00]

Fixing double quote marks after '@end lilypond'

[pwm, 2015-11-03 15:52:53]

Patch set 3 to removes the double quotes after '@end lilypond' in changes.tely. 
Should be all good to go now.
-Paul

https://codereview.appspot.com/275770043/diff/20001/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/275770043/diff/20001/Documentation/changes.tel...
Documentation/changes.tely:90: @end lilypond"
On 2015/11/03 15:33:19, pkx166h wrote:
> Paul, sorry - I did comment on this previously but it didn't seem to get saved
-
> there are unnecessary double quote marks here ('@end lilypond"') that need
> removing to allow make doc to succeed.

Done, thanks for catching this. See patch set 3.

[pkx166h, 2015-11-03 21:45:03]

doesn't compile doc

https://codereview.appspot.com/275770043/diff/40001/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/275770043/diff/40001/Documentation/changes.tel...
Documentation/changes.tely:91: 
This snippet doesn't compile. I cannot work out why.

Remember that this is all built using lilypond-book during doc compilation, so
you can test any snippet you add by using a basic texinfo file layout and adding
the TexInfo code:

--snip--

\input texinfo @node Top
@top

@lilypond[verbatim,quote]
\markup {
  \combine
    \filled-box #'(-1 . 15) #'(-3 . 4) #1
    \override #'(thickness . 2)
    \whiteout whiteout-box
}
\markup {
  \combine
    \filled-box #'(-1 . 18) #'(-3 . 4) #1
    \override #'(whiteout-style . outline)
    \override #'(thickness . 3)
    \whiteout whiteout-outline
}
\relative {
  \override Staff.Clef.whiteout-style = #'outline
  \override Staff.Clef.whiteout = 3
  g1'
}
@end lilypond


@bye

--snip--

So put this in a file (called foo.tely) and run

lilypond-book --pdf foo.tely

In this case it fails to compile:

Writing snippets...
Processing...
Running lilypond...
GNU LilyPond 2.19.31
Processing `./snippet-map-3978275931912102135.ly'
Parsing...
Processing `./39/lily-d81e2151.ly'
Parsing...
././39/lily-d81e2151.ly:4:1: error: markup outside of text script or \lyricmode

\markup {
././39/lily-d81e2151.ly:10:1: error: markup outside of text script or \lyricmode

\markup {
././39/lily-d81e2151.ly:20:5: error: syntax error, unexpected '\''
  g1
    '
././39/lily-d81e2151.ly:-4:1: error: errors found, ignoring music expression

\relative c'
fatal error: failed files: "39/lily-d81e2151.ly"
command failed: /home/james/lilypond-git/build/out/bin/lilypond --formats=ps,png
-dbackend=eps  -I  "./" --formats=eps  -deps-box-padding=3.000000 
-dread-file-list -dno-strip-output-dir 
"/home/james/test/snippet-names-3978275931912102135.ly"
Child returned 1


It's to do with how @lilypond[xxx] puts some \paper and other boilerplate
variables around the actual constructed example. But I cannot work out how to
fix it, I think some of the other devs in the list will.

[dak, 2015-11-03 22:21:55]

https://codereview.appspot.com/275770043/diff/40001/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/275770043/diff/40001/Documentation/changes.tel...
Documentation/changes.tely:88: g1'
This needs to be g'1 instead of g1' .  The rest looks to me like it should
compile, though.  No idea why James' isolated attempt flagged more problems.

[pwm, 2015-11-03 23:59:47]

Thanks for your help.  I made a test file and got it to compile without errors.

https://codereview.appspot.com/275770043/diff/40001/Documentation/changes.tely
File Documentation/changes.tely (right):

https://codereview.appspot.com/275770043/diff/40001/Documentation/changes.tel...
Documentation/changes.tely:88: g1'
On 2015/11/03 22:21:55, dak wrote:
> This needs to be g'1 instead of g1' .  

Yikes. How did I mess that up...

https://codereview.appspot.com/275770043/diff/40001/Documentation/changes.tel...
Documentation/changes.tely:91: 
On 2015/11/03 21:45:03, pkx166h wrote:
> This snippet doesn't compile. I cannot work out why.
> 
> Remember that this is all built using lilypond-book during doc compilation, so
> you can test any snippet you add by using a basic texinfo file layout and
adding
> the TexInfo code:

Thank you for showing me that; it's very helpful.  I made a test file, corrected
the g1' -> g'1, and fixed another problem.  This:

  \override #'(whiteout-style . outline)

should actually be this:

  \override #'(style . outline)

(Originally I was using 'whiteout-style' for the markup command property, but
changed it to just 'style' because other markup commands use 'style', but forgot
to update it here. (groan...))

The good news is that with these fixes, and running it with the compiled
version, everything went fine.  Log output below.  So I will submit another
patch set with these changes.


[build (whiteout-pad)]$ out/bin/lilypond-book --pdf mytest/test.tely
lilypond-book (GNU LilyPond) 2.19.31
Reading mytest/test.tely...
Running texi2pdf on file /tmp/tmpuL_Y2D.texi to detect default page settings.

Dissecting...
Writing snippets...
Processing...
Running lilypond...
GNU LilyPond 2.19.31
Processing `./snippet-map-513811304.ly'
Parsing...
Processing `./63/lily-ffa13262.ly'
Parsing...
Interpreting music...
Preprocessing graphical objects...
Calculating line breaks... 
Drawing systems... 
Layout output to `./63/lily-ffa13262.eps'...
Converting to `./63/lily-ffa13262.pdf'...
Converting to PNG...
Layout output to `./63/lily-ffa13262-1.eps'...
Layout output to `./63/lily-ffa13262-2.eps'...
Layout output to `./63/lily-ffa13262-3.eps'...
Converting to `./63/lily-ffa13262-1.pdf'...
Converting to `./63/lily-ffa13262-2.pdf'...
Converting to `./63/lily-ffa13262-3.pdf'...
Writing ./63/lily-ffa13262-systems.texi...
Writing ./63/lily-ffa13262-systems.tex...
Writing ./63/lily-ffa13262-systems.count...
Success: compilation successfully completed
Linking files...
Compiling /home/paul/lilypond-git/build/test.texi...
Writing `/home/paul/lilypond-git/build/test.texi'...
[build (whiteout-pad)]$

[pwm, 2015-11-04 00:05:06]

fix changes.tely lilypond example