Re: Issue 2245: always align dynamics and lyrics on "main" notehead (issue 108270044 by address@hidden)

[Janek Warchoł]

Hi,

2014-07-02 20:58 GMT+02:00  <address@hidden>:
> Janek Warchoł wrote:
>>
>> While I think that it's better to always align lyrics to
>> the "main notehead", I knew that some people would prefer
>> to do this otherwise, so the patch allows to choose how to
>> align LyricTexts (and DynamicTexts)
>
>
> Okay, this is less problematic than I thought it was, and
> I'm slowly convincing myself that for LyricText,
> main-notehead-alignment is better than NoteColumn-alignment.
> However, for DynamicText, I think NoteColumn-alignment is
> preferable, especially when the main notehead is on the
> right of the stem.  Gould (p.103) writes:
>
>   "When vertical space is limited, move a dynamic to the
>   left of the note -- never to the right, since the note has
>   already started!"

Hm.  Sounds reasonable.  On the other hand, if you have many staves
with dynamics, it's better if these dynamics are aligned - and with
alignment on NoteColumns, this would not happen.

>> \override LyricText.X-align-on-main-noteheads = ##f
>
>
> I think this interface (with booleans) is confusing; I would
> prefer a choice of symbols, something like this:
>
>   \override LyricText.X-alignment-anchor = #'NoteHead
>   \override LyricText.X-alignment-anchor = #'NoteColumn

Yeah, i'm also thinking about providing an interface like this in the
future iterations of the alignment rewrite.  However, can we push this
patch as-is, and make such changes later?  The thing is, there will be
so many changes in the alignment code that trying to come up with a
generic interface now (in the middle of work) is IMO a waste of
effort, because i'd continue to rewrite such code with every next
patch.  I think it will be much more efficient to first get all the
features in (using simple solutions), and then look at the resulting
code and generalize that - when we'll know what exactly we need.

>>> git complains about trailing whitespace here.
>> It was already there before, but i'll remove it in the
>> next patchset.
>
> I have this in my .vimrc:
>
>   " Remove trailing whitespace on write
>   autocmd BufWritePre * :%s/\s\+$//e

bad luck that i'm not using vim :)

> https://codereview.appspot.com/108270044/diff/40001/scm/define-grobs.scm
> File scm/define-grobs.scm (right):
>
> https://codereview.appspot.com/108270044/diff/40001/scm/define-grobs.scm#newcode589
> scm/define-grobs.scm:589: (X-offset .
> ,ly:self-alignment-interface::aligned-on-x-parent)
> Not a requirement, but you might as well alphabetize the prop-list while
> you're there (case-insensitive, so 'X-extent comes after
> 'vertical-skylines).  Same for the other prop-lists you modify below.

Ok, i'll remember this.

best,
Janek