[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden)
From: |
nine . fierce . ballads |
Subject: |
Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden) |
Date: |
Sat, 16 Nov 2019 10:07:37 -0800 |
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) 1997--2015 Jan
Nieuwenhuizen <address@hidden>
When you add a new file, I think you're supposed to use (C) <this year>
<your name>. At least, that's what I was once told.
https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh#newcode20
lily/include/measure-attached-spanner.hh:20: #ifndef
Measure_attached_spanner_HH
all caps, please
https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh#newcode24
lily/include/measure-attached-spanner.hh:24: #include "std-vector.hh"
I don't see anything in this header that uses a vector. Unless I'm
wrong, this should be moved to whichever cc files require it. Same goes
for any other unnecessary headers.
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#newcode45
lily/measure-attached-spanner.cc:45: //Direction dir =
get_grob_direction (me);
remove
https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc#newcode53
lily/measure-attached-spanner.cc:53: Spanner *orig_spanner =
dynamic_cast<Spanner *> (me->original ());
If I understand correctly, me->original () can only ever be either null
or another instance of the same type as me; therefore, use static_cast
here.
Also, if it's logically possible for me->original () to be null in this
case, check for it before dereferencing below.
https://codereview.appspot.com/571180043/
- Implement MeasureAttachedSpanner (issue 571180043 by address@hidden), lemzwerg, 2019/11/15
- Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden), thomasmorley65, 2019/11/16
- Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden),
nine . fierce . ballads <=
- Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden), thomasmorley65, 2019/11/16
- Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden), checkma, 2019/11/16
- Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden), david . nalesnik, 2019/11/21
- Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden), sphema72, 2019/11/24
- Re: Implement MeasureAttachedSpanner (issue 571180043 by address@hidden), sphema72, 2019/11/24