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

Issue 319160043: Let \alterBroken tweak ties again

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

Description

Let \alterBroken tweak ties again When tweaking Tie grobs, the music function \alterBroken has incorrectly rejected them since 2.17.6 as not being spanners. The method of spanner recognition is at fault: 'span-direction is not used by TieEvent.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use eqv? #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -1 line) Patch
M ly/music-functions-init.ly View 1 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 5
david.nalesnik
Please review. Thanks!
2 years, 7 months ago (2017-01-23 18:17:54 UTC) #1
dak
https://codereview.appspot.com/319160043/diff/1/ly/music-functions-init.ly File ly/music-functions-init.ly (right): https://codereview.appspot.com/319160043/diff/1/ly/music-functions-init.ly#newcode107 ly/music-functions-init.ly:107: (if (or (eq? (ly:music-property item 'span-direction) START) This may ...
2 years, 7 months ago (2017-01-23 18:24:29 UTC) #2
david.nalesnik
Use eqv?
2 years, 7 months ago (2017-01-23 18:59:10 UTC) #3
david.nalesnik
No problem. https://codereview.appspot.com/319160043/diff/1/ly/music-functions-init.ly File ly/music-functions-init.ly (right): https://codereview.appspot.com/319160043/diff/1/ly/music-functions-init.ly#newcode107 ly/music-functions-init.ly:107: (if (or (eq? (ly:music-property item 'span-direction) START) ...
2 years, 7 months ago (2017-01-23 19:00:43 UTC) #4
david.nalesnik
2 years, 7 months ago (2017-01-23 22:49:39 UTC) #5
On 2017/01/23 19:00:43, david.nalesnik wrote:
> No problem.
> 
> https://codereview.appspot.com/319160043/diff/1/ly/music-functions-init.ly
> File ly/music-functions-init.ly (right):
> 
>
https://codereview.appspot.com/319160043/diff/1/ly/music-functions-init.ly#ne...
> ly/music-functions-init.ly:107: (if (or (eq? (ly:music-property item
> 'span-direction) START)
> On 2017/01/23 18:24:29, dak wrote:
> > This may be stupid, but as long as we are touching the code, we might as
well
> > make it correct.  Numbers may not reliably be compared with eq? but require
> use
> > of eqv? instead.
> 
> Done.

I get 18 instances with the following (misses variables storing numbers, of
course):

git grep
"(eq?\s\+\((.\+)\|[A-Za-z-]\+\)\s\+\(-\?[[:digit:]]\+\|X\|Y\|LEFT\|RIGHT\|UP\|DOWN\))"

Probably worth a patch.
Sign in to reply to this message.

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