Re: Automatic LyricExtenders (issue 313240043 by address@hidden)

[perpeduumimmobile]

On 2016/12/26 19:14:00, pkx166h wrote:
> On 2016/12/25 21:53:55, akobel wrote:
> > Bottom line: I withdraw both proposals.

> Can you then re-submit a new patch or delete the one(s) that are
invalid?

Sorry, I'm a bit drawn up between my position as the Rietveld-proxy of
Knut and my own Alexander-role as a commenter.
I forwarded all patches by Knut, and I withdrew my two comments from #5
and the one from the mailing list that Knut answered in #6.

The patch set is up to date.

> I don't know what has and what has not been tested for sure, as far as
I can
> tell only patch set 1 has been tested which had a slew of warning
messages
> during the reg test output that I don't know are expected, desirable
or
> something else.

W.r.t. the regtest differences, as far as I (Alexander) can tell:

The warnings should be gone with Knut's parser modification in patch set
3.

The image differences are expected. A few short extenders are removed
(due to the different handling of collapse-length vs. the old
minimum-length). The exact setting of default collapse-length could be
discussed, but IMHO the setting is fine.
A few extenders appear that have not been written explicitly before, but
that is expected, too.
I guess for page-spacing-nonstaff-lines-independent.ly, the extender
should be forbidden, to have the intended effect of this regtest shown
more pronounced.

However, I propose that we first discuss the patch itself, and settle on
syntax and choice of default parameters (collapse-length, force-length).
AFAICS, once this is done, we will need a pass over the entire
documentation and regtests anyway that removes all __ in lyrics - not
for functionality, but to purge outdated and possibly confusing syntax.
That's basically a `sed -e 's/ __//g'`, but we will have to do a quick
review of at least the affected docs to check for errors.

Also, a convert-ly rule still has to be written that will, e.g., also
solve the difference in the page-spacing-nonstaff-lines-independent.ly
regtest.


Cheers,
Alexander

https://codereview.appspot.com/313240043/