Menu

#4639 Doc - inconsistency for ly:context-pushpop-property with regart to 'override'

Verified
Other
2015-11-12
2015-10-19
No

Paul Morris paul@paulwmorris.com writes:

The docs appear to be inconsistent about ly:context-pushpop-property.
Does it do a \temporary \override or just an \override ?

-Paul


ly:context-pushpop-property

do a \temporary \override or a \revert on a grob property

http://www.lilypond.org/doc/v2.19/Documentation/extending/context-evaluation


Function: ly:context-pushpop-property context grob eltprop val

Do a single \override or \revert operation in context. The grob
definition grob is extended with eltprop (if val is specified) or
reverted (if unspecified).

http://lilypond.org/doc/v2.19/Documentation/internals/scheme-functions

It does a "\temporary \override". Well spotted.

We also have the following definitions:

(define-public (make-grob-property-set grob gprop val)
"Make a @code{Music} expression that sets @var{gprop} to @var{val} in
@var{grob}. Does a pop first, i.e., this is not an override."
(make-music 'OverrideProperty
'symbol grob
'grob-property gprop
'grob-value val
'pop-first #t))

(define-public (make-grob-property-override grob gprop val)
"Make a @code{Music} expression that overrides @var{gprop} to @var{val}
in @var{grob}."
(make-music 'OverrideProperty
'symbol grob
'grob-property gprop
'grob-value val))

Which we need to maintain for compatibility, along with
make-voice-props-set and make-voice-props-override and
override-heads-style.

In all of those cases, "override" means "\temporary \override" while
"set" means "\override".

Maybe one should at least fix the documentation strings. Like
"i.e., this is not a @code{\temporary \override}."

and "that temporarily overrides" or "that does a @code{\temporary
\override}".

Apart from inventing the actual \temporary command, I merely inherited
this mess. Before \temporary, there was just "\override" which did the
same as the Scheme functions "-set" rather than "-override", namely
always implying pop-first = #t .

--
David Kastrup

Discussion

  • Trevor Daniels

    Trevor Daniels - 2015-10-26
    • status: New --> Accepted
     
  • Anonymous

    Anonymous - 2015-11-01
    • status: Accepted --> Started
    • assigned_to: pkx166h
     
  • Anonymous

    Anonymous - 2015-11-01

    I have a patch done but am struggling getting git-cl to work. If I cannot get git-cl working I will upload the patch here so that we don't have others duplicating work.

     
  • Anonymous

    Anonymous - 2015-11-02

    Doc: scm - Clarify ly:context-pushpop-property

    Issue 4639

    Clarified when we mean
    '\temporary \overrride'
    and just '\overrride'.

    http://codereview.appspot.com/274250043

     
  • Anonymous

    Anonymous - 2015-11-02
    • Needs: -->
    • Patch: new --> needs_work
    • Type: --> Other
     
  • Anonymous

    Anonymous - 2015-11-03

    With David K's corrections.

    http://codereview.appspot.com/274250043

     
  • Anonymous

    Anonymous - 2015-11-03

    Put back accidentally deleted line - note to self: don't do edits when you are tired (and distracted).

    http://codereview.appspot.com/274250043

     
  • Anonymous

    Anonymous - 2015-11-03
    • Needs: -->
    • Patch: new --> review
    • Type: --> Other
     
  • Anonymous

    Anonymous - 2015-11-03

    Passes make, make check and a full make doc.

     
  • Anonymous

    Anonymous - 2015-11-04

    Patch on countdown for November 7th

     
  • Anonymous

    Anonymous - 2015-11-04
    • Patch: review --> countdown
     
  • Anonymous

    Anonymous - 2015-11-07
    • Patch: countdown --> push
     
  • Anonymous

    Anonymous - 2015-11-07

    Patch counted down - please push

     
  • Anonymous

    Anonymous - 2015-11-07
    • labels: --> FIxed_2_19_31
    • status: Started --> Fixed
    • Patch: push -->
     
  • Anonymous

    Anonymous - 2015-11-07

    author James Lowe pkx166h@gmail.com
    Sun, 1 Nov 2015 15:02:54 +0000 (15:02 +0000)
    committer James Lowe pkx166h@gmail.com
    Sat, 7 Nov 2015 17:31:13 +0000 (17:31 +0000)
    commit 0f3099177a2a23347b0a8c0895adbebc87d416be

     
  • Federico Bruni

    Federico Bruni - 2015-11-12
    • status: Fixed --> Verified