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

Issue 4800051: Modify chord-name-engraver to call capo-handler

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 11 months ago by Janek Warchol
Modified:
7 years, 10 months ago
Reviewers:
antlists, janek, dak, carl.d.sorensen, Neil Puttock, c_sorensen, pkx166h, Carl
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Modify chord-name-engraver to call capo-handler Add capo properties to define-context-properties Add capo-handler function for guitar chords

Patch Set 1 #

Patch Set 2 : fix from Carl #

Total comments: 3

Patch Set 3 : fixed indentation #

Total comments: 1

Patch Set 4 : fixed regression in fret-diagrams-string-thickness.ly #

Patch Set 5 : add docs #

Patch Set 6 : adding reg test #

Total comments: 1

Patch Set 7 : modified regtest #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+110 lines, -2 lines) Patch
M Documentation/notation/chords.itely View 1 2 3 4 1 chunk +50 lines, -0 lines 1 comment Download
A input/regression/chord-capo.ly View 1 2 3 4 5 6 1 chunk +28 lines, -0 lines 1 comment Download
M lily/chord-name-engraver.cc View 1 2 3 4 5 6 1 chunk +2 lines, -2 lines 1 comment Download
M scm/chord-name.scm View 1 2 3 1 chunk +28 lines, -0 lines 0 comments Download
M scm/define-context-properties.scm View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 21
Janek Warchol
7 years, 11 months ago (2011-07-25 21:29:01 UTC) #1
Janek Warchol
New patch from Wol uploaded. According to Carl, it addresses previous concerns; we only have ...
7 years, 11 months ago (2011-07-25 21:31:22 UTC) #2
Janek Warchol
Uploaded Carl's fix, everything works now! I'm making from scratch and will run regtests.
7 years, 11 months ago (2011-07-25 22:04:56 UTC) #3
Carl
Wol, Please fix the indentation, and make a new patch so that you get credit ...
7 years, 11 months ago (2011-07-25 22:21:23 UTC) #4
Carl
Wol, Please fix the indentation, and make a new patch so that you get credit ...
7 years, 11 months ago (2011-07-25 22:21:23 UTC) #5
antlists_youngman.org.uk
On 25/07/11 23:21, Carl.D.Sorensen@gmail.com wrote: > Wol, Hi Carl > > Please fix the indentation, ...
7 years, 11 months ago (2011-07-29 22:02:14 UTC) #6
Janek Warchol
New patch from Wol uploaded.
7 years, 11 months ago (2011-07-30 17:23:16 UTC) #7
Carl
LGTM, but we need a regtest before pushing.
7 years, 11 months ago (2011-07-30 20:30:24 UTC) #8
pkx166h
Doesn't pass a make check. I get an error on regression/fret-diagrams-string-thickness.ly log file shows: Renaming ...
7 years, 11 months ago (2011-07-30 21:49:54 UTC) #9
antlists_youngman.org.uk
On 30/07/11 22:49, pkx166h@gmail.com wrote: > Doesn't pass a make check. > > I get ...
7 years, 11 months ago (2011-07-30 22:52:48 UTC) #10
dak
http://codereview.appspot.com/4800051/diff/10001/scm/chord-name.scm File scm/chord-name.scm (right): http://codereview.appspot.com/4800051/diff/10001/scm/chord-name.scm#newcode177 scm/chord-name.scm:177: (if (not capo-pitch) Seems like a Lispism: '() evaluates ...
7 years, 11 months ago (2011-07-30 23:01:32 UTC) #11
antlists_youngman.org.uk
On 30/07/11 23:52, Wols Lists wrote: > On 30/07/11 22:49, pkx166h@gmail.com wrote: >> Doesn't pass ...
7 years, 11 months ago (2011-07-30 23:15:30 UTC) #12
c_sorensen
On 7/30/11 5:15 PM, "Wols Lists" <antlists@youngman.org.uk> wrote: > On 30/07/11 23:52, Wols Lists wrote: ...
7 years, 11 months ago (2011-07-30 23:22:47 UTC) #13
antlists_youngman.org.uk
On 31/07/11 00:22, Carl Sorensen wrote: > > > > On 7/30/11 5:15 PM, "Wols ...
7 years, 11 months ago (2011-07-30 23:35:41 UTC) #14
Janek Warchol
New patch uploaded. Passes regtests made from scratch.
7 years, 11 months ago (2011-07-31 10:35:46 UTC) #15
Janek Warchol
New patch set uploaded (adding a regtest). 2011/8/1 Wols Lists <antlists@youngman.org.uk>: > Regression test attached. ...
7 years, 11 months ago (2011-08-01 05:17:37 UTC) #16
pkx166h
Passes Make and reg tests
7 years, 11 months ago (2011-08-01 07:34:25 UTC) #17
Janek Warchol
http://codereview.appspot.com/4800051/diff/12002/input/regression/chord-capo.ly File input/regression/chord-capo.ly (right): http://codereview.appspot.com/4800051/diff/12002/input/regression/chord-capo.ly#newcode12 input/regression/chord-capo.ly:12: g1 I would change all g major chords into ...
7 years, 10 months ago (2011-08-02 19:59:06 UTC) #18
Neil Puttock
http://codereview.appspot.com/4800051/diff/12002/Documentation/notation/chords.itely File Documentation/notation/chords.itely (right): http://codereview.appspot.com/4800051/diff/12002/Documentation/notation/chords.itely#newcode516 Documentation/notation/chords.itely:516: In make-pitch, leave the first argument at 0, the ...
7 years, 10 months ago (2011-08-02 20:24:15 UTC) #19
antlists_youngman.org.uk
On 02/08/11 21:24, n.puttock@gmail.com wrote: > > http://codereview.appspot.com/4800051/diff/12002/Documentation/notation/chords.itely > > File Documentation/notation/chords.itely (right): > > ...
7 years, 10 months ago (2011-08-02 21:02:25 UTC) #20
janek
7 years, 10 months ago (2011-08-22 15:20:23 UTC) #21
Hi,

after disappearing for some time (sorry for that) i'm reading my inbox
and found that guitar capo transposer isn't finished!
Wol, how can i help you?  Would you like me to investigate the scheme
function about which you wrote on frogs list ("After capo handler
comes capo key")?
Maybe it would be good to move that discussion to -devel - not many
people watch Frogs list.

cheers,
Janek
Sign in to reply to this message.

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