Add a \voicify command

Add a \voicify command


Also contains commits:

Document \voicify


Add regtest for \voicify


Let voicify-music receive an optional id list

[dak, 2017-04-03 08:46:14]

Actual patch after establish issue 5113 baseline in patch set 1

[lemzwerg, 2017-04-03 09:18:47]

Very nice!

[pkx166h, 2017-04-03 17:39:55]

Fails make doc.

https://codereview.appspot.com/320820043/diff/20001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

https://codereview.appspot.com/320820043/diff/20001/ly/music-functions-init.l...
ly/music-functions-init.ly:2007: \\voicify 1,2,3 << @dots {} \\\\ @dots {} \\\\
@dots {} >>
The white space between the macro @dots and the two braces causes make doc to
fail.

[dak, 2017-04-03 17:43:18]

On 2017/04/03 17:39:55, pkx166h wrote:
> Fails make doc.
> 
> https://codereview.appspot.com/320820043/diff/20001/ly/music-functions-init.ly
> File ly/music-functions-init.ly (right):
> 
>
https://codereview.appspot.com/320820043/diff/20001/ly/music-functions-init.l...
> ly/music-functions-init.ly:2007: \\voicify 1,2,3 << @dots {} \\\\ @dots {}
\\\\
> @dots {} >>
> The white space between the macro @dots and the two braces causes make doc to
> fail.

In my defense, make info worked fine.  Texinfo's various backends apparently
have some weird differences.  Will fix.

[dak, 2017-04-03 18:11:21]

Fix documentation problems (and some formatting)

[Trevor Daniels, 2017-04-06 14:29:58]

LGTM

David, you've found an excellent way of squaring the circle!

Trevor

[thomasmorley651, 2017-04-08 12:02:45]

Great!

Two thoughts:

(1)
it's a common request on the user list how to continue a tie into the << \\ >> -
construct.
I'm aware we explain it at the lines of "don't use the double-backslash".
Also, how to use named voices is explained, iirc.

Though, I'd like to see an explicit example like

\new Voice = main {
  a'4^~
  \voicify main , 2 
  << a' \\ c' >>
}

more prominently mentioned somewhere in the Learning Manual (even shorter than
the regtest).

Tbh, I'm not sure where to insert it, otherwise I'd commented infile.



(2)

https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm
File scm/music-functions.scm (right):

https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm#n...
scm/music-functions.scm:968: 
Why not throw an error if lengths of `lst' and `id' doesn't match?
Something at the lines of:

(if (not (= (length lst)(length id)))
	(ly:error 
		"~a: more voices (~a) than id's (~a)" 
		(*location*)
		(length lst)
		(length id)))

I'm aware about the warning below which will be printed in this case.
Though, "No more voicification ids" sounds not clear enough, imho.
And a the compilation of the .ly-file will not be successfull anyway.

[thomasmorley651, 2017-04-08 12:26:15]

https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm
File scm/music-functions.scm (right):

https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm#n...
scm/music-functions.scm:968: 
On 2017/04/08 12:02:45, thomasmorley651 wrote:
> Why not throw an error if lengths of `lst' and `id' doesn't match?

Other possibility would be to extend the id-list with defaults, if it is too
short and disregard supernumerous entries.
In both cases a warning would be appropriate.

[dak, 2017-04-09 15:13:32]

thomasmorley65@gmail.com writes:

> Great!
>
> Two thoughts:
>
> (1)
> it's a common request on the user list how to continue a tie into the <<
> \\ >> - construct.
> I'm aware we explain it at the lines of "don't use the
> double-backslash".
> Also, how to use named voices is explained, iirc.
>
> Though, I'd like to see an explicit example like
>
> \new Voice = main {
>   a'4^~
>   \voicify main , 2
>   << a' \\ c' >>
> }
>
> more prominently mentioned somewhere in the Learning Manual (even
> shorter than the regtest).
>
> Tbh, I'm not sure where to insert it, otherwise I'd commented infile.

That's sort of a "I'll leave this for the documentation guys to bikeshed
over" for me.  They tend to reorganize our stuff to make it less
makeshift and more cohesive and consistent regularly anyway.

> https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm
> File scm/music-functions.scm (right):
>
>
https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm#n...
> scm/music-functions.scm:968:
> Why not throw an error if lengths of `lst' and `id' doesn't match?
> Something at the lines of:
>
> (if (not (= (length lst)(length id)))
> 	(ly:error
> 		"~a: more voices (~a) than id's (~a)"
> 		(*location*)
> 		(length lst)
> 		(length id)))
>
> I'm aware about the warning below which will be printed in this case.
> Though, "No more voicification ids" sounds not clear enough, imho.

Well, then let's improve that warning text.  It's probably more suitable
for \voicify anyway and we more or less agreed on \voices instead.

> And a the compilation of the .ly-file will not be successfull anyway.

Why wouldn't it be successful in the manner of "file can be completed
without straying from intended meaning too far"?  At any rate, I don't
want to be too fussy here at the moment in case people want to write
stuff like

\fourVoice = \voices 1,2,3,4

and overuse it.  Just have a bad feeling over doing too much here, not a
really no-brainer argument.

> https://codereview.appspot.com/320820043/

-- 
David Kastrup

[dak, 2017-04-09 15:15:47]

thomasmorley65@gmail.com writes:

> https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm
> File scm/music-functions.scm (right):
>
>
https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm#n...
> scm/music-functions.scm:968:
> On 2017/04/08 12:02:45, thomasmorley651 wrote:
>> Why not throw an error if lengths of `lst' and `id' doesn't match?
>
> Other possibility would be to extend the id-list with defaults, if it is
> too short and disregard supernumerous entries.
> In both cases a warning would be appropriate.

Uh, effectively the id-list _is_ extended with defaults (1, 2, 3...) if
I remember correctly.

> https://codereview.appspot.com/320820043/

-- 
David Kastrup

[thomasmorley651, 2017-04-09 18:36:00]

On 2017/04/09 15:15:47, dak wrote:
> mailto:thomasmorley65@gmail.com writes:
> 
> > https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm
> > File scm/music-functions.scm (right):
> >
> >
>
https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm#n...
> > scm/music-functions.scm:968:
> > On 2017/04/08 12:02:45, thomasmorley651 wrote:
> >> Why not throw an error if lengths of `lst' and `id' doesn't match?
> >
> > Other possibility would be to extend the id-list with defaults, if it is
> > too short and disregard supernumerous entries.
> > In both cases a warning would be appropriate.
> 
> Uh, effectively the id-list _is_ extended with defaults (1, 2, 3...) if
> I remember correctly.
> 
> > https://codereview.appspot.com/320820043/
> 
> -- 
> David Kastrup

I then would have expected the following to work (with a warning):

{ \voicify 1 << a' \\ c' >> }

instead I get:

$ lilypond-git atest-54.ly
GNU LilyPond 2.19.59
Processing `atest-54.ly'
Parsing.../home/hermann/lilypond-git/build/out/share/lilypond/current/scm/music-functions.scm:978:31:
In expression ("No more voicification ids"):
/home/hermann/lilypond-git/build/out/share/lilypond/current/scm/music-functions.scm:978:31:
Wrong type to apply: "No more voicification ids"

Which is the full log.

-Harm

[thomasmorley651, 2017-04-09 18:38:16]

On 2017/04/09 15:13:32, dak wrote:
> mailto:thomasmorley65@gmail.com writes:
> 
> > Great!
> >
> > Two thoughts:
> >
> > (1)
> > it's a common request on the user list how to continue a tie into the <<
> > \\ >> - construct.
> > I'm aware we explain it at the lines of "don't use the
> > double-backslash".
> > Also, how to use named voices is explained, iirc.
> >
> > Though, I'd like to see an explicit example like
> >
> > \new Voice = main {
> >   a'4^~
> >   \voicify main , 2
> >   << a' \\ c' >>
> > }
> >
> > more prominently mentioned somewhere in the Learning Manual (even
> > shorter than the regtest).
> >
> > Tbh, I'm not sure where to insert it, otherwise I'd commented infile.
> 
> That's sort of a "I'll leave this for the documentation guys to bikeshed
> over" for me.  They tend to reorganize our stuff to make it less
> makeshift and more cohesive and consistent regularly anyway.

Touché...
I'll think more over it. No need to delay push for it.

[dak, 2017-04-09 18:50:01]

thomasmorley65@gmail.com writes:

> I then would have expected the following to work (with a warning):
>
> { \voicify 1 << a' \\ c' >> }
>
> instead I get:
>
> $ lilypond-git atest-54.ly
> GNU LilyPond 2.19.59
> Processing `atest-54.ly'
>
Parsing.../home/hermann/lilypond-git/build/out/share/lilypond/current/scm/music-functions.scm:978:31:
> In expression ("No more voicification ids"):
>
/home/hermann/lilypond-git/build/out/share/lilypond/current/scm/music-functions.scm:978:31:
> Wrong type to apply: "No more voicification ids"

This so is a bug, looks like trying to call a string as if it were a
function.  I'll take a look what I did wrong here: thanks for spotting
it!

> https://codereview.appspot.com/320820043/

-- 
David Kastrup

[dak, 2017-04-09 19:25:10]

David Kastrup <dak@gnu.org> writes:

> thomasmorley65@gmail.com writes:
>
>> I then would have expected the following to work (with a warning):
>>
>> { \voicify 1 << a' \\ c' >> }
>>
>> instead I get:
>>
>> $ lilypond-git atest-54.ly
>> GNU LilyPond 2.19.59
>> Processing `atest-54.ly'
>>
Parsing.../home/hermann/lilypond-git/build/out/share/lilypond/current/scm/music-functions.scm:978:31:
>> In expression ("No more voicification ids"):
>>
/home/hermann/lilypond-git/build/out/share/lilypond/current/scm/music-functions.scm:978:31:
>> Wrong type to apply: "No more voicification ids"
>
> This so is a bug, looks like trying to call a string as if it were a
> function.  I'll take a look what I did wrong here: thanks for spotting
> it!

Oh good grief.  After fixing that, voicify-item actually gets a list to
work with and can't use ly:music-warning, and that list can be empty.
Getting good error messages is apparently a bit harder than I thought...

>> https://codereview.appspot.com/320820043/

-- 
David Kastrup

[dak, 2017-04-09 20:30:45]

Fix the error behavior, rename \voicify to \voices

[thomasmorley651, 2017-04-10 09:12:47]

On 2017/04/09 20:30:45, dak wrote:
> Fix the error behavior, rename \voicify to \voices

Error handling is nice now.

{ 
  \voices 1 << e''2 \\ a  \\ c'' \\ a' >> 
  \voices 1,2 << e'' \\ a  \\ c'' \\ a' >>
  \voices 1,2,3 << e'' \\ a  \\ c'' \\ a' >>
}

returns appropriate warnings.

The visual output is another pointer for the user that something is wrong.

I'd say we do the documentation issue (as mentioned earlier) in a new issue.

LGTM

[dak, 2017-04-11 16:26:24]

Fix stupid blunder/omission