Re: shortened flags affair, part 8: choosing appropriate flag (issue4312057)

[Janek Warchoł]

2011/4/4 <address@hidden>
>
> This patch loses flags!  is that deliberate?!
>  stem-tremolo.ly
>  flags-default.ly
>  beam-collision-beamcount.ly
> ... etc...

Did you "make clean" before building? There are some missing
dependancies in the fonts, so "make clean" must be called at least in
/mf directory, otherwise glyph names would not match what stem.cc
expects.
However, if you did this, and still some of the normal flags
disappear, then i suppose it's a bug related to adding "standard" to
the flag name (i.e. "flags.standardu5" instead of "flags.u5"). I will
investigate and fix it, however i'd like to know what word should we
use (maybe my comment in which i asked about it was lost?). The
question is: should we use word "standard" or maybe "default" or
"normal" (for regular flags)? Word "standard" has one problem: it
blends with direction ("standardu" looks acceptable, but "standardd"
can be confusing at first sight.). Also, i'm not sure which one has
the best-fitting meaning. Maybe "regular"?

> Have you tried a regtest comparison?

Ooops! Sorry, i forgot about it.

> Please read this:
> http://lilypond.org/doc/v2.13/Documentation/contributor/regtest-comparison
> and let me know if you need help with any of these steps.

Regtests exit with an error, here's what i tried:
git checkout master (a clone of origin/master, updated and without differencies)
/build make clean
/build make all
/build make test-baseline (it ran without errors)
changed branch to the one containing patches, also updated with git pull -r
/build make clean
/build make all
/build make check

It returned
(...)
Processing a2/lily-47d720ec
Processing ed/lily-26c56735
Processing 6c/lily-1839e4ca
Processing bc/liKilled
command failed: /home/janek/lilypond-git/build/out/bin/lilypond -I
/home/janek/lilypond-git/input/regression/ -I ./out-test -I
/home/janek/lilypond-git/input -I
/home/janek/lilypond-git/Documentation -I
/home/janek/lilypond-git/Documentation/snippets -I
/home/janek/lilypond-git/input/regression/ -I
/home/janek/lilypond-git/Documentation/included/ -I
/home/janek/lilypond-git/build/mf/out/ -I
/home/janek/lilypond-git/build/mf/out/ -I
/home/janek/lilypond-git/Documentation/pictures -I
/home/janek/lilypond-git/build/Documentation/pictures/./out-test
-dbackend=eps --formats=ps  -dseparate-log-files -dinclude-eps-fonts
-dgs-load-lily-fonts --header=texidoc -I
/home/janek/lilypond-git/Documentation/included/ -ddump-profile
-dcheck-internal-types -ddump-signatures -danti-alias-factor=1 -I
"/home/janek/lilypond-git/build/out/lybook-testdb"  -I
"/home/janek/lilypond-git/build/input/regression"  -I
"/home/janek/lilypond-git/input/regression"  -I
"/home/janek/lilypond-git/build/input/regression/out-test"  -I
"/home/janek/lilypond-git/input"  -I
"/home/janek/lilypond-git/Documentation"  -I
"/home/janek/lilypond-git/Documentation/snippets"  -I
"/home/janek/lilypond-git/input/regression"  -I
"/home/janek/lilypond-git/Documentation/included"  -I
"/home/janek/lilypond-git/build/mf/out"  -I
"/home/janek/lilypond-git/build/mf/out"  -I
"/home/janek/lilypond-git/Documentation/pictures"  -I
"/home/janek/lilypond-git/build/Documentation/pictures/out-test"
--formats=eps  -deps-box-padding=3.000000  -dread-file-list
-dno-strip-output-dir
"/home/janek/lilypond-git/build/out/lybook-testdb/snippet-names-1446493823.ly"
Child returned 137
make[2]: *** [out-test/collated-files.texi] Error 1
make[2]: Leaving directory `/home/janek/lilypond-git/build/input/regression'
make[1]: *** [local-test] Error 2
make[1]: Leaving directory `/home/janek/lilypond-git/build/input/regression'
make: *** [test] Error 2

I don't see anything special in lily-1839e4ca (if i understand
correctly, it contains a regtest named grace-nest5.ly). When i tried
compiling grace-nest5.ly separately, there were no problems.

> (if this patch is still in the "gathering information" mode, then ok...
> but I would like that to be much more explicit that I should not be
> evaluating the patch for potential inclusion)

Sorry for that. I though that patch_review meant that it's time that
other people see this and give their opinions, not necessarily that
the patch is ready for including.
I was puzzled and worried why there were no reviews for a whole week
and thought that maybe needs_work label was the cause.

> http://codereview.appspot.com/4312057/diff/19001/lily/stem.cc
> File lily/stem.cc (right):
>
> http://codereview.appspot.com/4312057/diff/19001/lily/stem.cc#newcode615
> lily/stem.cc:615: return (a>b);
> indent

Fixed.

> http://codereview.appspot.com/4312057/

thanks,
Janek