Re: Fixx #3314 : use superscript for powerChordSymbol (issue 348060043 by v.villen...@gmail.com)

2018-12-15 Thread dak 

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)

2018-12-13 Thread Joram 
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)

2018-12-13 Thread v . villenave 

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)

2018-12-13 Thread 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" }
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