Re: Let parser accept symbols after \new, \context, \unset and implicit \set (issue 13180044)

[Janek Warchoł]

ok, thanks.

2013/8/27  <address@hidden>:
> 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))
>
>