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

Issue 7799048: Fix composition of markup lists containing markup command list calls (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years ago by dak
Modified:
11 years, 11 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Fix composition of markup lists containing markup command list calls Composed markup lists in the parser apply normal markup commands to markup lists, like \with-color #red { a b c } If those markup lists contained calls to markup list commands (like \column-lines), the resulting behavior was undefined, at worst producing segfaults (cf issue 2393). This patch series cleans up the inconsistencies. It contains the following commits: Allow unbraced forms of composed markup lists Since composing markup lists may now work at run time, there is no particular need for the associated syntactic restrictions. Allow markup lists to be composed at run-time Using a markup command on a markup list generated by a markup list command was not previously possible. Now things like \with-color #red { \column-lines { x y z } } work as well. Move map-markup-command-list into parser internals Simplify interpret-markup-list Use fold in map-markup-command-list Text_interface::is_markup should reject markup-list-command calls

Patch Set 1 #

Patch Set 2 : Combine successive composed markup list commands #

Total comments: 1

Patch Set 3 : Address Ian's comment. Hopefully. #

Patch Set 4 : Some comments, make robust about throw not actually happening #

Unified diffs Side-by-side diffs Delta from patch set Stats (+100 lines, -45 lines) Patch
M lily/parser.yy View 1 2 4 chunks +16 lines, -9 lines 0 comments Download
M lily/text-interface.cc View 2 chunks +9 lines, -8 lines 0 comments Download
M scm/define-markup-commands.scm View 1 2 3 1 chunk +37 lines, -0 lines 0 comments Download
M scm/ly-syntax-constructors.scm View 1 2 chunks +28 lines, -1 line 0 comments Download
M scm/markup.scm View 1 chunk +10 lines, -9 lines 0 comments Download
M scm/markup-macros.scm View 1 chunk +0 lines, -18 lines 0 comments Download

Messages

Total messages: 8
lemzwerg
LGTM.
12 years ago (2013-03-24 05:55:00 UTC) #1
dak
Combine successive composed markup list commands
12 years ago (2013-03-24 12:08:01 UTC) #2
dak
I just changed the summary of the commit messages since one commit in the middle ...
12 years ago (2013-03-24 14:33:22 UTC) #3
janek
judging mostly from description, LGTM.
12 years ago (2013-03-24 21:50:52 UTC) #4
Ian Hulin (gmail)
LGTM, apart from comment on the doc-string. Cheers, Ian https://codereview.appspot.com/7799048/diff/3001/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/7799048/diff/3001/scm/define-markup-commands.scm#newcode4286 scm/define-markup-commands.scm:4286: ...
12 years ago (2013-03-25 11:37:59 UTC) #5
dak
Address Ian's comment. Hopefully.
12 years ago (2013-03-25 21:07:45 UTC) #6
Ian Hulin (gmail)
On 2013/03/25 21:07:45, dak wrote: > Address Ian's comment. Hopefully. LGTM Cheers, Ian
12 years ago (2013-03-27 15:00:17 UTC) #7
dak
12 years ago (2013-03-27 21:08:40 UTC) #8
Some comments, make robust about throw not actually happening
Sign in to reply to this message.

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