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

Issue 571190043: Issue 5627: clean up the breaking of breakable Items (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by Dan Eble
Modified:
4 years, 3 months ago
Reviewers:
lemzwerg
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

https://sourceforge.net/p/testlilyissues/issues/5627/ 1: remove pointless Paper_column::do_break_processing It did nothing but call the inherited method. This isn't related to the remaining changes; I just noticed it while I was working. 2: clean up the breaking of breakable Items * rename Grob::discretionary_processing () to the more descriptive break_breakable_item () * avoid calling break_breakable_item () on the clones that it creates * provide the calling System to each called Item to avoid having to find it by walking the chain of parents * remove an unnecessary dynamic_cast

Patch Set 1 #

Total comments: 2

Patch Set 2 : remove braces #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -33 lines) Patch
M lily/grob.cc View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/grob.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/item.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/paper-column.hh View 1 chunk +0 lines, -1 line 0 comments Download
M lily/item.cc View 2 chunks +22 lines, -21 lines 0 comments Download
M lily/paper-column.cc View 1 chunk +0 lines, -6 lines 0 comments Download
M lily/system.cc View 1 1 chunk +8 lines, -2 lines 0 comments Download

Messages

Total messages: 4
Dan Eble
4 years, 3 months ago (2019-12-03 23:31:20 UTC) #1
lemzwerg
LGTM, thanks! https://codereview.appspot.com/571190043/diff/559290043/lily/system.cc File lily/system.cc (right): https://codereview.appspot.com/571190043/diff/559290043/lily/system.cc#newcode525 lily/system.cc:525: { Is there a reason for grouping ...
4 years, 3 months ago (2019-12-04 08:08:10 UTC) #2
Dan Eble
https://codereview.appspot.com/571190043/diff/559290043/lily/system.cc File lily/system.cc (right): https://codereview.appspot.com/571190043/diff/559290043/lily/system.cc#newcode525 lily/system.cc:525: { On 2019/12/04 08:08:09, lemzwerg wrote: > Is there ...
4 years, 3 months ago (2019-12-04 15:02:58 UTC) #3
Dan Eble
4 years, 3 months ago (2019-12-04 15:06:24 UTC) #4
remove braces
Sign in to reply to this message.

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