Re: Fixx #3314 : use superscript for powerChordSymbol (issue 348060043 by v.villen...@gmail.com)
message: On 2018/12/13 12:26:07, Valentin Villenave wrote: https://codereview.appspot.com/348060043/diff/1/ly/chord-modifiers-init.ly File ly/chord-modifiers-init.ly (right): https://codereview.appspot.com/348060043/diff/1/ly/chord-modifiers-init.ly#newcode49 ly/chord-modifiers-init.ly:49: 1-\markup { \super "5" } On 2018/12/13 11:32:34, dak wrote: > The way this is arranged currently, it reads jarringly. I agree, there are a few things that look quite bad: among others, \partialJazzMusic should be rewritten (and its use is hardly documented); there’s a TODO line 534 in chord.itely that should be addressed; Documentation/included/chord-names-jazz.ly should be entirely rewritten by taking advantage of predefined exception lists rather than duplicating lots of stuff; "banter-chord-names" isn’t documented anywhere and looks deprecated anyway; and there are a bunch of regtests that have been scrapped over the years. Since I don’t know much about all of these and was reluctant to open that particular can of worms, I was merely aiming for the low-hanging fruits here :-) Putting the change up where it does not look out of place _is_ low-hanging fruit. The reason this was kept the way it was most likely is because the change looked out of place. Moving it among its kind would fix that. That's a maintainability issue making the difference between people daring to touch things and not (as it has proven to be by the long history of the issue if nothing else). So I don't really get the "we could do more, so let's do less" argument here. https://codereview.appspot.com/348060043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixx #3314 : use superscript for powerChordSymbol (issue 348060043 by v.villen...@gmail.com)
Hi, Am 13.12.18 um 13:26 schrieb v.villen...@gmail.com: > There doesn’t appear to be any reason why normal-size-super > was used in the first place, unlike in other chords. Thanks for addressing this. I always had a part in my standard includes to correct that. Of course, it's a small thing, but thanks for taking care! Joram ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fixx #3314 : use superscript for powerChordSymbol (issue 348060043 by v.villen...@gmail.com)
Reviewers: dak, https://codereview.appspot.com/348060043/diff/1/ly/chord-modifiers-init.ly File ly/chord-modifiers-init.ly (right): https://codereview.appspot.com/348060043/diff/1/ly/chord-modifiers-init.ly#newcode49 ly/chord-modifiers-init.ly:49: 1-\markup { \super "5" } On 2018/12/13 11:32:34, dak wrote: The way this is arranged currently, it reads jarringly. I agree, there are a few things that look quite bad: among others, \partialJazzMusic should be rewritten (and its use is hardly documented); there’s a TODO line 534 in chord.itely that should be addressed; Documentation/included/chord-names-jazz.ly should be entirely rewritten by taking advantage of predefined exception lists rather than duplicating lots of stuff; "banter-chord-names" isn’t documented anywhere and looks deprecated anyway; and there are a bunch of regtests that have been scrapped over the years. Since I don’t know much about all of these and was reluctant to open that particular can of worms, I was merely aiming for the low-hanging fruits here :-) Description: Fixx #3314 : use superscript for powerChordSymbol There doesn’t appear to be any reason why normal-size-super was used in the first place, unlike in other chords. Please review this at https://codereview.appspot.com/348060043/ Affected files (+2, -2 lines): M ly/chord-modifiers-init.ly Index: ly/chord-modifiers-init.ly diff --git a/ly/chord-modifiers-init.ly b/ly/chord-modifiers-init.ly index 4f7483fc99c42fff8c1d0bff872fd56cf8c6ea0a..72b99a33fa72cb1481455b29cffcc5a31d9aaa4b 100644 --- a/ly/chord-modifiers-init.ly +++ b/ly/chord-modifiers-init.ly @@ -46,8 +46,8 @@ partialJazzMusic = { } powerChordSymbol = { - -\markup { \normal-size-super "5" } - 1-\markup { \normal-size-super "5" } + 1-\markup { \super "5" } + -\markup { \super "5" } } ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Fixx #3314 : use superscript for powerChordSymbol (issue 348060043 by v.villen...@gmail.com)
https://codereview.appspot.com/348060043/diff/1/ly/chord-modifiers-init.ly File ly/chord-modifiers-init.ly (right): https://codereview.appspot.com/348060043/diff/1/ly/chord-modifiers-init.ly#newcode49 ly/chord-modifiers-init.ly:49: 1-\markup { \super "5" } This looks out of whack with the preceding definition of \partialJazzMusic (which however has its own powerchord definition). Maybe move the whole definition up to after \ignatzekExceptionMusic where it logically belongs and which uses the same kind of superscript? The way this is arranged currently, it reads jarringly. https://codereview.appspot.com/348060043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel