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

Issue 348060043: Fixx #3314 : use superscript for powerChordSymbol (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 9 months ago by Valentin Villenave
Modified:
3 years, 7 months ago
Reviewers:
dak
CC:
lilypond-devel_gnu.org
Visibility:
Public.

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.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M ly/chord-modifiers-init.ly View 1 chunk +2 lines, -2 lines 2 comments Download

Messages

Total messages: 3
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: <c g>1-\markup { \super "5" } This looks out ...
3 years, 9 months ago (2018-12-13 11:32:34 UTC) #1
Valentin Villenave
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: <c g>1-\markup { \super "5" } On 2018/12/13 11:32:34, ...
3 years, 9 months ago (2018-12-13 12:26:07 UTC) #2
dak
3 years, 9 months ago (2018-12-15 13:31:55 UTC) #3
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#ne...
> ly/chord-modifiers-init.ly:49: <c g>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.
Sign in to reply to this message.

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