Make accidental styles available as context mods.

Make accidental styles available as context mods.

* lily/context-mod-scheme.cc (ly_make_context_mod):

  add exported function to create new context mod, either empty or with
  optional init list

* lily/context-mod.cc, lily/include/context-mod.hh:

  add ctor which takes modification list

* lily/parser.yy (context_def_spec_body):

  ignore `description' tag when adding context modification to prevent its
  overwriting \description inside engraver-init.ly

* ly/context-mods-init.ly:

  define all accidental styles as context modifications via helper function

* ly/engraver-init.ly (Score, MensuralStaff):

  use context mod definitions for accidental style settings

* scm/music-functions.scm (set-accidental-style):

  move styles to alist (all-accidental-styles), adding documentation strings

[Neil Puttock, 2011-08-07 15:01:33]

Hi,

This patch adds context modification identifiers for all supported accidental
styles.  This should make it much easier for users to set a style globally,
since there's no need to work out what each style sets internally.

I've followed Kieren's example here,
http://lists.gnu.org/archive/html/lilypond-user/2011-07/msg00048.html, for
naming, though I'd be happy to change it if anybody can think of more suitable
names.

There is one minor annoyance with the documentation: since most styles use the
curried function `make-accidental-rule', this doesn't give any useful
information in the notation appendix (it appears each time as an anonymous
lambda).  I'd appreciate any thoughts on how we can improve this without having
to define each use of `make-accidental-rule'.

Cheers,
Neil

[Neil Puttock, 2011-08-14 22:35:19]

http://codereview.appspot.com/4819064/diff/1/scm/music-functions.scm
File scm/music-functions.scm (right):

http://codereview.appspot.com/4819064/diff/1/scm/music-functions.scm#newcode1267
scm/music-functions.scm:1267: Staff ,(make-accidental-rule 'same-octave 0)
As Carl's pointed out here, http://codereview.appspot.com/4896043/, the current
indentation cannot be maintained.  I agree that the data structure should be
revised instead.

[pkx166h, 2011-08-28 10:07:16]

passes make and reg tests

[Reinhold, 2011-09-05 16:21:43]

LGTM.

http://codereview.appspot.com/4819064/diff/1/lily/parser.yy
File lily/parser.yy (right):

http://codereview.appspot.com/4819064/diff/1/lily/parser.yy#newcode670
lily/parser.yy:670: continue;
What exactly is the reason for this hardcoded workaround? Somehow I don't get
your comment in the patch description...

http://codereview.appspot.com/4819064/diff/1/ly/context-mods-init.ly
File ly/context-mods-init.ly (right):

http://codereview.appspot.com/4819064/diff/1/ly/context-mods-init.ly#newcode30
ly/context-mods-init.ly:30: (cdr style-settings))))))
I don't see any way around the lambda functions displayed for
make-accidental-style...

[Carl, 2011-09-05 16:29:01]

It's hard for me to track all of this carefully, but I much prefer the context
mod approach to the ly:export approach.

LGTM

[dak, 2011-09-05 16:29:08]

http://codereview.appspot.com/4819064/diff/1/lily/context-mod-scheme.cc
File lily/context-mod-scheme.cc (right):

http://codereview.appspot.com/4819064/diff/1/lily/context-mod-scheme.cc#newco...
lily/context-mod-scheme.cc:45: LY_DEFINE (ly_make_context_mod,
"ly:make-context-mod",
Can you map this to #{ \with { ... } #} ?

http://codereview.appspot.com/4819064/diff/1/ly/context-mods-init.ly
File ly/context-mods-init.ly (right):

http://codereview.appspot.com/4819064/diff/1/ly/context-mods-init.ly#newcode25
ly/context-mods-init.ly:25: (ly:make-context-mod
#{ \with { \set extraNatural #(second $style-settings)
           \set autoAccidentals #(third $style-settings)
           \set autoCautionaries #(fourth $style-settings) } #}
No, I have not actually tested it, but with the recent changes to #{ #}
something quite similar to this should actually work.  Is there a reason you
prepend the description in order to strip it again in parser.yy?

[Neil Puttock, 2011-09-05 18:30:36]

On 5 September 2011 17:21,  <reinhold.kainhofer@gmail.com> wrote:
> LGTM.

Thank you. :)

> http://codereview.appspot.com/4819064/diff/1/lily/parser.yy
> File lily/parser.yy (right):
>
> http://codereview.appspot.com/4819064/diff/1/lily/parser.yy#newcode670
> lily/parser.yy:670: continue;
> What exactly is the reason for this hardcoded workaround? Somehow I
> don't get your comment in the patch description...

Since David has also queried this, I'll reply below.

> http://codereview.appspot.com/4819064/diff/1/ly/context-mods-init.ly
> File ly/context-mods-init.ly (right):
>
> http://codereview.appspot.com/4819064/diff/1/ly/context-mods-init.ly#newcode30
> ly/context-mods-init.ly:30: (cdr style-settings))))))
> I don't see any way around the lambda functions displayed for
> make-accidental-style...

It's rather unfortunate, since it really defeats the object of having
documentation.  I suppose the only alternative would be avoid currying
and have several specific rule functions.

Cheers,
Neil

[Neil Puttock, 2011-09-05 18:33:36]

On 5 September 2011 17:29,  <Carl.D.Sorensen@gmail.com> wrote:
> It's hard for me to track all of this carefully, but I much prefer the
> context mod approach to the ly:export approach.
>
> LGTM

Thanks. :)

BTW, this doesn't bypass the existing ly:export version, since that's
still required for setting accidental styles in music.  We could hack
the parser to allow music inside a context defintion, but that's
frowned upon seeing as we used to allow a similar construct inside
\midi {} for \tempo which was removed.

Cheers,
Neil

[Neil Puttock, 2011-09-05 18:47:01]

On 5 September 2011 17:29,  <dak@gnu.org> wrote:
>
> http://codereview.appspot.com/4819064/diff/1/lily/context-mod-scheme.cc
> File lily/context-mod-scheme.cc (right):
>
>
http://codereview.appspot.com/4819064/diff/1/lily/context-mod-scheme.cc#newco...
> lily/context-mod-scheme.cc:45: LY_DEFINE (ly_make_context_mod,
> "ly:make-context-mod",
> Can you map this to #{ \with { ... } #} ?

Hmm, I don't think so, since a context modification isn't music.

> http://codereview.appspot.com/4819064/diff/1/ly/context-mods-init.ly
> File ly/context-mods-init.ly (right):
>
> http://codereview.appspot.com/4819064/diff/1/ly/context-mods-init.ly#newcode25
> ly/context-mods-init.ly:25: (ly:make-context-mod
> #{ \with { \set extraNatural #(second $style-settings)
>           \set autoAccidentals #(third $style-settings)
>           \set autoCautionaries #(fourth $style-settings) } #}
> No, I have not actually tested it, but with the recent changes to #{ #}
> something quite similar to this should actually work.  Is there a reason
> you prepend the description in order to strip it again in parser.yy?

The \description keyword is used inside a context modification to set
the documentation string for the notation appendix.  Since I've
replaced the existing ly:export versions of the default accidental
styles in engraver-init.ly with their context modification
equivalents, I don't want their \description settings to overshadow
the engraver documentation strings (also set via \description): so the
context modification strings are dropped.

Cheers,
Neil

[Keith, 2011-10-20 04:06:03]

Neil, don't forget to push this.
I've been looking forward to it.

[dak, 2011-10-20 05:44:38]

Sorry, I always start thinking only when far too late in the game.  Anyway, here
is a musing:
should't the _main_ interface to accidental modifications be context mods?  You
won't need to set them in midcontext except in unusual situations, and then
\applyContext (?) is available.

Or does that make it hard to establish a global setting?  I really have no clue,
and this should not affect patch status.

[Neil Puttock, 2011-10-20 12:57:21]

On 2011/10/20 04:06:03, Keith wrote:
> Neil, don't forget to push this.
> I've been looking forward to it.

I'll try to sort it out at the weekend.  Unfortunately I don't have a working
Linux install at the moment, and foolishly neglected to back it up for easier
restoring.

Cheers,
Neil

[Neil Puttock, 2011-10-20 13:04:33]

On 2011/10/20 05:44:38, dak wrote:
> Sorry, I always start thinking only when far too late in the game.  Anyway,
here
> is a musing:
> should't the _main_ interface to accidental modifications be context mods? 
You
> won't need to set them in midcontext except in unusual situations, and then
> \applyContext (?) is available.

That would be possible, though you'd probably want a nicer interface than using
\applyContext directly.  A music function would have to have two args: the
optional context string and the context-mod identifier.

Cheers,
Neil

[dak, 2011-10-21 10:23:07]

n.puttock@gmail.com writes:

> On 2011/10/20 05:44:38, dak wrote:
>> Sorry, I always start thinking only when far too late in the game.
> Anyway, here
>> is a musing:
>> should't the _main_ interface to accidental modifications be context
> mods?  You
>> won't need to set them in midcontext except in unusual situations, and
> then
>> \applyContext (?) is available.
>
> That would be possible, though you'd probably want a nicer interface
> than using \applyContext directly.  A music function would have to
> have two args: the optional context string and the context-mod
> identifier.

I think the current state of dev/syntax should be committable.  It has
no problems accepting context modifications.  The function signature of
((string? "Bottom") ly:context-mod?) should work just fine for accepting
an optional context string defaulting to "Bottom" and a mandatory
context modification.

I'll try submitting a Rietveld review soonish.

-- 
David Kastrup

[Neil Puttock, 2011-10-21 10:36:57]

On 20 October 2011 15:01, David Kastrup <dak@gnu.org> wrote:

> I think the current state of dev/syntax should be committable.  It has
> no problems accepting context modifications.  The function signature of
> ((string? "Bottom") ly:context-mod?) should work just fine for accepting
> an optional context string defaulting to "Bottom" and a mandatory
> context modification.

The default context would be "Staff".  There's some code to switch to
GrandStaff for the piano styles.

Cheers,
Neil