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

Issue 5686046: Add layout changing command \layout-from for getting context defs from music (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 2 months ago by dak
Modified:
12 years, 2 months ago
Reviewers:
janek
CC:
lilypond-devel_gnu.org
Base URL:
http://git.savannah.gnu.org/gitweb/?p=lilypond.git/trunk/
Visibility:
Public.

Description

Add layout changing command \layout-from for getting context defs from music

Patch Set 1 #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+86 lines, -0 lines) Patch
A input/regression/layout-from.ly View 1 chunk +19 lines, -0 lines 0 comments Download
M ly/context-mods-init.ly View 1 chunk +67 lines, -0 lines 7 comments Download

Messages

Total messages: 3
janek
LGTM http://codereview.appspot.com/5686046/diff/1/ly/context-mods-init.ly File ly/context-mods-init.ly (right): http://codereview.appspot.com/5686046/diff/1/ly/context-mods-init.ly#newcode45 ly/context-mods-init.ly:45: contexts. I like this description. http://codereview.appspot.com/5686046/diff/1/ly/context-mods-init.ly#newcode49 ly/context-mods-init.ly:49: to ...
12 years, 2 months ago (2012-02-23 00:13:34 UTC) #1
dak
http://codereview.appspot.com/5686046/diff/1/ly/context-mods-init.ly File ly/context-mods-init.ly (right): http://codereview.appspot.com/5686046/diff/1/ly/context-mods-init.ly#newcode49 ly/context-mods-init.ly:49: to @code{'Voice}. On 2012/02/23 00:13:34, janek wrote: > I ...
12 years, 2 months ago (2012-02-23 00:53:57 UTC) #2
janek
12 years, 2 months ago (2012-02-23 05:52:32 UTC) #3
HTH,
Janek

http://codereview.appspot.com/5686046/diff/1/ly/context-mods-init.ly
File ly/context-mods-init.ly (right):

http://codereview.appspot.com/5686046/diff/1/ly/context-mods-init.ly#newcode49
ly/context-mods-init.ly:49: to @code{'Voice}.
On 2012/02/23 00:53:58, dak wrote:
> On 2012/02/23 00:13:34, janek wrote:
> > I don't get what these Bottoms are.
> 
> \set whatever = value is the same as \set Bottom.whatever = value.  Bottom is
an
> alias for Voice most of the time, but in some contexts (like TabStaff and
below)
> also for TabVoice, or for Lyrics or some other things.
> 
> How should I write this instead?

maybe
"if you specify optional @var{bottom} argument, all instructions that apply to
bottom-level contexts (for example Voice) will be applied to the context you
specified.  For example, you can tell layout-from to apply \override #'font-size
= #2 to TabVoice or Lyrics context instead of default Voice context."
(if i understood it correctly)

http://codereview.appspot.com/5686046/diff/1/ly/context-mods-init.ly#newcode55
ly/context-mods-init.ly:55: ;; Scheme.
On 2012/02/23 00:53:58, dak wrote:
> The parser turns all sets, overrides etc into something wrapped in
> ContextSpeccedMusic.  If you ever get a set, override etc that is not wrapped
in
> ContextSpeccedMusic, the user has created it in Scheme himself.  In that case,
> the code will try to use #f as a context modification resulting in an error. 
> It's a situation that can only be triggered when you are messing in
experienced
> areas, so I don't bother providing a different behavior.
> 
> One could let "mods" be an empty context modification instead: in that case,
the
> bad commands outside of ContextSpeccedMusic would just silently get eaten.  I
> prefer an explicit error, even if it is a Scheme execution error.  It is close
> enough to the cause to be informative.
> 
> Proposal for a comment expressing that better?

Something like
"All standard layout instruction events occur inside ContextSpeccedMusic.  We
initialize the context with #f and overwrite it using ContextSpeccedMusic - this
will fail and produce an error in case of special instructions written by user
in Scheme."
perhaps.
Or you could simply use the first paragraph of your explanation (The parser
turns...).  It's nice & clear to me.
Sign in to reply to this message.

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