Let parser accept symbols after \new, \context, \unset and implicit \set

Let parser accept symbols after \new, \context, \unset and implicit \set

After the symbol list changes of issue 2883, not allowing single
symbols in these places seems like an anachronism.

[janek, 2013-08-27 07:48:08]

Looks like a good thing to do, but what does this change mean?  Will we be a)
able to do things that were previously impossible or b) is there no user-visible
difference?
I think the answer is b), but i'd like to make sure.

[dak, 2013-08-27 08:25:11]

On 2013/08/27 07:48:08, janek wrote:
> Looks like a good thing to do, but what does this change mean?

It means it lets the parser accept symbols after \new, \context, \unset and
implicit \set.  After the symbol list changes of issue 2883 (which made x.y
equivalent to #'(x y)), not allowing single symbols in these places seems like
an anachronism.

To wit, \override #'Staff #'NoteHead #'color = #red is proper code, while \new
#'Staff { ... } isn't, but \new #"Staff" { ... } is allowed again.  That seems
like an anachronism.

>  Will we be a)
> able to do things that were previously impossible or b) is there no
user-visible
> difference?
> I think the answer is b), but i'd like to make sure.

I think "Let the parser accept ..." makes pretty clear that we are talking about
extending permitted constructs.

[janek, 2013-08-27 08:39:07]

ok, thanks.

2013/8/27  <dak@gnu.org>:
> Reviewers: janek,
>
> Message:
>
> On 2013/08/27 07:48:08, janek wrote:
>>
>> Looks like a good thing to do, but what does this change mean?
>
>
> It means it lets the parser accept symbols after \new, \context, \unset
> and implicit \set.  After the symbol list changes of issue 2883 (which
> made x.y equivalent to #'(x y)), not allowing single symbols in these
> places seems like an anachronism.
>
> To wit, \override #'Staff #'NoteHead #'color = #red is proper code,
> while \new #'Staff { ... } isn't, but \new #"Staff" { ... } is allowed
> again.  That seems like an anachronism.
>
>
>>   Will we be a)
>> able to do things that were previously impossible or b) is there no
>
> user-visible
>>
>> difference?
>> I think the answer is b), but i'd like to make sure.
>
>
> I think "Let the parser accept ..." makes pretty clear that we are
> talking about extending permitted constructs.
>
> Description:
> Let parser accept symbols after \new, \context, \unset and implicit \set
>
> After the symbol list changes of issue 2883, not allowing single
> symbols in these places seems like an anachronism.
>
> Please review this at https://codereview.appspot.com/13180044/
>
> Affected files:
>   M lily/parser.yy
>   M scm/ly-syntax-constructors.scm
>
>
> Index: lily/parser.yy
> diff --git a/lily/parser.yy b/lily/parser.yy
> index
>
53bddf5eaf64dcf48824e05740d20d330919faab..5ac60f65bc4aa3c3aaf08bc10c494b0bf2a31a09
> 100644
> --- a/lily/parser.yy
> +++ b/lily/parser.yy
> @@ -1877,14 +1877,14 @@ complex_music:
>         ;
>
>  complex_music_prefix:
> -       CONTEXT simple_string optional_id optional_context_mod {
> +       CONTEXT symbol optional_id optional_context_mod {
>                  Context_mod *ctxmod = unsmob_context_mod ($4);
>                  SCM mods = SCM_EOL;
>                  if (ctxmod)
>                          mods = ctxmod->get_mods ();
>                 $$ = START_MAKE_SYNTAX ("context-specification", $2, $3,
> mods, SCM_BOOL_F);
>         }
> -       | NEWCONTEXT simple_string optional_id optional_context_mod {
> +       | NEWCONTEXT symbol optional_id optional_context_mod {
>                  Context_mod *ctxmod = unsmob_context_mod ($4);
>                  SCM mods = SCM_EOL;
>                  if (ctxmod)
> @@ -2024,13 +2024,11 @@ property_path:
>         ;
>
>  property_operation:
> -       STRING '=' scalar {
> -               $$ = scm_list_3 (ly_symbol2scm ("assign"),
> -                       scm_string_to_symbol ($1), $3);
> +       symbol '=' scalar {
> +               $$ = scm_list_3 (ly_symbol2scm ("assign"), $1, $3);
>         }
> -       | UNSET simple_string {
> -               $$ = scm_list_2 (ly_symbol2scm ("unset"),
> -                       scm_string_to_symbol ($2));
> +       | UNSET symbol {
> +               $$ = scm_list_2 (ly_symbol2scm ("unset"), $2);
>         }
>         | OVERRIDE property_path '=' scalar {
>                 if (scm_ilength ($2) < 2) {
> @@ -2296,6 +2294,26 @@ simple_string: STRING {
>         }
>         ;
>
> +symbol:
> +       STRING {
> +               $$ = scm_string_to_symbol ($1);
> +       }
> +       | embedded_scm_bare
> +       {
> +               // This is a bit of overkill but makes the same
> +               // routine responsible for all symbol interpretations.
> +               $$ = try_string_variants (ly_lily_module_constant
> ("symbol?"),
> +                                         $1);
> +               if (SCM_UNBNDP ($$))
> +               {
> +                       parser->parser_error (@1, (_ ("symbol expected")));
> +                       // Generate a unique symbol in case it is used
> +                       // for an assignment or similar
> +                       $$ = scm_make_symbol (ly_string2scm ("undefined"));
> +               }
> +       }
> +       ;
> +
>  scalar:
>         embedded_scm_arg
>         | SCM_IDENTIFIER
> Index: scm/ly-syntax-constructors.scm
> diff --git a/scm/ly-syntax-constructors.scm b/scm/ly-syntax-constructors.scm
> index
>
01fc9bcef820c41cdc6c6add54c9b7459f5efb7c..8b6f4ffefbf01c2ff05fdedce13467e679872e1e
> 100644
> --- a/scm/ly-syntax-constructors.scm
> +++ b/scm/ly-syntax-constructors.scm
> @@ -152,8 +152,7 @@ into a @code{MultiMeasureTextEvent}."
>                'origin location))
>
>  (define-ly-syntax-simple (context-specification type id ops create-new mus)
> -  (let* ((type-sym (if (symbol? type) type (string->symbol type)))
> -         (csm (context-spec-music mus type-sym id)))
> +  (let ((csm (context-spec-music mus type id)))
>      (set! (ly:music-property csm 'property-operations) ops)
>      (if create-new (set! (ly:music-property csm 'create-new) #t))
>      csm))
>
>