lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/5] Remove parser/location global variable setup


From: David Kastrup
Subject: Re: [PATCH 1/5] Remove parser/location global variable setup
Date: Sun, 31 May 2015 07:47:38 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux)

"Trevor Daniels" <address@hidden> writes:

> David Kastrup wrote Saturday, May 30, 2015 7:51 PM
>  
>> Oh, and regarding the %parser and %location names: GUILE itself uses
>> names like that for several fluids.
>> 
>> At any rate: huh.  I propose getting rid of a 10-year old syntactic
>> artifact and the only response are naming convention enquiries.  I mean:
>> good to know that there are still developers alive. 
>
> I should have responded, having spent quite some time yesterday
> browsing the patch set.  I tried to understand how it worked, but
> having little familiarity with C++, or even C for that matter, I
> decided the technique was too esoteric for me to understand, at least
> without a lot of background reading first.

Well, the C part of the patch is all of

--- a/lily/parse-scm.cc
+++ b/lily/parse-scm.cc
@@ -146,15 +146,15 @@ protected_ly_parse_scm (Parse_start *ps)
 }
 
 SCM
-protected_ly_eval_scm (Parse_start *ps)
+protected_ly_eval_scm (void *ps)
 {
   /*
     Catch #t : catch all Scheme level errors.
    */
   return scm_internal_catch (SCM_BOOL_T,
                              catch_protected_eval_body,
-                             (void *) ps,
-                             &parse_handler, (void *) ps);
+                             ps,
+                             &parse_handler, ps);
 }
 
 bool parse_protect_global = true;
@@ -178,8 +178,14 @@ ly_eval_scm (SCM form, Input i, bool safe, Lily_parser 
*parser)
 {
   Parse_start ps (form, i, safe, parser);
 
-  SCM ans = parse_protect_global ? protected_ly_eval_scm (&ps)
-            : internal_ly_eval_scm (&ps);
+  SCM ans = scm_c_with_fluids
+    (scm_list_2 (ly_lily_module_constant ("%parser"),
+                 ly_lily_module_constant ("%location")),
+     scm_list_2 (parser->self_scm (),
+                 i.smobbed_copy ()),
+     parse_protect_global ? protected_ly_eval_scm
+     : catch_protected_eval_body, (void *) &ps);
+
   scm_remember_upto_here_1 (form);
   return ans;
 }

Where the only non-trivial part is the last hunk.  Which replaces a
simple call with one through scm_c_with_fluids.  But now I am nitpicking
about irrelevant details.  Which might be part of the reason people
don't bother with feedback.

> Apart from commenting about my ignorance there seemed little to say at
> the time.
>
> But of course the result is very welcome, removing a requirement which
> is irritating and mysterious when first encountered, and a nuisance
> ever after.

Well, that was actually more what I considered strange: the lack of
comment on the result/merits rather than on the particular
implementation.  It's a bit of a "if you don't have anything bad to say,
say nothing at all" culture we are sometimes projecting, and it's not
like I am not a solid part of it.

>> I still think it may have the potential of issue 2883 where its
>> general availability lagged behind people becoming used to it.
>
> Not really.  No one will miss the need to add parser location.  The
> change just normalises the syntax to what one would expect anyway.  It
> will be appreciated in the same way that good health is appreciated
> after recovering from an illness.

Aaaaand I need to forego the habit of talking in cryptic little riddles
when I want to have a conversation.

The "potential of issue 2883", namely

    \override Voice.Stem #'details #'beamed-lengths = #'(6 10 8) =>
    \override Voice.Stem.details.beamed-lengths = #'(6 10 8)

was such that people got used to it very fast, leading to the situation
where a lot of advice was handed out on user and developer list (and was
read in our development manuals on the web) that led to the response
"doesn't work for me".  The advice could have sticked with the old,
still supported forms.  But people got used to not having to think here
fast.  Now admittedly leaving off the "parser location" incantation is
not a brain saver on a similar level.

It still pushes the eyes-glaze-over-don't-bother threshold a bit further
away hopefully.

> Thanks for doing this, David.

And thanks for sticking the head out and providing the feedback and
heads-up.  It's appreciated even though it may appear getting received
with a glare and all up in arms.

-- 
David Kastrup



reply via email to

[Prev in Thread] Current Thread [Next in Thread]