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

Issue 6109046: Macro for(UP_and_DOWN) and 3 similar. (issue 2491) (Closed)

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

Description

Macro for(UP_and_DOWN) and 3 similar. Replaces do{ ... } while(flip (&d) != DOWN/UP/... with a macro for(DOWN_and_UP/UP_and_DOWN/....) { } http://code.google.com/p/lilypond/issues/detail?id=2491

Patch Set 1 #

Total comments: 9

Patch Set 2 : Corrected a bug that could be a cause of issue 2493 (needs check whether it indeed corrected that). #

Patch Set 3 : Removed the last occurrence of flip(&d) and flip() function. #

Total comments: 6

Patch Set 4 : Removed corrections of the bug: loop that executes only once. No changes in regression tests :) #

Patch Set 5 : Corrected GNU style in for (*_and_* (d)) macros. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+161 lines, -388 lines) Patch
M flower/include/direction.hh View 1 2 3 2 chunks +11 lines, -1 line 0 comments Download
M lily/ambitus-engraver.cc View 1 2 3 4 4 chunks +3 lines, -9 lines 0 comments Download
M lily/axis-group-interface.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M lily/beam.cc View 1 2 3 4 13 chunks +9 lines, -26 lines 0 comments Download
M lily/beam-quanting.cc View 1 2 3 4 14 chunks +11 lines, -33 lines 0 comments Download
M lily/bezier.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M lily/break-substitution.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M lily/dynamic-align-engraver.cc View 1 2 3 4 2 chunks +2 lines, -3 lines 0 comments Download
M lily/figured-bass-continuation.cc View 1 2 3 4 2 chunks +2 lines, -3 lines 0 comments Download
M lily/hairpin.cc View 1 2 3 4 5 chunks +4 lines, -11 lines 0 comments Download
M lily/horizontal-bracket.cc View 1 2 3 4 3 chunks +2 lines, -6 lines 0 comments Download
M lily/interval-minefield.cc View 1 2 3 4 2 chunks +2 lines, -4 lines 0 comments Download
M lily/item.cc View 1 2 3 4 3 chunks +5 lines, -12 lines 0 comments Download
M lily/ledger-line-spanner.cc View 1 2 3 4 3 chunks +2 lines, -6 lines 0 comments Download
M lily/line-spanner.cc View 1 2 3 4 6 chunks +5 lines, -11 lines 0 comments Download
M lily/lookup.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/lyric-hyphen.cc View 1 2 3 4 3 chunks +2 lines, -6 lines 0 comments Download
M lily/multi-measure-rest.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/multi-measure-rest-engraver.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/new-fingering-engraver.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/note-collision.cc View 1 2 3 4 11 chunks +8 lines, -21 lines 0 comments Download
M lily/note-spacing.cc View 1 2 3 4 3 chunks +1 line, -4 lines 0 comments Download
M lily/optimal-page-breaking.cc View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M lily/ottava-bracket.cc View 1 2 3 4 5 chunks +3 lines, -7 lines 0 comments Download
M lily/ottava-engraver.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/paper-column.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M lily/piano-pedal-bracket.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/rest-collision.cc View 1 2 3 4 4 chunks +4 lines, -8 lines 0 comments Download
M lily/rod.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M lily/script-column.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/slur.cc View 1 2 3 4 6 chunks +5 lines, -15 lines 0 comments Download
M lily/slur-configuration.cc View 1 2 3 4 5 chunks +5 lines, -9 lines 0 comments Download
M lily/slur-scoring.cc View 1 2 3 4 18 chunks +14 lines, -34 lines 0 comments Download
M lily/spacing-determine-loose-columns.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/spacing-interface.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/spacing-spanner.cc View 1 2 2 chunks +4 lines, -3 lines 0 comments Download
M lily/spanner.cc View 1 2 3 4 6 chunks +8 lines, -20 lines 0 comments Download
M lily/staff-symbol.cc View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M lily/stem.cc View 1 2 3 4 4 chunks +3 lines, -9 lines 0 comments Download
M lily/system.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M lily/system-start-delimiter.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M lily/tie.cc View 1 2 3 4 3 chunks +2 lines, -6 lines 0 comments Download
M lily/tie-column.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M lily/tie-engraver.cc View 1 2 3 4 1 chunk +1 line, -3 lines 0 comments Download
M lily/tie-formatting-problem.cc View 1 2 3 4 15 chunks +10 lines, -26 lines 0 comments Download
M lily/tuplet-bracket.cc View 1 2 3 4 16 chunks +14 lines, -36 lines 0 comments Download

Messages

Total messages: 14
Keith
Looks good to me, but I suggest you also solve the bug you found (issue ...
7 years, 2 months ago (2012-04-24 03:32:28 UTC) #1
Graham Percival
LGTM http://codereview.appspot.com/6109046/diff/1/flower/include/direction.hh File flower/include/direction.hh (right): http://codereview.appspot.com/6109046/diff/1/flower/include/direction.hh#newcode78 flower/include/direction.hh:78: #define DOWN_and_UP(d) \ I see that our code ...
7 years, 2 months ago (2012-04-24 04:00:02 UTC) #2
Keith
http://codereview.appspot.com/6109046/diff/1/lily/ledger-line-spanner.cc File lily/ledger-line-spanner.cc (right): http://codereview.appspot.com/6109046/diff/1/lily/ledger-line-spanner.cc#newcode52 lily/ledger-line-spanner.cc:52: + current_extents[d].length (); On 2012/04/24 03:32:28, Keith wrote: > ...
7 years, 2 months ago (2012-04-24 04:10:36 UTC) #3
Milimetr88
http://codereview.appspot.com/6109046/diff/1/flower/include/direction.hh File flower/include/direction.hh (right): http://codereview.appspot.com/6109046/diff/1/flower/include/direction.hh#newcode63 flower/include/direction.hh:63: // huh? On 2012/04/24 03:32:28, Keith wrote: > If ...
7 years, 2 months ago (2012-04-24 16:21:17 UTC) #4
dak
It would appear that you ignored <http://permalink.gmane.org/gmane.comp.gnu.lilypond.auto/465>, <http://permalink.gmane.org/gmane.comp.gnu.lilypond.auto/466>, the end of <http://permalink.gmane.org/gmane.comp.gnu.lilypond.devel/46312> and <http://permalink.gmane.org/gmane.comp.gnu.lilypond.auto/442>. Code ...
7 years, 2 months ago (2012-04-25 09:57:48 UTC) #5
Milimetr88
On 25 April 2012 11:57, <dak@gnu.org> wrote: > It would appear that you ignored > ...
7 years, 2 months ago (2012-04-25 11:44:35 UTC) #6
Milimetr88
http://codereview.appspot.com/6109046/diff/9001/lily/ledger-line-spanner.cc File lily/ledger-line-spanner.cc (left): http://codereview.appspot.com/6109046/diff/9001/lily/ledger-line-spanner.cc#oldcode68 lily/ledger-line-spanner.cc:68: while (flip (&d) != DOWN); On 2012/04/25 09:57:48, dak ...
7 years, 2 months ago (2012-04-25 11:47:07 UTC) #7
Milimetr88
http://codereview.appspot.com/6109046/diff/9001/lily/ledger-line-spanner.cc File lily/ledger-line-spanner.cc (left): http://codereview.appspot.com/6109046/diff/9001/lily/ledger-line-spanner.cc#oldcode68 lily/ledger-line-spanner.cc:68: while (flip (&d) != DOWN); On 2012/04/25 11:47:07, Milimetr88 ...
7 years, 2 months ago (2012-04-25 11:49:02 UTC) #8
Milimetr88
> > I know that this is not a very positive message, but I am ...
7 years, 2 months ago (2012-04-25 13:14:54 UTC) #9
Keith
http://codereview.appspot.com/6109046/diff/9001/lily/slur.cc File lily/slur.cc (right): http://codereview.appspot.com/6109046/diff/9001/lily/slur.cc#newcode105 lily/slur.cc:105: ret.add_point (d[dir]); Here is the history: http://codereview.appspot.com/5431065/diff/8002/lily/slur.cc#newcode101
7 years, 2 months ago (2012-04-25 17:23:35 UTC) #10
Milimetr88
http://codereview.appspot.com/6109046/diff/9001/lily/slur.cc File lily/slur.cc (right): http://codereview.appspot.com/6109046/diff/9001/lily/slur.cc#newcode105 lily/slur.cc:105: ret.add_point (d[dir]); On 2012/04/25 17:23:35, Keith wrote: > Here ...
7 years, 2 months ago (2012-04-25 17:54:07 UTC) #11
Trevor Daniels
Almost there, if the regtests run clean, but the added 'for' loops are still not ...
7 years, 2 months ago (2012-04-25 21:46:29 UTC) #12
mike_apollinemike.com
On 26 avr. 2012, at 07:28, Graham Percival wrote: > > Well, right now we ...
7 years, 2 months ago (2012-04-26 06:55:27 UTC) #13
Graham Percival
7 years, 1 month ago (2012-04-27 07:51:32 UTC) #14
LGTM
Sign in to reply to this message.

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