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

Issue 571180043: Implement MeasureSpanner

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 4 months ago by david.nalesnik
Modified:
4 years, 4 months ago
Reviewers:
carl.d.sorensen, c_sorensen, checkma, lemzwerg, P Soul, Dan Eble, t.daniels, thomasmorley651, kieren
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Implement MeasureSpanner This patch creates a spanner whose ends are oriented to measure boundaries, a frequent request from users. The ends of the spanner may be aligned in various ways to prefatory material. The spanner may hold text or markup in a centered gap. The spanner is begun with the command '\startMeasureSpanner' and ended with '\stopMeasureSpanner'. Provide two regression tests.

Patch Set 1 #

Total comments: 30

Patch Set 2 : Changes in response to reviews #

Unified diffs Side-by-side diffs Delta from patch set Stats (+317 lines, -16 lines) Patch
A input/regression/measure-spanner.ly View 1 chunk +30 lines, -0 lines 0 comments Download
A input/regression/measure-spanner-spacing-pair.ly View 1 chunk +32 lines, -0 lines 0 comments Download
M lily/bracket.cc View 3 chunks +5 lines, -5 lines 0 comments Download
M lily/enclosing-bracket.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M lily/include/bracket.hh View 1 chunk +2 lines, -1 line 0 comments Download
lily/include/measure-spanner.hh View 2 chunks +8 lines, -8 lines 0 comments Download
A lily/measure-spanner.cc View 1 chunk +143 lines, -0 lines 0 comments Download
M ly/spanners-init.ly View 1 chunk +4 lines, -0 lines 0 comments Download
M scm/define-event-classes.scm View 1 chunk +1 line, -0 lines 0 comments Download
M scm/define-grobs.scm View 1 chunk +21 lines, -0 lines 0 comments Download
scm/define-music-types.scm View 1 chunk +6 lines, -0 lines 0 comments Download
M scm/scheme-engravers.scm View 2 chunks +63 lines, -0 lines 0 comments Download

Messages

Total messages: 14
lemzwerg
LGTM from reading the code without testing. Thanks! https://codereview.appspot.com/571180043/diff/565230043/input/regression/measure-spanner.ly File input/regression/measure-spanner.ly (right): https://codereview.appspot.com/571180043/diff/565230043/input/regression/measure-spanner.ly#newcode5 input/regression/measure-spanner.ly:5: Measure ...
4 years, 4 months ago (2019-11-15 17:49:25 UTC) #1
Carl
I have not reviewed the code, but this looks like a worthwhile addition. Thanks for ...
4 years, 4 months ago (2019-11-15 19:21:09 UTC) #2
Dan Eble
On 2019/11/15 19:21:09, Carl wrote: > I think the name should be changed from MeasureAttachedSpanner ...
4 years, 4 months ago (2019-11-16 14:27:25 UTC) #3
thomasmorley651
On 2019/11/16 14:27:25, Dan Eble wrote: > On 2019/11/15 19:21:09, Carl wrote: > > I ...
4 years, 4 months ago (2019-11-16 14:30:43 UTC) #4
thomasmorley651
https://codereview.appspot.com/571180043/diff/565230043/ly/spanners-init.ly File ly/spanners-init.ly (right): https://codereview.appspot.com/571180043/diff/565230043/ly/spanners-init.ly#newcode25 ly/spanners-init.ly:25: "View side-by-side diff with in-line comments" is broken for ...
4 years, 4 months ago (2019-11-16 14:42:02 UTC) #5
t.daniels_treda.co.uk
------ Original Message ------ From: thomasmorley65@gmail.com To: david.nalesnik@gmail.com; lemzwerg@googlemail.com; carl.d.sorensen@gmail.com; nine.fierce.ballads@gmail.com Cc: reply@codereview-hr.appspotmail.com; lilypond-devel@gnu.org Sent: ...
4 years, 4 months ago (2019-11-16 14:47:24 UTC) #6
kieren_kierenmacmillan.info
>> I'd vote for MeasureSpanner. > +1 +1 Kieren.
4 years, 4 months ago (2019-11-16 14:49:46 UTC) #7
Dan Eble
I haven't reviewed the ly or scm. https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh File lily/include/measure-attached-spanner.hh (right): https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh#newcode4 lily/include/measure-attached-spanner.hh:4: Copyright (C) ...
4 years, 4 months ago (2019-11-16 18:07:39 UTC) #8
thomasmorley651
Thanks for working on it !! Some other nits: https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc File lily/measure-attached-spanner.cc (right): https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc#newcode93 lily/measure-attached-spanner.cc:93: ...
4 years, 4 months ago (2019-11-16 18:41:00 UTC) #9
c_sorensen
On 11/16/19, 7:49 AM, "Kieren MacMillan" <kieren@kierenmacmillan.info> wrote: >> I'd vote for MeasureSpanner. > +1 ...
4 years, 4 months ago (2019-11-16 19:55:56 UTC) #10
checkma
Is it nestable? (Can you put one of these spanners inside of another?) --- Christopher ...
4 years, 4 months ago (2019-11-17 00:34:16 UTC) #11
david.nalesnik
Name has been changed to MeasureSpanner, and all references (including file names) have been adjusted. ...
4 years, 4 months ago (2019-11-21 17:32:16 UTC) #12
P Soul
how to wipe my dept & how to put money on y bank account ,pay ...
4 years, 4 months ago (2019-11-24 11:53:56 UTC) #13
P Soul
4 years, 4 months ago (2019-11-24 12:07:32 UTC) #14
Dear Sir/Madam . I want to know something, how to get nulled to someone's
business system and do whatever I like to do.
Sign in to reply to this message.

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