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

Issue 314610043: Create Bracket class

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years ago by david.nalesnik
Modified:
7 years ago
Reviewers:
lemzwerg
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Create Bracket class Code involving brackets suffers from two confusing organizational issues: (1) Tuplet_bracket::make_bracket is used to create brackets for a number of grobs: BassFigureBracket, HorizontalBracket, OttavaBracket, PianoPedalBracket, VoltaBracket, along with TupletBracket (2) Methods belonging to Horizontal_bracket are used to draw both horizonal brackets (HorizontalBracket) and vertical brackets (BassFigureBracket) To remedy this, a new Bracket class is created. This new class contains the old Tuplet_bracket::make_bracket, Horizontal_bracket::make_bracket, and Horizontal_bracket::make_enclosing_bracket. These methods have been renamed to clarify their purpose.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+199 lines, -178 lines) Patch
A lily/bracket.cc View 1 chunk +151 lines, -0 lines 0 comments Download
M lily/enclosing-bracket.cc View 3 chunks +9 lines, -7 lines 0 comments Download
M lily/horizontal-bracket.cc View 2 chunks +4 lines, -69 lines 0 comments Download
A + lily/include/bracket.hh View 2 chunks +12 lines, -8 lines 0 comments Download
M lily/include/horizontal-bracket.hh View 1 chunk +0 lines, -5 lines 0 comments Download
M lily/include/tuplet-bracket.hh View 1 chunk +0 lines, -4 lines 0 comments Download
M lily/ottava-bracket.cc View 2 chunks +5 lines, -6 lines 0 comments Download
M lily/piano-pedal-bracket.cc View 2 chunks +4 lines, -6 lines 0 comments Download
M lily/tuplet-bracket.cc View 3 chunks +11 lines, -67 lines 0 comments Download
M lily/volta-bracket.cc View 2 chunks +3 lines, -6 lines 0 comments Download

Messages

Total messages: 2
david.nalesnik
Please review. Thanks!
7 years ago (2017-03-06 16:52:40 UTC) #1
lemzwerg
7 years ago (2017-03-06 16:57:49 UTC) #2
From an organizational point of view: LGTM.
Sign in to reply to this message.

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