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

Issue 5293053: Sketch for in-notes. (Closed)

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

Description

Sketch for in-notes.

Patch Set 1 #

Patch Set 2 : Adds property definitions. #

Patch Set 3 : Fixes memory management problem (issue 1980). #

Patch Set 4 : First version that passes regtests. #

Total comments: 33

Patch Set 5 : Changes from Bertrand's comments. #

Patch Set 6 : Incorporates changes, clean regtest. #

Total comments: 9

Patch Set 7 : Updates after Neil's comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+245 lines, -80 lines) Patch
M input/regression/footnote-break-visibility.ly View 1 2 3 4 5 6 1 chunk +4 lines, -3 lines 0 comments Download
A input/regression/in-note.ly View 1 2 3 4 5 6 1 chunk +20 lines, -0 lines 0 comments Download
M lily/align-interface.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M lily/constrained-breaking.cc View 1 2 3 4 5 6 2 chunks +11 lines, -2 lines 0 comments Download
M lily/include/constrained-breaking.hh View 1 2 1 chunk +4 lines, -1 line 0 comments Download
M lily/include/page-breaking.hh View 1 2 3 3 chunks +4 lines, -2 lines 0 comments Download
M lily/include/page-layout-problem.hh View 1 2 3 4 3 chunks +5 lines, -2 lines 0 comments Download
M lily/include/system.hh View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M lily/page-breaking.cc View 1 2 3 4 5 6 8 chunks +24 lines, -17 lines 0 comments Download
M lily/page-layout-problem.cc View 1 2 3 4 5 6 12 chunks +96 lines, -35 lines 0 comments Download
M lily/page-spacing.cc View 1 2 3 4 2 chunks +19 lines, -4 lines 0 comments Download
M lily/system.cc View 1 2 3 4 5 5 chunks +32 lines, -10 lines 0 comments Download
M scm/define-grob-interfaces.scm View 1 chunk +1 line, -1 line 0 comments Download
M scm/define-grob-properties.scm View 1 2 3 4 5 6 3 chunks +5 lines, -0 lines 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download
M scm/paper-system.scm View 1 2 3 4 5 6 1 chunk +14 lines, -1 line 0 comments Download

Messages

Total messages: 18
MikeSol
Hey all, As part of project markup & at the suggestion of Nicolas Sceaux by ...
7 years, 8 months ago (2011-10-20 13:11:05 UTC) #1
MikeSol
The memory management issue is now fixed. A new patch will be coming down the ...
7 years, 8 months ago (2011-10-20 13:55:36 UTC) #2
MikeSol
Hey all, The patch is now ready to be tested - I got a clean ...
7 years, 8 months ago (2011-10-20 16:43:54 UTC) #3
Bertrand Bordage
I can't compile because of valgrind. Bertrand http://codereview.appspot.com/5293053/diff/12001/lily/include/page-layout-problem.hh File lily/include/page-layout-problem.hh (right): http://codereview.appspot.com/5293053/diff/12001/lily/include/page-layout-problem.hh#newcode100 lily/include/page-layout-problem.hh:100: Direction in_note_direction_; ...
7 years, 8 months ago (2011-10-21 11:52:25 UTC) #4
MikeSol
I'm not sure how to deal with some of the "d├ęBordage" of line lengths - ...
7 years, 8 months ago (2011-10-21 12:21:57 UTC) #5
Bertrand Bordage
http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode79 lily/page-layout-problem.cc:79: footnotes_added = g->get_property ("footnote-stencil") != SCM_EOL; Of course not ...
7 years, 8 months ago (2011-10-21 20:07:37 UTC) #6
Bertrand Bordage
A small comment to show you a possible solution for the indentation. This can be ...
7 years, 8 months ago (2011-10-21 20:15:47 UTC) #7
Graham Percival (old account)
http://codereview.appspot.com/5293053/diff/12001/lily/page-breaking.cc File lily/page-breaking.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-breaking.cc#newcode189 lily/page-breaking.cc:189: old.in_note_heights_.begin (), old.in_note_heights_.end ()); Why are we talking about ...
7 years, 8 months ago (2011-10-21 23:08:16 UTC) #8
Neil Puttock
http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode79 lily/page-layout-problem.cc:79: footnotes_added = g->get_property ("footnote-stencil") != SCM_EOL; On 2011/10/21 20:07:37, ...
7 years, 8 months ago (2011-10-21 23:19:27 UTC) #9
dak
http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc File lily/page-layout-problem.cc (right): http://codereview.appspot.com/5293053/diff/12001/lily/page-layout-problem.cc#newcode79 lily/page-layout-problem.cc:79: footnotes_added = g->get_property ("footnote-stencil") != SCM_EOL; On 2011/10/21 20:07:37, ...
7 years, 8 months ago (2011-10-21 23:40:24 UTC) #10
Bertrand Bordage
Sorry for this total mistake. !ly_is_equal (..., SCM_EOL) should do it, then.
7 years, 8 months ago (2011-10-21 23:49:26 UTC) #11
dak
On 2011/10/21 23:49:26, Bertrand Bordage wrote: > Sorry for this total mistake. !ly_is_equal (..., SCM_EOL) ...
7 years, 8 months ago (2011-10-22 00:36:25 UTC) #12
mike_apollinemike.com
On Oct 22, 2011, at 1:08 AM, percival.music.ca@gmail.com wrote: > > http://codereview.appspot.com/5293053/diff/12001/lily/page-breaking.cc > File lily/page-breaking.cc ...
7 years, 8 months ago (2011-10-22 19:56:26 UTC) #13
Graham Percival
On Sat, Oct 22, 2011 at 09:56:16PM +0200, mike@apollinemike.com wrote: > > Running fix-cc.py, I ...
7 years, 8 months ago (2011-10-22 20:36:38 UTC) #14
Neil Puttock
http://codereview.appspot.com/5293053/diff/11005/input/regression/footnote-break-visibility.ly File input/regression/footnote-break-visibility.ly (left): http://codereview.appspot.com/5293053/diff/11005/input/regression/footnote-break-visibility.ly#oldcode1 input/regression/footnote-break-visibility.ly:1: \version "2.14.0" 2.15.17 http://codereview.appspot.com/5293053/diff/11005/input/regression/in-note.ly File input/regression/in-note.ly (right): http://codereview.appspot.com/5293053/diff/11005/input/regression/in-note.ly#newcode1 input/regression/in-note.ly:1: ...
7 years, 8 months ago (2011-10-28 13:24:12 UTC) #15
mike_apollinemike.com
On Oct 28, 2011, at 3:24 PM, n.puttock@gmail.com wrote: > > http://codereview.appspot.com/5293053/diff/11005/input/regression/footnote-break-visibility.ly > File input/regression/footnote-break-visibility.ly ...
7 years, 8 months ago (2011-10-28 13:26:22 UTC) #16
dak
On 2011/10/28 13:26:22, mike_apollinemike.com wrote: > On Oct 28, 2011, at 3:24 PM, mailto:n.puttock@gmail.com wrote: ...
7 years, 8 months ago (2011-10-29 07:00:21 UTC) #17
mike_apollinemike.com
7 years, 8 months ago (2011-10-29 13:29:46 UTC) #18
On Oct 29, 2011, at 9:00 AM, dak@gnu.org wrote:

>> 
> 
> Patch has been backed out.  It broke the documentation build this
> morning.  And it broke the documentation build with a line that Neil has
> already pointed out yesterday at noon.  It is not that this particular
> mistake was unheard of.
> 
> Since this kept the second in unfriendliness from accepting a patch
> series from the most unfriendly developer, consider the remarks to be
> expected as having been made.
> 
> I have not checked in this particular instance, but in my experience
> "make doc-clean info" detects most documentation building failures while
> appearing somewhat faster than a web build.
> 
> http://codereview.appspot.com/5293053/

New patch set uploaded.

Cheers,
MS
Sign in to reply to this message.

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