Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(976)

Issue 553760043: Add dynamic-interface to keepAliveInterfaces

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 months, 2 weeks ago by jean
Modified:
2 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Add dynamic-interface to keepAliveInterfaces This will prevent \Remove[All]EmptyStaves from removing Dynamics contexts containing dynamics attached to spacer rests but no actual notes (the standard use case for a Dynamics context).

Patch Set 1 #

Patch Set 2 : Add dynamic-interface to keepAliveInterfaces #

Patch Set 3 : Add dynamic-interface to keepAliveInterfaces #

Patch Set 4 : Add dynamic-interface to keepAliveInterfaces #

Patch Set 5 : Add dynamic-interface to keepAliveInterfaces #

Patch Set 6 : Add dynamic-interface to keepAliveInterfaces #

Patch Set 7 : Add dynamic-interface to keepAliveInterfaces #

Patch Set 8 : Add dynamic-interface to keepAliveInterfaces #

Patch Set 9 : Add dynamic-interface to keepAliveInterfaces #

Total comments: 2

Patch Set 10 : Adress review comments on the documentation #

Patch Set 11 : Documentation improvements #

Patch Set 12 : Add a regression test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -7 lines) Patch
M Documentation/notation/staff.itely View 1 2 3 4 5 6 7 8 9 10 4 chunks +35 lines, -7 lines 0 comments Download
A input/regression/remove-empty-staves-dynamics.ly View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +13 lines, -0 lines 0 comments Download
M ly/engraver-init.ly View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 25
jean
Add dynamic-interface to keepAliveInterfaces
3 months, 1 week ago (2020-03-25 21:51:14 UTC) #1
jean
Add dynamic-interface to keepAliveInterfaces
3 months, 1 week ago (2020-03-25 21:52:13 UTC) #2
jean
Add dynamic-interface to keepAliveInterfaces
3 months, 1 week ago (2020-03-26 10:33:10 UTC) #3
jean
Add dynamic-interface to keepAliveInterfaces
3 months, 1 week ago (2020-03-26 10:33:44 UTC) #4
jean
Add dynamic-interface to keepAliveInterfaces
3 months, 1 week ago (2020-03-26 10:39:15 UTC) #5
jean
Add dynamic-interface to keepAliveInterfaces
3 months, 1 week ago (2020-03-26 11:26:12 UTC) #6
jean
Add dynamic-interface to keepAliveInterfaces
3 months, 1 week ago (2020-03-26 11:35:59 UTC) #7
dak
What I am worried about is a partitura that puts common dynamics on every staff, ...
3 months, 1 week ago (2020-03-26 12:01:40 UTC) #8
jean
Add dynamic-interface to keepAliveInterfaces
3 months, 1 week ago (2020-03-26 12:24:33 UTC) #9
lemzwerg
LGTM https://codereview.appspot.com/553760043/diff/565830044/Documentation/notation/staff.itely File Documentation/notation/staff.itely (right): https://codereview.appspot.com/553760043/diff/565830044/Documentation/notation/staff.itely#newcode801 Documentation/notation/staff.itely:801: rests, skips, or a combination of these elements. ...
3 months, 1 week ago (2020-03-26 12:50:11 UTC) #10
wl_gnu.org
> What I am worried about is a partitura that puts common dynamics on > ...
3 months, 1 week ago (2020-03-26 12:52:29 UTC) #11
Valentin Villenave
On 2020/03/26 12:52:29, wl_gnu.org wrote: > Never seen that, AFAIK. Can you provide a link ...
3 months, 1 week ago (2020-03-26 16:31:17 UTC) #12
wl_gnu.org
>> Never seen that, AFAIK. Can you provide a link to such a score? > ...
3 months, 1 week ago (2020-03-26 16:41:41 UTC) #13
jean
Adress review comments on the documentation
3 months, 1 week ago (2020-03-27 18:39:35 UTC) #14
jean
This is a relevant point, but I think Werner is right. You can put your ...
3 months, 1 week ago (2020-03-27 19:00:08 UTC) #15
Valentin Villenave
On 2020/03/27 19:00:08, jean wrote: > I can see no situation where the current value ...
3 months, 1 week ago (2020-03-27 23:08:23 UTC) #16
Dan Eble
Is the impact (if any) on existing scores important? (cases that we might not have ...
3 months, 1 week ago (2020-03-27 23:16:40 UTC) #17
Dan Eble
On 2020/03/27 23:16:40, Dan Eble wrote: > Is the impact (if any) on existing scores ...
3 months, 1 week ago (2020-03-27 23:29:56 UTC) #18
jean
On 2020/03/27 23:16:40, Dan Eble wrote: > Is the impact (if any) on existing scores ...
3 months, 1 week ago (2020-03-31 20:14:44 UTC) #19
jean
On 2020/03/31 20:14:44, jean wrote: > I'll update the documentation with this. Hum, while exploring ...
3 months, 1 week ago (2020-03-31 20:26:51 UTC) #20
Valentin Villenave
On 3/31/20, jean@abou-samra.fr <jean@abou-samra.fr> wrote: > By the way, why do we say \RemoveEmptyStaves instead ...
3 months, 1 week ago (2020-03-31 20:43:03 UTC) #21
Valentin Villenave
On 3/31/20, jean@abou-samra.fr <jean@abou-samra.fr> wrote: > I'll take a look at it, and try to ...
3 months, 1 week ago (2020-03-31 20:51:05 UTC) #22
jean
Documentation improvements
2 months ago (2020-05-03 19:19:42 UTC) #23
jean
Add a regression test
2 months ago (2020-05-03 19:37:17 UTC) #24
jean
2 months ago (2020-05-03 20:03:56 UTC) #25
Thanks for you support, Valentin.

> … And, yes, I do realize that’s way too convoluted of an explanation.
If someone else can do it more straightforwardly, have at it! :-)
Hey, don't make me feel bad about my own more-than-lengthy explanations!

On 2020/03/31 20:51:05, Valentin Villenave wrote:
> You can’t say it’s undocumented, its documentation is just not meant
> for regular users:

True, I meant that this isn't just useful for internal use, you may need it as a
regular user, I think, because a conductor would think the choir or the strings
as a whole, so it can sound weird to have a system with only violin I without
violin II (hope I'm clear).

There is now a regression test as asked for, and a bit more documentation. I
added an example about Keep_alive_together_engraver, though I'm not completely
satified with it--feel free to improve. It is taken from
https://www.universaledition.com/kurt-weill-764/works/der-jasager-1900 (Kurt
Weill, Der Jasager). This score isn't actually demonstrating the need for this
(it could have been if the breaks were placed another way) but if this is
wanted, here is an example showing that it happens in printed music:
http://imslp.eu/files/imglnks/euimg/5/54/IMSLP522307-PMLP3653-NBE_-_Symphonie...
(the first page).

Yet, there is still an issue. What's happening in this example?
\version "2.21.0"

keepRests =
\applyContext
  #(lambda (context)
       (ly:context-set-property! context 'keepAliveInterfaces
         (cons 'rest-interface (ly:context-property context
'keepAliveInterfaces))))

\layout {
  \context {
    \Score
    \RemoveAllEmptyStaves
    \keepRests
  }
}

<<
  { c'1 c'1 c'1 }
  { R1 \break s1 \break R1 }
>>

I expect the second staff in the second system to disappear, but this is not the
case. Yet if you replace the surrounding R1 with r1, then it works. Did I
misunderstand the way \RemoveEmptyStaves works? How can it depend on the other
systems? It can't be put in the documentation if it doesn't work...

Thanks!
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b