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

Issue 4894052: Creates pure closures (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 10 months ago by MikeSol
Modified:
7 years, 9 months ago
Reviewers:
pkx166h, mikesol, Neil Puttock, reinhold
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Creates pure closures

Patch Set 1 #

Total comments: 3

Patch Set 2 : Various small changes to make this work better. #

Patch Set 3 : Some more changes. #

Total comments: 7

Patch Set 4 : Incorporates Neil's comments. #

Total comments: 2

Patch Set 5 : Incorporates Neil's comments. #

Patch Set 6 : Incorporates Neil's suggestions. #

Total comments: 2

Patch Set 7 : Version to be pushed to master. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+213 lines, -19 lines) Patch
A input/regression/unpure-pure-container.ly View 1 2 3 4 5 6 1 chunk +25 lines, -0 lines 0 comments Download
M lily/axis-group-interface.cc View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M lily/context-property.cc View 1 2 3 2 chunks +10 lines, -4 lines 0 comments Download
M lily/function-documentation.cc View 1 2 3 4 5 6 2 chunks +2 lines, -0 lines 0 comments Download
M lily/grob-property.cc View 1 2 3 6 chunks +8 lines, -3 lines 0 comments Download
A lily/include/unpure-pure-container.hh View 1 2 3 1 chunk +30 lines, -0 lines 0 comments Download
M lily/system.cc View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
A lily/unpure-pure-container.cc View 1 2 3 4 1 chunk +110 lines, -0 lines 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 5 6 2 chunks +26 lines, -12 lines 0 comments Download

Messages

Total messages: 19
MikeSol
This is a more extensible way to deal with pure properties. I'd like this patch ...
7 years, 10 months ago (2011-08-18 09:21:59 UTC) #1
reinhold_kainhofer.com
Am Thursday, 18. August 2011, 11:21:59 schrieb mtsolo@gmail.com: > This is a more extensible way ...
7 years, 10 months ago (2011-08-18 11:06:57 UTC) #2
Neil Puttock
On 2011/08/18 11:06:57, reinhold_kainhofer.com wrote: > Wow, that would be VERY user-unfriendly. Just imagine explaining ...
7 years, 10 months ago (2011-08-18 12:05:46 UTC) #3
Neil Puttock
Hi Mike, I have reservations about the naming, since you're basically creating a smob which ...
7 years, 10 months ago (2011-08-18 12:31:27 UTC) #4
mikesol_ufl.edu
On Aug 18, 2011, at 1:06 PM, Reinhold Kainhofer wrote: > Am Thursday, 18. August ...
7 years, 10 months ago (2011-08-18 12:31:59 UTC) #5
mikesol_ufl.edu
On Aug 18, 2011, at 2:31 PM, n.puttock@gmail.com wrote: > Hi Mike, > > I ...
7 years, 10 months ago (2011-08-18 12:44:49 UTC) #6
Neil Puttock
http://codereview.appspot.com/4894052/diff/9001/input/regression/pure-closure.ly File input/regression/pure-closure.ly (right): http://codereview.appspot.com/4894052/diff/9001/input/regression/pure-closure.ly#newcode18 input/regression/pure-closure.ly:18: #(ly:make-pure-closure ly:stem::height '(-3 . 10)) I'm a bit worried ...
7 years, 10 months ago (2011-08-19 22:39:50 UTC) #7
Neil Puttock
On 18 August 2011 13:44, Mike Solomon <mikesol@ufl.edu> wrote: > What about pure-container ? It's ...
7 years, 10 months ago (2011-08-19 22:42:24 UTC) #8
mikesol_ufl.edu
On Aug 20, 2011, at 12:02 AM, Neil Puttock wrote: > On 18 August 2011 ...
7 years, 10 months ago (2011-08-20 08:30:02 UTC) #9
mikesol_ufl.edu
On Aug 20, 2011, at 12:39 AM, n.puttock@gmail.com wrote: > > http://codereview.appspot.com/4894052/diff/9001/input/regression/pure-closure.ly > File input/regression/pure-closure.ly ...
7 years, 10 months ago (2011-08-20 20:21:30 UTC) #10
pkx166h
passes make and reg tests
7 years, 10 months ago (2011-08-27 08:45:24 UTC) #11
Neil Puttock
On 2011/08/20 20:21:30, mikesol_ufl.edu wrote: > I'll try to think of something better...if you have ...
7 years, 9 months ago (2011-08-31 20:37:31 UTC) #12
Neil Puttock
http://codereview.appspot.com/4894052/diff/19001/lily/system.cc File lily/system.cc (right): http://codereview.appspot.com/4894052/diff/19001/lily/system.cc#newcode773 lily/system.cc:773: || is_unpure_pure_container (elts[i]->get_property_data ("Y-extent")))) move this to pure-relevant? http://codereview.appspot.com/4894052/diff/19001/lily/unpure-pure-container.cc ...
7 years, 9 months ago (2011-08-31 20:41:02 UTC) #13
mikesol_ufl.edu
On Aug 31, 2011, at 10:37 PM, n.puttock@gmail.com wrote: > On 2011/08/20 20:21:30, mikesol_ufl.edu wrote: ...
7 years, 9 months ago (2011-09-01 07:23:03 UTC) #14
mikesol_ufl.edu
On Aug 31, 2011, at 10:41 PM, n.puttock@gmail.com wrote: > > http://codereview.appspot.com/4894052/diff/19001/lily/system.cc > File lily/system.cc ...
7 years, 9 months ago (2011-09-01 07:27:43 UTC) #15
Neil Puttock
http://codereview.appspot.com/4894052/diff/29001/lily/unpure-pure-container.cc File lily/unpure-pure-container.cc (right): http://codereview.appspot.com/4894052/diff/29001/lily/unpure-pure-container.cc#newcode35 lily/unpure-pure-container.cc:35: LY_ASSERT_TYPE (is_unpure_pure_container, smob, 1); You sure this works? It ...
7 years, 9 months ago (2011-09-01 08:39:24 UTC) #16
mikesol_ufl.edu
On Sep 1, 2011, at 10:39 AM, n.puttock@gmail.com wrote: > > http://codereview.appspot.com/4894052/diff/29001/lily/unpure-pure-container.cc > File lily/unpure-pure-container.cc ...
7 years, 9 months ago (2011-09-01 08:40:19 UTC) #17
MikeSol
http://codereview.appspot.com/4894052/diff/29001/lily/unpure-pure-container.cc File lily/unpure-pure-container.cc (right): http://codereview.appspot.com/4894052/diff/29001/lily/unpure-pure-container.cc#newcode35 lily/unpure-pure-container.cc:35: LY_ASSERT_TYPE (is_unpure_pure_container, smob, 1); On 2011/09/01 08:39:24, Neil Puttock ...
7 years, 9 months ago (2011-09-01 09:47:51 UTC) #18
MikeSol
7 years, 9 months ago (2011-09-01 13:05:35 UTC) #19
Pushed as 69622b49b7a5a9c992e36ef11ba60c1fdd3c34b6.
I made the regtest more unique to unpure-pure-closures so that people reading it
don't mistake it for the same thing as extra-spacing-height.

Have fun with these!

Cheers,
MS
Sign in to reply to this message.

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