lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: input/regression/scheme-text-spanner.ly: fix problem with constants


From: david . nalesnik
Subject: Re: input/regression/scheme-text-spanner.ly: fix problem with constants (issue 11614044)
Date: Tue, 23 Jul 2013 13:05:47 +0000

Thank you, David.


https://codereview.appspot.com/11614044/diff/1/input/regression/scheme-text-spanner.ly
File input/regression/scheme-text-spanner.ly (right):

https://codereview.appspot.com/11614044/diff/1/input/regression/scheme-text-spanner.ly#newcode28
input/regression/scheme-text-spanner.ly:28: (set! meta-entry (assoc-set!
meta-entry 'name grob-name))
On 2013/07/23 12:31:25, dak wrote:
Of course, these lines are also modifying constants.

Yes, but this routine is cribbed from completize-grob-entry, which
suffers from the same issues as you pointed out.  Shouldn't this be a
focus of another issue which addresses major problems in
define-grobs.scm?  My intention here was simply to fix a minor glitch.

https://codereview.appspot.com/11614044/diff/1/input/regression/scheme-text-spanner.ly#newcode81
input/regression/scheme-text-spanner.ly:81: (set! lst (assoc-set! lst
'name (car x)))
On 2013/07/23 12:31:25, dak wrote:
And these modify constants as well.

Yes; again, shouldn't this be the focus of another issue?

https://codereview.appspot.com/11614044/diff/1/input/regression/scheme-text-spanner.ly#newcode117
input/regression/scheme-text-spanner.ly:117: (event-drul (cons '()
'())))
On 2013/07/23 12:31:25, dak wrote:
Frankly, just throw out the crap event-drul and current-event, and
instead use
event-start and event-stop instead of (car event-drul) and (cdr
event-drul).
current-event also is never set to anything except (car event-drul),
so you can
just replace it with event-start.

This whole file is a huge parade of bad code.

OK, will do.

https://codereview.appspot.com/11614044/diff/1/input/regression/scheme-text-spanner.ly#newcode157
input/regression/scheme-text-spanner.ly:157: (set! event-drul (cons '()
'())))))
On 2013/07/23 12:31:25, dak wrote:
You could do (set-car! event-drul '())
              (set-cdr! event-drul '())
instead in order to not create a new cons cell.

OK, will do.

https://codereview.appspot.com/11614044/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]