Implement new markup-command combine-list

Implement new markup-command combine-list

Allows for \markup \combine-list { a list }

[thomasmorley651, 2015-08-30 11:25:01]

Please review

PATCH_NEW

[dak, 2015-08-30 11:44:52]

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

https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.sc...
scm/define-markup-commands.scm:1758: (define-markup-command (combine-list layout
props args)
For better or worse, we don't have other commands of *-list form.  Commands that
_deliver_ a markup list are usually named *-lines for whatever reason.  But this
command delivers a single markup.  I don't have a better proposal though.

https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.sc...
scm/define-markup-commands.scm:1778: (map (lambda (e) (interpret-markup layout
props e)) args)))
This does not work for markup lists built using markup list commands.  The whole
line should rather be (interpret-markup-list layout props args)))

[pkx166h, 2015-08-30 11:51:40]

Passes make, make check and a full make doc

PATCH_REVIEW

[thomasmorley651, 2015-08-30 11:55:04]

David's comment

[thomasmorley651, 2015-08-30 12:02:14]

On 2015/08/30 11:44:52, dak wrote:
> https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.scm
> File scm/define-markup-commands.scm (right):
> 
>
https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.sc...
> scm/define-markup-commands.scm:1758: (define-markup-command (combine-list
layout
> props args)
> For better or worse, we don't have other commands of *-list form.  Commands
that
> _deliver_ a markup list are usually named *-lines for whatever reason.  But
this
> command delivers a single markup.  I don't have a better proposal though.

I thought about deleting old \combine, but too much user-code relies on it.
Apart from not knowing python I really have no clue how something like the below
could be caught by a convert-rule:

\markup {
\combine
 arg1
 \combine
  arg2
  \combine
   arg3
   arg4
}

and transformed to:

\markup \combine-list { arg1 arg2 arg3 arg4 }

> 
>
https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.sc...
> scm/define-markup-commands.scm:1778: (map (lambda (e) (interpret-markup layout
> props e)) args)))
> This does not work for markup lists built using markup list commands.  The
whole
> line should rather be (interpret-markup-list layout props args)))

Done. 
Thanks.

PATCH_NEW

[dak, 2015-08-30 12:08:29]

On 2015/08/30 12:02:14, thomasmorley651 wrote:
> On 2015/08/30 11:44:52, dak wrote:
> >
https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.scm
> > File scm/define-markup-commands.scm (right):
> > 
> >
>
https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.sc...
> > scm/define-markup-commands.scm:1758: (define-markup-command (combine-list
> layout
> > props args)
> > For better or worse, we don't have other commands of *-list form.  Commands
> that
> > _deliver_ a markup list are usually named *-lines for whatever reason.  But
> this
> > command delivers a single markup.  I don't have a better proposal though.
> 
> I thought about deleting old \combine,

Yes, that sounds like a sensible path forward.

>but too much user-code relies on it.
> Apart from not knowing python I really have no clue how something like the
below
> could be caught by a convert-rule:
> 
> \markup {
> \combine
>  arg1
>  \combine
>   arg2
>   \combine
>    arg3
>    arg4
> }
> 
> and transformed to:
> 
> \markup \combine-list { arg1 arg2 arg3 arg4 }

Probably more doable than recognizing a single markup reliably.  I'd have to see
how well this works.

[pkx166h, 2015-08-30 12:29:01]

Passes make, make check and a full make doc

PATCH_REVIEW

[dak, 2015-08-31 10:27:04]

On 2015/08/30 12:08:29, dak wrote:
> On 2015/08/30 12:02:14, thomasmorley651 wrote:
> > On 2015/08/30 11:44:52, dak wrote:
> > >
> https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.scm
> > > File scm/define-markup-commands.scm (right):
> > > 
> > >
> >
>
https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.sc...
> > > scm/define-markup-commands.scm:1758: (define-markup-command (combine-list
> > layout
> > > props args)
> > > For better or worse, we don't have other commands of *-list form. 
Commands
> > that
> > > _deliver_ a markup list are usually named *-lines for whatever reason. 
But
> > this
> > > command delivers a single markup.  I don't have a better proposal though.
> > 
> > I thought about deleting old \combine,
> 
> Yes, that sounds like a sensible path forward.
> 
> >but too much user-code relies on it.
> > Apart from not knowing python I really have no clue how something like the
> below
> > could be caught by a convert-rule:
> > 
> > \markup {
> > \combine
> >  arg1
> >  \combine
> >   arg2
> >   \combine
> >    arg3
> >    arg4
> > }
> > 
> > and transformed to:
> > 
> > \markup \combine-list { arg1 arg2 arg3 arg4 }
> 
> Probably more doable than recognizing a single markup reliably.  I'd have to
see
> how well this works.

Ok, I've taken a look.  Markups are so undelimited in their nature that there is
not much of an option other than convert-ly actually knowing all markup commands
with their signatures.  That's doable but rather onerous.  In addition,
musicxml2ly uses the \combine command as well.

So we are likely better off with a new command name.  "\combines" would also be
a possibility but is probably too cute/confusing/unhyphenated.  Now if we
basically phase out \combine and replace most of its uses manually, it would
become an option just to use some _unrelated_ word.

Like \superimpose or \overlay or \collect or something other nice.  Or just keep
the bikeshed in the originally proposed color.

[thomasmorley651, 2015-09-05 06:29:07]

On 2015/08/31 10:27:04, dak wrote:
> 
> Ok, I've taken a look.  Markups are so undelimited in their nature that there
is
> not much of an option other than convert-ly actually knowing all markup
commands
> with their signatures.  That's doable but rather onerous.  In addition,
> musicxml2ly uses the \combine command as well.
> 
> So we are likely better off with a new command name.  "\combines" would also
be
> a possibility but is probably too cute/confusing/unhyphenated.  Now if we
> basically phase out \combine and replace most of its uses manually, it would
> become an option just to use some _unrelated_ word.
> 
> Like \superimpose or \overlay or \collect or something other nice.  Or just
keep
> the bikeshed in the originally proposed color.

Thanks for your time looking into a possible convert-rule.

I'm really tempted to delete old \combine and fixing all occurences in our
source.
Would it be an option to do a convert-rule outputting "Not smart enough to ..."?
for a combine-command without a markup-list as argument?
I seem to remember something like that has been done before.

If that's not feasible or wanted, I'd prefer \combine-list, because it's
descriptive (in a technical way) or \overlay, because there is a LSR-snippet
providing the functionality already and some users may be used to it.
http://lsr.di.unimi.it/LSR/Item?id=628

What do people think?

@James
I suggest to let this one on REVIEW for another circle

[dak, 2015-09-05 09:47:09]

thomasmorley65@gmail.com writes:

> On 2015/08/31 10:27:04, dak wrote:
>
>> Ok, I've taken a look.  Markups are so undelimited in their nature
> that there is
>> not much of an option other than convert-ly actually knowing all
> markup commands
>> with their signatures.  That's doable but rather onerous.  In
> addition,
>> musicxml2ly uses the \combine command as well.
>
>> So we are likely better off with a new command name.  "\combines"
> would also be
>> a possibility but is probably too cute/confusing/unhyphenated.  Now if
> we
>> basically phase out \combine and replace most of its uses manually, it
> would
>> become an option just to use some _unrelated_ word.
>
>> Like \superimpose or \overlay or \collect or something other nice.
>> Or just keep the bikeshed in the originally proposed color.
>
> Thanks for your time looking into a possible convert-rule.
>
> I'm really tempted to delete old \combine and fixing all occurences in
> our source.
> Would it be an option to do a convert-rule outputting "Not smart enough
> to ..."? for a combine-command without a markup-list as argument?
> I seem to remember something like that has been done before.

It's been a really long time since we last did something like that.

> If that's not feasible or wanted, I'd prefer \combine-list, because it's
> descriptive (in a technical way) or \overlay, because there is a
> LSR-snippet providing the functionality already and some users may be
> used to it.
> http://lsr.di.unimi.it/LSR/Item?id=628
>
> What do people think?

I'd go for \overlay then.  The LSR snippet has slightly different
semantics: it excludes empty stencils and non-stencils and does not
return an empty stencil but at least a point-stencil, but all that are
decisions I don't consider a good idea for a general command.  It would
probably make sense to change the snippet to just do
(apply ly:stencil-add (interpret-markup-list layout props args))
until it's retired altogether.  That way we avoid entertaining competing
slightly incompatible definitions.

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

-- 
David Kastrup

[thomasmorley651, 2015-09-06 10:31:16]

change name to overlay

[thomasmorley651, 2015-09-06 10:39:02]

On 2015/09/06 10:31:16, thomasmorley651 wrote:
> change name to overlay

After it's been pushed I'll change said LSR-snippet accordingly.

For now:
PATch_NEW

[dak, 2015-09-06 10:45:08]

LGTM