Issue 4811066: Rewrite regtest mozart-hrn-3.ly (Closed)
|
Created:
13 years, 9 months ago by PhilEHolmes Modified:
13 years, 8 months ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionThis regtest had a number of old lines of syntax and produces warnings with the latest version of LilyPond. It also uses tabs for indents. This is a cleaner, warning-free version.
Patch Set 1 #
Total comments: 39
Patch Set 2 : Version 2 of the updated mozart horn regtest #Patch Set 3 : Final changes to mozart horn regtest #
Total comments: 1
MessagesTotal messages: 20
Please review.
Sign in to reply to this message.
LGTM, although I initially misread the commit message as saying that the *new* version uses tabs for indents. Maybe change that to "the old version used tabs", just for extra clarity ?
Sign in to reply to this message.
http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly File input/regression/mozart-hrn-3.ly (right): http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:25: \fill-line { "This music is part of the Mutopia project," newline I suspect there's a missing \line here (where the extra braces were) http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:28: \fill-line { #(ly:export (string-append "It has been typeset and placed in the public " newline http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:29: "domain by " maintainer ".")) this is inside the scheme form, so its indentation should follow the string above: (string-append "It has been typeset and placed in the public " "domain by " maintainer ".")) http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:31: \fill-line { #(ly:export (string-append "Unrestricted modification and redistribution" newline http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:32: " is permitted and encouraged---copy this music" same indentation as above http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:33: " and share it!")) indent http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:53: system-system-spacing #'basic-distance = 10 \mm remove \mm - it's redundant since the new spacing variables aren't scalable (they're in terms of staff-space only) http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:54: score-system-spacing #'basic-distance = 20 \mm remove \mm http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:61: \header { piece = "Allegro" opus = "" } \header { piece = "Allegro" opus = "" } http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:73: \header { piece = "Romanze" opus = "" } \header { piece = "Allegro" opus = "" } http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:82: \layout {} \layout { } http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:88: \header { piece = "Rondo" opus = "" } \header { piece = "Rondo" opus = "" } http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... File input/regression/mozart-hrn3-defs.ily (right): http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:16: \override MultiMeasureRest #'padding = #0.5 this doesn't do anything; it's more likely to be MultiMeasureRestNumber #'padding http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:22: \override Beam #'thickness = #0.6 #'beam-thickness = #0.6 http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:23: \override Beam #'space-function = #(lambda (beam mult) 0.8) #'length-fraction = #0.8 http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:27: \override VerticalAxisGroup #'minimum-Y-extent = #'(-2.5 . 3.5) remove http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:32: indent = 10. \mm 10\mm http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:33: line-width = 189. \mm 189\mm http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-ron... File input/regression/mozart-hrn3-rondo.ily (right): http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-ron... input/regression/mozart-hrn3-rondo.ily:23: rondo = \relative c' { = \relative c'
Sign in to reply to this message.
Again - not too familiar with the codereview tool, so I hope this makes sense. New patch set soon. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly File input/regression/mozart-hrn-3.ly (right): http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:25: \fill-line { "This music is part of the Mutopia project," On 2011/08/02 19:59:39, Neil Puttock wrote: Can't see a missing line. I was trying to recreate the old regtest, but with better syntax, so wasn't adding lines where they didn't exist. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:28: \fill-line { #(ly:export (string-append "It has been typeset and placed in the public " On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:29: "domain by " maintainer ".")) On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:31: \fill-line { #(ly:export (string-append "Unrestricted modification and redistribution" On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:32: " is permitted and encouraged---copy this music" On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:33: " and share it!")) On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:53: system-system-spacing #'basic-distance = 10 \mm On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:54: score-system-spacing #'basic-distance = 20 \mm On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:61: \header { piece = "Allegro" opus = "" } On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:73: \header { piece = "Romanze" opus = "" } On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:82: \layout {} On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:88: \header { piece = "Rondo" opus = "" } On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... File input/regression/mozart-hrn3-defs.ily (right): http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:16: \override MultiMeasureRest #'padding = #0.5 On 2011/08/02 19:59:39, Neil Puttock wrote: If it doesn't do anything, I'll just take it out. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:22: \override Beam #'thickness = #0.6 On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:23: \override Beam #'space-function = #(lambda (beam mult) 0.8) On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:27: \override VerticalAxisGroup #'minimum-Y-extent = #'(-2.5 . 3.5) On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:32: indent = 10. \mm On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-def... input/regression/mozart-hrn3-defs.ily:33: line-width = 189. \mm On 2011/08/02 19:59:39, Neil Puttock wrote: Done. http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-ron... File input/regression/mozart-hrn3-rondo.ily (right): http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn3-ron... input/regression/mozart-hrn3-rondo.ily:23: rondo = \relative c' { On 2011/08/02 19:59:39, Neil Puttock wrote: Done.
Sign in to reply to this message.
----- Original Message ----- From: <PhilEHolmes@googlemail.com> To: <percival.music.ca@gmail.com>; <n.puttock@gmail.com> Cc: <reply@codereview.appspotmail.com>; <lilypond-devel@gnu.org> Sent: Saturday, August 06, 2011 3:57 PM Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) > Again - not too familiar with the codereview tool, so I hope this makes > sense. New patch set soon. > > > http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly > File input/regression/mozart-hrn-3.ly (right): > > http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... > input/regression/mozart-hrn-3.ly:25: \fill-line { "This music is part of > the Mutopia project," > On 2011/08/02 19:59:39, Neil Puttock wrote: > > Can't see a missing line. I was trying to recreate the old > regtest, but with better syntax, so wasn't adding lines > where they didn't exist. Hmm. Wherever it says "Neil Puttock wrote" it means Neil made a comment here and I responded with the words below. Must work out how the codereview site works... -- Phil Holmes
Sign in to reply to this message.
http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly File input/regression/mozart-hrn-3.ly (right): http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... input/regression/mozart-hrn-3.ly:25: \fill-line { "This music is part of the Mutopia project," On 2011/08/06 14:57:14, PhilEHolmes wrote: > On 2011/08/02 19:59:39, Neil Puttock wrote: > > Can't see a missing line. I was trying to recreate the old > regtest, but with better syntax, so wasn't adding lines > where they didn't exist. Not a missing line, but a missing \line (the markup command); I suspect it's a convert-ly problem from a while back (when the markup syntax changed). Here's the last page from the 2.4 docs: http://lilypond.org/doc/v2.4/input/mutopia/W.A.Mozart/out-www/mozart-hrn-3-pa...
Sign in to reply to this message.
----- Original Message ----- From: <n.puttock@gmail.com> To: <PhilEHolmes@googlemail.com>; <percival.music.ca@gmail.com>; <mail@philholmes.net> Cc: <lilypond-devel@gnu.org>; <reply@codereview.appspotmail.com> Sent: Saturday, August 06, 2011 4:23 PM Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) > > http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly > File input/regression/mozart-hrn-3.ly (right): > > http://codereview.appspot.com/4811066/diff/1/input/regression/mozart-hrn-3.ly... > input/regression/mozart-hrn-3.ly:25: \fill-line { "This music is part of > the Mutopia project," > On 2011/08/06 14:57:14, PhilEHolmes wrote: >> On 2011/08/02 19:59:39, Neil Puttock wrote: > >> Can't see a missing line. I was trying to recreate the old >> regtest, but with better syntax, so wasn't adding lines >> where they didn't exist. > > Not a missing line, but a missing \line (the markup command); I suspect > it's a convert-ly problem from a while back (when the markup syntax > changed). > > Here's the last page from the 2.4 docs: > http://lilypond.org/doc/v2.4/input/mutopia/W.A.Mozart/out-www/mozart-hrn-3-pa... > > http://codereview.appspot.com/4811066/ Thanks for your time on this one Neil. Final patch (?) uploaded for review - it uses \line where appropriate, I hope. (BTW - I also got rid of the code that changed the beams and stems - I think it's better without.) -- Phil Holmes
Sign in to reply to this message.
LGTM. My comment is a tiny nitpick; I don't think it needs to hold up pushing this. http://codereview.appspot.com/4811066/diff/8003/input/regression/mozart-hrn-3.ly File input/regression/mozart-hrn-3.ly (right): http://codereview.appspot.com/4811066/diff/8003/input/regression/mozart-hrn-3... input/regression/mozart-hrn-3.ly:27: \typewriter { "http://mutopiaproject.org/" } this would be slightly nicer if you used \url instead of \typewriter. That would use that typeface, but would also produce a clickable link.
Sign in to reply to this message.
----- Original Message ----- From: <percival.music.ca@gmail.com> To: <PhilEHolmes@googlemail.com>; <n.puttock@gmail.com>; <mail@philholmes.net>; <email@philholmes.net> Cc: <lilypond-devel@gnu.org>; <reply@codereview.appspotmail.com> Sent: Sunday, August 07, 2011 11:42 PM Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) > LGTM. My comment is a tiny nitpick; I don't think it needs to hold up > pushing this. > > > http://codereview.appspot.com/4811066/diff/8003/input/regression/mozart-hrn-3.ly > File input/regression/mozart-hrn-3.ly (right): > > http://codereview.appspot.com/4811066/diff/8003/input/regression/mozart-hrn-3... > input/regression/mozart-hrn-3.ly:27: \typewriter { > "http://mutopiaproject.org/" } > this would be slightly nicer if you used \url instead of \typewriter. > That would use that typeface, but would also produce a clickable link. > > http://codereview.appspot.com/4811066/ Nitpick changed - I've kept typewriter for the look, but added url. Patch attached. Please push. -- Phil Holmes
Sign in to reply to this message.
I'll push it. james )-----Original Message----- )From: lilypond-devel-bounces+james.lowe=datacore.com@gnu.org )[mailto:lilypond-devel-bounces+james.lowe=datacore.com@gnu.org] On )Behalf Of Phil Holmes )Sent: 08 August 2011 15:54 )To: PhilEHolmes@googlemail.com; percival.music.ca@gmail.com; )n.puttock@gmail.com; mail@philholmes.net; email@philholmes.net; )lilypond-devel@gnu.org; reply@codereview.appspotmail.com )Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) ) )----- Original Message ----- )From: <percival.music.ca@gmail.com> )To: <PhilEHolmes@googlemail.com>; <n.puttock@gmail.com>; )<mail@philholmes.net>; <email@philholmes.net> )Cc: <lilypond-devel@gnu.org>; <reply@codereview.appspotmail.com> )Sent: Sunday, August 07, 2011 11:42 PM )Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) ) ) )> LGTM. My comment is a tiny nitpick; I don't think it needs to hold up )> pushing this. )> )> )> )http://codereview.appspot.com/4811066/diff/8003/input/regression/mo )zar )> t-hrn-3.ly File input/regression/mozart-hrn-3.ly (right): )> )> )http://codereview.appspot.com/4811066/diff/8003/input/regression/mo )zar )> t-hrn-3.ly#newcode27 )> input/regression/mozart-hrn-3.ly:27: \typewriter { )> "http://mutopiaproject.org/" } this would be slightly nicer if you )> used \url instead of \typewriter. )> That would use that typeface, but would also produce a clickable link. )> )> http://codereview.appspot.com/4811066/ ) )Nitpick changed - I've kept typewriter for the look, but added url. Patch )attached. Please push. ) )-- )Phil Holmes
Sign in to reply to this message.
commit 403e66a45e752231fb49deea2e8ccb1976505c7c Thanks Mr Holmes! James )-----Original Message----- )From: lilypond-devel-bounces+james.lowe=datacore.com@gnu.org )[mailto:lilypond-devel-bounces+james.lowe=datacore.com@gnu.org] On )Behalf Of James Lowe )Sent: 08 August 2011 16:13 )To: Phil Holmes; PhilEHolmes@googlemail.com; )percival.music.ca@gmail.com; n.puttock@gmail.com; )mail@philholmes.net; lilypond-devel@gnu.org; )reply@codereview.appspotmail.com )Subject: RE: Rewrite regtest mozart-hrn-3.ly (issue4811066) )Importance: Low ) )I'll push it. ) )james ) ))-----Original Message----- ))From: lilypond-devel-bounces+james.lowe=datacore.com@gnu.org ))[mailto:lilypond-devel-bounces+james.lowe=datacore.com@gnu.org] )On )Behalf Of Phil Holmes ))Sent: 08 August 2011 15:54 ))To: PhilEHolmes@googlemail.com; percival.music.ca@gmail.com; ))n.puttock@gmail.com; mail@philholmes.net; email@philholmes.net; ))lilypond-devel@gnu.org; reply@codereview.appspotmail.com ))Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) )) ))----- Original Message ----- ))From: <percival.music.ca@gmail.com> ))To: <PhilEHolmes@googlemail.com>; <n.puttock@gmail.com>; ))<mail@philholmes.net>; <email@philholmes.net> ))Cc: <lilypond-devel@gnu.org>; <reply@codereview.appspotmail.com> ))Sent: Sunday, August 07, 2011 11:42 PM ))Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) )) )) ))> LGTM. My comment is a tiny nitpick; I don't think it needs to hold up )> )pushing this. ))> ))> ))> ))http://codereview.appspot.com/4811066/diff/8003/input/regression/m )o ))zar ))> t-hrn-3.ly File input/regression/mozart-hrn-3.ly (right): ))> ))> ))http://codereview.appspot.com/4811066/diff/8003/input/regression/m )o ))zar ))> t-hrn-3.ly#newcode27 ))> input/regression/mozart-hrn-3.ly:27: \typewriter { )> )"http://mutopiaproject.org/" } this would be slightly nicer if you )> used )\url instead of \typewriter. ))> That would use that typeface, but would also produce a clickable link. ))> ))> http://codereview.appspot.com/4811066/ )) ))Nitpick changed - I've kept typewriter for the look, but added url. Patch ))attached. Please push. )) ))-- ))Phil Holmes ) )_______________________________________________ )lilypond-devel mailing list )lilypond-devel@gnu.org )https://lists.gnu.org/mailman/listinfo/lilypond-devel
Sign in to reply to this message.
Woh. There's an error - \url isn't part of Lilypond syntax! I'd corrected this to GP, but here's a corrected patch. -- Phil Holmes ----- Original Message ----- From: "James Lowe" <James.Lowe@datacore.com> To: "Phil Holmes" <email@philholmes.net>; <PhilEHolmes@googlemail.com>; <percival.music.ca@gmail.com>; <n.puttock@gmail.com>; <mail@philholmes.net>; <lilypond-devel@gnu.org>; <reply@codereview.appspotmail.com> Sent: Monday, August 08, 2011 4:13 PM Subject: RE: Rewrite regtest mozart-hrn-3.ly (issue4811066) I'll push it. james )-----Original Message----- )From: lilypond-devel-bounces+james.lowe=datacore.com@gnu.org )[mailto:lilypond-devel-bounces+james.lowe=datacore.com@gnu.org] On )Behalf Of Phil Holmes )Sent: 08 August 2011 15:54 )To: PhilEHolmes@googlemail.com; percival.music.ca@gmail.com; )n.puttock@gmail.com; mail@philholmes.net; email@philholmes.net; )lilypond-devel@gnu.org; reply@codereview.appspotmail.com )Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) ) )----- Original Message ----- )From: <percival.music.ca@gmail.com> )To: <PhilEHolmes@googlemail.com>; <n.puttock@gmail.com>; )<mail@philholmes.net>; <email@philholmes.net> )Cc: <lilypond-devel@gnu.org>; <reply@codereview.appspotmail.com> )Sent: Sunday, August 07, 2011 11:42 PM )Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) ) ) )> LGTM. My comment is a tiny nitpick; I don't think it needs to hold up )> pushing this. )> )> )> )http://codereview.appspot.com/4811066/diff/8003/input/regression/mo )zar )> t-hrn-3.ly File input/regression/mozart-hrn-3.ly (right): )> )> )http://codereview.appspot.com/4811066/diff/8003/input/regression/mo )zar )> t-hrn-3.ly#newcode27 )> input/regression/mozart-hrn-3.ly:27: \typewriter { )> "http://mutopiaproject.org/" } this would be slightly nicer if you )> used \url instead of \typewriter. )> That would use that typeface, but would also produce a clickable link. )> )> http://codereview.appspot.com/4811066/ ) )Nitpick changed - I've kept typewriter for the look, but added url. Patch )attached. Please push. ) )-- )Phil Holmes
Sign in to reply to this message.
no problem. I'll do it now. James )-----Original Message----- )From: Phil Holmes [mailto:mail@philholmes.net] )Sent: 08 August 2011 16:46 )To: James Lowe; PhilEHolmes@googlemail.com; )percival.music.ca@gmail.com; n.puttock@gmail.com; lilypond- )devel@gnu.org; reply@codereview.appspotmail.com )Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) ) ) )Woh. There's an error - \url isn't part of Lilypond syntax! I'd corrected this )to GP, but here's a corrected patch. ) )-- )Phil Holmes ) ) )----- Original Message ----- )From: "James Lowe" <James.Lowe@datacore.com> )To: "Phil Holmes" <email@philholmes.net>; )<PhilEHolmes@googlemail.com>; <percival.music.ca@gmail.com>; )<n.puttock@gmail.com>; <mail@philholmes.net>; <lilypond- )devel@gnu.org>; <reply@codereview.appspotmail.com> )Sent: Monday, August 08, 2011 4:13 PM )Subject: RE: Rewrite regtest mozart-hrn-3.ly (issue4811066) ) ) )I'll push it. ) )james ) ))-----Original Message----- ))From: lilypond-devel-bounces+james.lowe=datacore.com@gnu.org ))[mailto:lilypond-devel-bounces+james.lowe=datacore.com@gnu.org] )On )Behalf Of Phil Holmes ))Sent: 08 August 2011 15:54 ))To: PhilEHolmes@googlemail.com; percival.music.ca@gmail.com; ))n.puttock@gmail.com; mail@philholmes.net; email@philholmes.net; ))lilypond-devel@gnu.org; reply@codereview.appspotmail.com ))Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) )) ))----- Original Message ----- ))From: <percival.music.ca@gmail.com> ))To: <PhilEHolmes@googlemail.com>; <n.puttock@gmail.com>; ))<mail@philholmes.net>; <email@philholmes.net> ))Cc: <lilypond-devel@gnu.org>; <reply@codereview.appspotmail.com> ))Sent: Sunday, August 07, 2011 11:42 PM ))Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) )) )) ))> LGTM. My comment is a tiny nitpick; I don't think it needs to hold up )> )pushing this. ))> ))> ))> ))http://codereview.appspot.com/4811066/diff/8003/input/regression/m )o ))zar ))> t-hrn-3.ly File input/regression/mozart-hrn-3.ly (right): ))> ))> ))http://codereview.appspot.com/4811066/diff/8003/input/regression/m )o ))zar ))> t-hrn-3.ly#newcode27 ))> input/regression/mozart-hrn-3.ly:27: \typewriter { )> )"http://mutopiaproject.org/" } this would be slightly nicer if you )> used )\url instead of \typewriter. ))> That would use that typeface, but would also produce a clickable link. ))> ))> http://codereview.appspot.com/4811066/ )) ))Nitpick changed - I've kept typewriter for the look, but added url. Patch ))attached. Please push. )) ))-- ))Phil Holmes
Sign in to reply to this message.
Phil, ________________________________________ From: Phil Holmes [mail@philholmes.net] Sent: 08 August 2011 16:45 To: James Lowe; PhilEHolmes@googlemail.com; percival.music.ca@gmail.com; n.puttock@gmail.com; lilypond-devel@gnu.org; reply@codereview.appspotmail.com Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) Woh. There's an error - \url isn't part of Lilypond syntax! I'd corrected this to GP, but here's a corrected patch. ---- It's a bit blind leading the partially sighted here. I cannot apply this patch because I've already applied the original one. So can you reset (big red button in lilygit.tcl), then update source (git pull -r) and then make a new patch with just the correction? James
Sign in to reply to this message.
On Mon, Aug 08, 2011 at 04:45:49PM +0100, Phil Holmes wrote: > > Woh. There's an error - \url isn't part of Lilypond syntax! I'd > corrected this to GP, but here's a corrected patch. apparently the command is called \with-url instead. Cheers, - Graham
Sign in to reply to this message.
----- Original Message ----- From: "James Lowe" <James.Lowe@datacore.com> To: "Phil Holmes" <mail@philholmes.net>; <PhilEHolmes@googlemail.com>; <percival.music.ca@gmail.com>; <n.puttock@gmail.com>; <lilypond-devel@gnu.org>; <reply@codereview.appspotmail.com> Sent: Monday, August 08, 2011 4:58 PM Subject: RE: Rewrite regtest mozart-hrn-3.ly (issue4811066) Phil, ________________________________________ From: Phil Holmes [mail@philholmes.net] Sent: 08 August 2011 16:45 To: James Lowe; PhilEHolmes@googlemail.com; percival.music.ca@gmail.com; n.puttock@gmail.com; lilypond-devel@gnu.org; reply@codereview.appspotmail.com Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) Woh. There's an error - \url isn't part of Lilypond syntax! I'd corrected this to GP, but here's a corrected patch. ---- It's a bit blind leading the partially sighted here. I cannot apply this patch because I've already applied the original one. So can you reset (big red button in lilygit.tcl), then update source (git pull -r) and then make a new patch with just the correction? James= ===================== Thanks. Here you go. -- Phil Holmes
Sign in to reply to this message.
Second time's the charm ;) commit 6f818c30809d32c10b33d13e51f0910788ac0493 James )-----Original Message----- )From: Phil Holmes [mailto:phileholmes@googlemail.com] On Behalf Of )Phil Holmes )Sent: 08 August 2011 17:03 )To: James Lowe; PhilEHolmes@googlemail.com; )percival.music.ca@gmail.com; n.puttock@gmail.com; lilypond- )devel@gnu.org; reply@codereview.appspotmail.com )Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) ) )----- Original Message ----- )From: "James Lowe" <James.Lowe@datacore.com> )To: "Phil Holmes" <mail@philholmes.net>; )<PhilEHolmes@googlemail.com>; <percival.music.ca@gmail.com>; )<n.puttock@gmail.com>; <lilypond-devel@gnu.org>; )<reply@codereview.appspotmail.com> )Sent: Monday, August 08, 2011 4:58 PM )Subject: RE: Rewrite regtest mozart-hrn-3.ly (issue4811066) ) ) )Phil, )________________________________________ )From: Phil Holmes [mail@philholmes.net] )Sent: 08 August 2011 16:45 )To: James Lowe; PhilEHolmes@googlemail.com; )percival.music.ca@gmail.com; n.puttock@gmail.com; lilypond- )devel@gnu.org; reply@codereview.appspotmail.com )Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) ) )Woh. There's an error - \url isn't part of Lilypond syntax! I'd corrected this )to GP, but here's a corrected patch. ) )---- ) )It's a bit blind leading the partially sighted here. ) )I cannot apply this patch because I've already applied the original one. ) )So can you reset (big red button in lilygit.tcl), then update source (git pull - )r) and then make a new patch with just the correction? ) )James= )===================== ) )Thanks. Here you go. ) )-- )Phil Holmes
Sign in to reply to this message.
Since you pushed it, James, could you correct the \typewriter to \with-url ? Before you push, compile it on the command-line to make sure you have the right syntax and you have a clickable link. Cheers, - Graham On Mon, Aug 08, 2011 at 04:08:16PM +0000, James Lowe wrote: > Second time's the charm > > ;) > > commit 6f818c30809d32c10b33d13e51f0910788ac0493 > > James > > )-----Original Message----- > )From: Phil Holmes [mailto:phileholmes@googlemail.com] On Behalf Of > )Phil Holmes > )Sent: 08 August 2011 17:03 > )To: James Lowe; PhilEHolmes@googlemail.com; > )percival.music.ca@gmail.com; n.puttock@gmail.com; lilypond- > )devel@gnu.org; reply@codereview.appspotmail.com > )Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) > ) > )----- Original Message ----- > )From: "James Lowe" <James.Lowe@datacore.com> > )To: "Phil Holmes" <mail@philholmes.net>; > )<PhilEHolmes@googlemail.com>; <percival.music.ca@gmail.com>; > )<n.puttock@gmail.com>; <lilypond-devel@gnu.org>; > )<reply@codereview.appspotmail.com> > )Sent: Monday, August 08, 2011 4:58 PM > )Subject: RE: Rewrite regtest mozart-hrn-3.ly (issue4811066) > ) > ) > )Phil, > )________________________________________ > )From: Phil Holmes [mail@philholmes.net] > )Sent: 08 August 2011 16:45 > )To: James Lowe; PhilEHolmes@googlemail.com; > )percival.music.ca@gmail.com; n.puttock@gmail.com; lilypond- > )devel@gnu.org; reply@codereview.appspotmail.com > )Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) > ) > )Woh. There's an error - \url isn't part of Lilypond syntax! I'd corrected this > )to GP, but here's a corrected patch. > ) > )---- > ) > )It's a bit blind leading the partially sighted here. > ) > )I cannot apply this patch because I've already applied the original one. > ) > )So can you reset (big red button in lilygit.tcl), then update source (git pull - > )r) and then make a new patch with just the correction? > ) > )James= > )===================== > ) > )Thanks. Here you go. > ) > )-- > )Phil Holmes > > _______________________________________________ > lilypond-devel mailing list > lilypond-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/lilypond-devel
Sign in to reply to this message.
----- Original Message ----- From: "Graham Percival" <graham@percival-music.ca> To: "James Lowe" <James.Lowe@datacore.com> Cc: "Phil Holmes" <email@philholmes.net>; <PhilEHolmes@googlemail.com>; <percival.music.ca@gmail.com>; <n.puttock@gmail.com>; <lilypond-devel@gnu.org>; <reply@codereview.appspotmail.com> Sent: Monday, August 08, 2011 10:32 PM Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066) > Since you pushed it, James, could you correct the \typewriter to > \with-url ? Before you push, compile it on the command-line to > make sure you have the right syntax and you have a clickable link. > > Cheers, > - Graham I said in a separate message that this isn't necessary. The link is automatically converted to a clickable link. \with-url allows you to make a link with text different from the link, which we don't want here. -- Phil Holmes
Sign in to reply to this message.
On 9 August 2011 09:47, Phil Holmes <email@philholmes.net> wrote: > I said in a separate message that this isn't necessary. The link is > automatically converted to a clickable link. This only works reliably with Adobe Reader. Foxit produces an incorrect link: mutopia.org/It (it picks up the start of the next line) Neither okular nor evince produces a link. Cheers, Neil
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
