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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by dak
Modified:
11 years 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.
11 years, 1 month ago (2013-03-24 05:55:00 UTC) #1
dak
Combine successive composed markup list commands
11 years, 1 month 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 ...
11 years, 1 month ago (2013-03-24 14:33:22 UTC) #3
janek
judging mostly from description, LGTM.
11 years, 1 month 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: ...
11 years, 1 month ago (2013-03-25 11:37:59 UTC) #5
dak
Address Ian's comment. Hopefully.
11 years, 1 month 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
11 years, 1 month ago (2013-03-27 15:00:17 UTC) #7
dak
11 years, 1 month 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