Re: GOP-PROP 9: behavior of make doc

[Graham Percival]

On Wed, Aug 10, 2011 at 05:31:56AM +, Keith OHara wrote:
> Graham Percival  percival-music.ca> writes:
> 
> > That proposal failed to achieve clarity and consensus
> > within a month, mostly due to the proposal being too broad.
> 
> By "to broad" I guess you mean that the old proposal affected `make bin`,
> which doesn't suffer the sea-of-output problem.
> The proposal was pretty clear, but required careful reading.

Yes and no -- the worst problem is that IIRC we reached agreement
on using some kind of log files for "make doc", but then got hung
up on arguing about "make".  However, the initial spark for this
work was the difficulty of "make doc", and the end result is that
serious work on "make doc" will be delayed by at least 6 weeks.

If the initial proposal had only dealt with "make doc", it might
have gotten approved with no trouble in 2 weeks, and we could have
had a full month of work by now.


That said, we could have still been working on the
--redirect-lilypond-output option for lilypond-book.  We'd be
thinking about our build process in the backs of our minds, but
explictly we'd be "trying to making it easier to write a thesis in
lilypond-book".  I mean, that's exactly the same use-case as our
build system, and this way we might even involve users in
debugging and/or ideas!

> However, the solidly-defined details pretty much redirect the sea
> of output to log files, similar to 
> 
>   make 2>err.log || echo "failed; see err.log"

That's not clear enough.  I'd want a separate log file for each
process (because we often have 2/4/8 processes going at once), and
to automatically echo the relevant part of the log file to the
screen.

It's not hard.  At some level, lilypond-book knows which
lily-1234.ly file it's trying to compile.  Each lily-1234.ly
contains a
  \sourcefilename "foo.ly"
that says where it came from.

Automatically extracting that information and informing the user
exactly where the problem is, could be done by me in about 2 hours
for an initial hack (30 minutes if I'm lucky), and about 4 hours
for a good job including reviews.  You could probably do it in the
same time frame.

Question is, does anybody want to spend that time if the final
answer might be "nice idea, but don't you dare touch our build
system" ?  ok, in the case of lilypond-book itself we might be
able to sneak it through (with regards to the "debugging a
lilypond-book thesis idea)... but if I was working on this with a
view to improving things for our doc writers, I'd be pissed if I
got stuff working and was told "that's fine for people writing a
thesis, but don't touch the build system".

So I wanted to get agreement in principle about this type of
system before encouraging Phil to move ahead with producing a
patch.


> >   The user may optionally request additional output to be
> > printed; this is controlled with the VERBOSE=x flag. In such
> > cases, all output will still be written to log files; the console
> > output is strictly additional to the log files.
> 
> If "all output" means the current verbose output, it might be hard
> to both get all output in log files, and desired output on screen.

Worst case: wrap the entire command in a python script.

> The easier way would be to tell the sub-processes how verbose to be,
> and tee all of stderr to file and screen.

Sure.  That's an IMO minor implementation detail, though.

> > * Both stderr and stdout will be saved in *.log. 
> 
> Probably like you mean "in /the same/ *.log file".

Yes.

> Have I said recently how much I really really like `make bin` ? 

Sure -- but that highlights one of the huge frustrations I have
with the build system.

There's a lot of awesome stuff there.  "make bin" only needed two
lines of "syntactic sugar".  Compiling an individual manual is
something that doc people have been requesting for *years*.  I've
known that it was possible for years, and always gave a vague
guideline on how to do it.  Phil recently tried it and said that
it didn't work, and I was sufficiently annoyed that I went and
figured it out.
(but the instructions aren't in the CG, so we'll probably go
through the same thing in another 6 months)

My general impression is that 95% of whatever we want to do is
already there.  It's just that we either don't know how to do it,
or nobody takes the time to explain how to do it to other people.

The whole situation leaves a really bad taste in my mouth.  :(
- Graham

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 9: behavior of make doc

[Keith OHara]

Graham Percival  percival-music.ca> writes:

> That proposal failed to achieve clarity and consensus
> within a month, mostly due to the proposal being too broad.

By "to broad" I guess you mean that the old proposal affected `make bin`,
which doesn't suffer the sea-of-output problem.
The proposal was pretty clear, but required careful reading.

They way I misread the old one was to assume that the goal:

> If there are build problems, then it should be easier to find out
> why it’s failing. 
[...]
> ** Sea of output

would be directly addressed in the 

> ** Proposal details
> 

However, the solidly-defined details pretty much redirect the sea
of output to log files, similar to 

  make 2>err.log || echo "failed; see err.log"

Maybe that is the first step to sort through the `make` process, and 
the *real* benefit comes from fishing the important bits out of the sea:

>   The user may optionally request additional output to be
> printed; this is controlled with the VERBOSE=x flag. In such
> cases, all output will still be written to log files; the console
> output is strictly additional to the log files.
>

If "all output" means the current verbose output, it might be hard
to both get all output in log files, and desired output on screen.
The easier way would be to tell the sub-processes how verbose to be,
and tee all of stderr to file and screen.
If "all output" is just what we asked for with the VERBOSE=x flag,
then the current verbose output could be requested on a second 
`make` if the desired output didn't tell us enough.

> * Both stderr and stdout will be saved in *.log. 

Probably like you mean "in /the same/ *.log file".

> However, there is a danger in this approach, that vital error
> messages can also be lost, 

Maybe Phil could play it safe and retain the old make rules that run 
the sub-processes in verbose mode.  Then it should be safe to make new 
make rules that run the sub-processes to report errors/warnings only 
(and maybe prefixed with @ so they don't echo the command-line).
This might be awkward when make(1) descends to subdirectories and builds
the default target there, or with whatever 'stepmake' is.

This does seem hard to define in advance before seeing what is possible.

Have I said recently how much I really really like `make bin` ? 


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Get rid of some compiler warnings (issue4854049)

[percival . music . ca]

On 2011/08/10 05:19:42, Graham Percival wrote:

input/regression/completion-heads-factor.ly:12: \paper {

#(set-paper-size "a6")

}
is this part of the compiler warnings patch?


sorry, ignore that, I only just looked at the changelog message.

http://codereview.appspot.com/4854049/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Get rid of some compiler warnings (issue4854049)

[percival . music . ca]

http://codereview.appspot.com/4854049/diff/5001/input/regression/completion-heads-factor.ly
File input/regression/completion-heads-factor.ly (right):

http://codereview.appspot.com/4854049/diff/5001/input/regression/completion-heads-factor.ly#newcode12
input/regression/completion-heads-factor.ly:12: \paper {
#(set-paper-size "a6") }
is this part of the compiler warnings patch?

http://codereview.appspot.com/4854049/diff/5001/lily/glissando-engraver.cc
File lily/glissando-engraver.cc (right):

http://codereview.appspot.com/4854049/diff/5001/lily/glissando-engraver.cc#newcode124
lily/glissando-engraver.cc:124: if (n1 < 0 || n2 < 0 || unsigned(n1) >=
note_heads.size ())
could we also get some parentheses around the cases?  If/when we add
  -Wextra
we'll get warnings about those anyway.

Also, my initial preference would be to case note_heads.size() to int
rather than n1 to unsigned, but I confess that I can't think of an
actual rule as to why that is.  It's just from seeing things done that
way more often than the alternative.

http://codereview.appspot.com/4854049/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Should this be applied?

[David Kastrup]

Neil Puttock  writes:

> On 8 August 2011 12:43, David Kastrup  wrote:
>
>> I have no clue what start and end are
>
> start and end are column ranks, i.e., an index to the position of a
> particular column in a score.  You can see them if you enable
> debugging for columns:
>
> \relative c' {
>   c1
> }
>
> \layout {
>   \context {
> \Score
> \override PaperColumn #'stencil = #ly:paper-column::print
> \override NonMusicalPaperColumn #'stencil = #ly:paper-column::print
>   }
> }
>
>> and I have no clue what pure is.
>
> https://secure.wikimedia.org/wikipedia/en/wiki/Pure_function
>
>> But get_pure_property takes start and end as additional arguments over
>> get_property, and so does pure_is_visible.
>>
>> So from a mere text-matching hand-waving likelihood point of view, the
>> above change seems plausible.
>>
>> Is there anybody that could shed light on this?
>
> The only effect I can see is that break-visiibility won't be cached.
> There are no pure break-visibility callbacks so the start/end args
> will never be applied.

So we have a basically unpure lookup function (caching is not all that
pure, and the callback functionality could be used for impurity as well)
that is called from a pure function, and one can determine by code
review that this will not cause trouble.

It was my distrust in this sort of reasoning that made me try to remove
cases of casting away const in the first place.

-- 
David Kastrup


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: [PATCH] Clean up some build warnings

[Graham Percival]

On Tue, Aug 09, 2011 at 10:13:53PM +0200, Reinhold Kainhofer wrote:
> Here is a patch that removes most of the compiler warnings on a 32-bit 
> platform:
> http://codereview.appspot.com/4854049

Thanks, added to:
http://code.google.com/p/lilypond/issues/detail?id=804

Cheers,
- Graham

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Get rid of some compiler warnings (issue4854049)

[k-ohara5a5a]

Reinhold Kainhofer  kainhofer.com> writes:
LGTM


Am Freitag, 5. August 2011, 21:22:49 schrieb Keith OHara:



> The entire source tree generates only seven warnings, all but one of

which

> point to code we could write more clearly.



Ah, great. Will you try to fix them?


You beat me to it :-)
I'll clean up the .ll, unless Neil has already done that.


http://codereview.appspot.com/4854049/diff/2001/lily/glissando-engraver.cc
File lily/glissando-engraver.cc (right):

http://codereview.appspot.com/4854049/diff/2001/lily/glissando-engraver.cc#newcode124
lily/glissando-engraver.cc:124: if (n1 < 0 || n2 < 0 || unsigned(n1) >=
note_heads.size ())
just confirming, that was what the compiler was doing, per the C++
standard.

http://codereview.appspot.com/4854049/diff/2001/lily/skyline.cc
File lily/skyline.cc (right):

http://codereview.appspot.com/4854049/diff/2001/lily/skyline.cc#newcode409
lily/skyline.cc:409: // FIXME: This has hardcoded logic, assuming a ==
AXIS_X!
 assert (AXIS_X == a);
instead?

http://codereview.appspot.com/4854049/diff/5001/lily/lyric-combine-music-iterator.cc
File lily/lyric-combine-music-iterator.cc (right):

http://codereview.appspot.com/4854049/diff/5001/lily/lyric-combine-music-iterator.cc#newcode224
lily/lyric-combine-music-iterator.cc:224:
Lyric_combine_music_iterator::check_new_context (SCM /*sev*/)
On 2011/08/09 21:20:29, Carl wrote:

Why not /*SCM sev*/ with a FIXME?


the function is declared with DECLARE_LISTENTER so it has to take a
stream event, even if it doesn't use it.

http://codereview.appspot.com/4854049/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

GOP-PROP 9: behavior of make doc

[Graham Percival]

The previous "build system" proposal, GOP-PROP 5, has been
cancelled.  That proposal failed to achieve clarity and consensus
within a month, mostly due to the proposal being too broad.
My apologies for failing to anticipate the level of disagreement;
I should have organized this discussion in a different way, or at
the very least cancelled the discussion earlier.

This new proposal focuses on "make doc" specifically.

http://lilypond.org/~graham/gop/gop_9.html


** Proposal summary

If there are build problems, then it should be easier to find out
why it’s failing. This will be achieved with log files, as well as
possibly including scripts which automatically display portions of
those log files for a failing build.

We will also add targets for building a specific manual (for
quick+easy checking of doc work), as well as for building all
documentation in a specific language (either English or a
translated language).


** Rationale

When the lilypond doc build breaks, it’s too hard to figure out
why it broke.

We see emails to lilypond-devel approximately once every four
months about broken doc builds. On a subjective note, Graham has
been the documentation editor since 2003, but even he cannot
reliably pinpoint the cause of a failing doc build within 10
minutes. We waste a ridiculous amount of time, effort, and
patience on doc build problems.


** Sea of output

Before any of the current work on reducing output from make, the
result of a “make doc” was over 500,000 lines of text. The prime
reason for the output being so verbose is that all the processes
that run as a result of the call to make echo their output to the
screen, often in verbose mode. Lilypond itself produces around
370,000 lines of output as a result of lilypond-book building all
the snippets.

Much of this output can be redirected to logfiles and so the
impossible-to-read clutter on the screen is cut out and could be
referred to later.


** Proposal details

When you run make doc,

* All output will be saved to various log files, with the
  exception of output directly from make(1).

  Note that make(1) refers to a specific executable file on
unix computers, and is not a general term for the build system.

* By default, no other output will be displayed on the
  console, with one exception: if a build fails, we might
  display some portion(s) of log file(s) which give useful
  clues about the reason for the failure.

  The user may optionally request additional output to be
printed; this is controlled with the VERBOSE=x flag. In such
cases, all output will still be written to log files; the console
output is strictly additional to the log files.

* Logfiles from calling lilypond (as part of lilypond-book)
  will go in the relevant
  ‘build/out/lybook-db/12/lily-123456.log’ file. All other
  logfiles will go in the ‘build/logfiles/’ directory.
* Both stderr and stdout will be saved in *.log. The order of
  lines from these streams should be preserved.
* There will be no additional “progress messages” during the
  build process. If you run make --silent, a non-failing build
  should print absolutely nothing to the screen.
* Assuming that the loglevels patch is accepted, lilypond
  (inside lilypond-book) will be run with –loglevel=WARN.
  http://codereview.appspot.com/4822055/
* Ideally, a failing build should provide hints about the
  reason why it failed, or at least hints about which log
  file(s) to examine. 

If this proposal is accepted, none of these policies will be
assumed to apply to any other aspect of the build system. Policies
for any other aspect of the build system will be discussed in
separate proposals.


** Don’t cause more build problems

However, there is a danger in this approach, that vital error
messages can also be lost, thus preventing the cause of the
failure of a make being found. We therefore need to be
exceptionally careful to move cautiously, include plenty of tests,
and give time for people to experiment/find problems in each stage
before proceeding to the next stage.

This will be done by starting from individual lilypond calls
within lilypond-book, and slowly moving to “larger” targets of the
build system – after the individual lilypond calls are are
producing the appropriate amount of output and this is saved in
the right place and we can automatically isolate parts of a
failing build, we will work on lilypond-book in general, and only
then will we look at the build system itself.


** Implementation notes

There is an existing make variable QUIET_BUILD, which alter the
amount of output being displayed
(http://lilypond.org/doc/v2.15/Documentation/contributor/useful-make-variables
). We are not planning on keeping this make variable.

The standard way for GNU packages to give more output is with a
V=x option. Presumably this is done by increasing x? If we support
this option, we should still write log files; we would simp

PATCH: 48-hour countdown

[Colin Campbell]

22:00 MDT Thursday Aug 11.

Issue 1800 : 
Segfault compiling LSR 346 - Rietveld Issue 4830064 

Issue 1604 : 
MultiMeasureRest staff position 0 is taken as staff position 2 - 
Rietveld Issue 4822046 
Issue 1735 : 
modifying default behaviour of tremolo slashes - Rietveld Issue 4636081 

Issue 1214 : 
Cue notes to music that creates a sub-voice does not work - Rietveld 
Issue 4816044 
Issue 509 : 
collision nested tuplet numbers - Rietveld Issue 4808082 

Issue 163 : huge 
(ugly) slur (both phrasing and normal) - Rietveld Issue 4810072 



Owners/developers, please push, close and mark ASAP after Thursday evening.

Thanks,
Colin

--
The human race has one really effective weapon, and that is laughter.
-- Mark Twain

 

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Reinhold Kainhofer]

Am Tuesday, 9. August 2011, 17:34:57 schrieb Phil Holmes:
> They're not being ignored.  They're not even being seen.  Please address my
> point of how you would see them in 37,000 lines of console output.

Of these 37071 lines, exactly 34111 are only from the font generation in mf/.
When coding, that's no problem, because we are only interested in the warnings 
from flower/ and lily/. The font generation (and the docs generation in make) 
comes long after the interesting file have been compiled. I usually just kill 
make as soon as lilypond has been linked... Linking takes a while, so I have 
enough time to check the warnings in the console output above. 

If we could silence metafont a little bit (but keep the warnings and errors, 
just to be on the safe side), the build output would be much shorter.

Cheers,
Reinhold
-- 
--
Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Doc: NR Warning added to para for cueduring (issue4850051)

[tdanielsmusic]

LGTM, with a tweak as suggested below to explain why.


http://codereview.appspot.com/4850051/diff/1/Documentation/notation/staff.itely
File Documentation/notation/staff.itely (right):

http://codereview.appspot.com/4850051/diff/1/Documentation/notation/staff.itely#newcode1304
Documentation/notation/staff.itely:1304: @warning{In this example, the
@code{Voice} context must be explicitly
@warning{When a @code{Voice} starts with @code{\cueDuring}, as in the
following example, ... }

http://codereview.appspot.com/4850051/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Progress on loose columns. (issue4841052)

[n . puttock]

Hi Mike,

The latest patch set breaks spacing-empty-bar.ly (we want
left-loose-column-padding to be 0 in this case), but apart from that it
looks great.

Cheers,
Neil


http://codereview.appspot.com/4841052/diff/14002/input/regression/loose-column-spacing.ly
File input/regression/loose-column-spacing.ly (right):

http://codereview.appspot.com/4841052/diff/14002/input/regression/loose-column-spacing.ly#newcode4
input/regression/loose-column-spacing.ly:4: texidoc = "Lilypond spaces
loose columns correctly in
remove `Lilypond'

http://codereview.appspot.com/4841052/diff/14002/input/regression/loose-column-spacing.ly#newcode10
input/regression/loose-column-spacing.ly:10: \new Staff \relative c' {
\times 2/3 {g'4 a2}}
indent (two spaces)

\times 2/3 { g'4 a2 } }

http://codereview.appspot.com/4841052/diff/14002/input/regression/loose-column-spacing.ly#newcode11
input/regression/loose-column-spacing.ly:11: \new Staff \relative c' {
indent

http://codereview.appspot.com/4841052/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fixes issue 40. (issue4801083)

[n . puttock]

LGTM.


http://codereview.appspot.com/4801083/diff/14002/scm/define-grobs.scm
File scm/define-grobs.scm (right):

http://codereview.appspot.com/4801083/diff/14002/scm/define-grobs.scm#newcode933
scm/define-grobs.scm:933: (end-on-accidental . #t)
trailing whitespace

http://codereview.appspot.com/4801083/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Carl Sorensen]

On 8/9/11 2:04 PM, "Wols Lists"  wrote:

> On 09/08/11 20:44, Neil Puttock wrote:
>> On 9 August 2011 20:21, Reinhold Kainhofer  wrote:
>> 
 So having only 9 warnings in our codebase (four of which are in the
 lexer/parser, which hardly anyone of us really understands!) is amazing.
>> There are many more warnings (> 180) if you're compiling a 64-bit
>> binary.  They could be silenced via casting, but Han-Wen isn't in
>> favour of that approach (http://codereview.appspot.com/1724041/):
>> 
>> "* Why are all the casts there?  Is this a 64 bit compiler thing?  Lily
>> compiles
>> virutally without warnings over here (core duo, gcc 4.4.4).  I think all the
>> casting hinders readability, so I propose to not add casts unless necessary.
>> If
>> the warnings bother you, add a targeted -Wno-xxx option to the Makefile.  I
>> doubt that there are any cases where there is the risk of a real error."
>> 
> Hmmm ...
> 
> "Is this a 64 bit compiler thing?" - not really. It's an "int" thing in
> all probability. sizeof(int) is explicitly undefined. As for there
> aren't any cases where there is a real risk of error, well I don't know
> of any compilers where sizeof(int) = 1, but that would be perfectly
> legal ...
> 
> If you shouldn't use cast, you shouldn't use int. Which is a shame,
> because I think int is defined to be "the natural length of the platform
> you're running on" ie it's the most efficient integer variant to use.
> 
> Imho (as I've said before) *all* warnings should be explained or fixed.
> Papering over the cracks by hiding them is likely to result sooner or
> later in a real problem slipping through (like a compiler defining
> sizeof(int) as 1 :-)

Looking at Han-Wen's comment in context, I think the best way to approach
this is to do as he suggests, which is to define v_size back to int.

Thanks,

Carl


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Get rid of some compiler warnings (issue4854049)

[Carl . D . Sorensen]

LGTM, with a few comments.

Thanks,

Carl



http://codereview.appspot.com/4854049/diff/2001/lily/skyline.cc
File lily/skyline.cc (right):

http://codereview.appspot.com/4854049/diff/2001/lily/skyline.cc#newcode417
lily/skyline.cc:417: buildings_ = internal_build_skyline (&boxes,
horizon_padding, a, UP);
What if instead of changing X_AXIS to a in line 417, we checked a to see
if it were equal to X_AXIS and issued a warning (or maybe a programming
error)?

Note that we have UP hardcoded as well, so at this point I don't think
we ought to remove the hardcoding on only the a.

http://codereview.appspot.com/4854049/diff/2001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):

http://codereview.appspot.com/4854049/diff/2001/ly/music-functions-init.ly#newcode894
ly/music-functions-init.ly:894: (_i "Scale @var{arg} up by a factor of
2^@var{dur}*(2-(1/2)^@var{dots}).")
I think the whole expression 2^... should be wrapped in a @code{} block.

Also, should this be part of the warning fix patch?

http://codereview.appspot.com/4854049/diff/5001/lily/lyric-combine-music-iterator.cc
File lily/lyric-combine-music-iterator.cc (right):

http://codereview.appspot.com/4854049/diff/5001/lily/lyric-combine-music-iterator.cc#newcode224
lily/lyric-combine-music-iterator.cc:224:
Lyric_combine_music_iterator::check_new_context (SCM /*sev*/)
Why not /*SCM sev*/ with a FIXME?

http://codereview.appspot.com/4854049/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fixes issue 40. (issue4801083)

[m...@apollinemike.com]

On Aug 9, 2011, at 10:04 PM, n.putt...@gmail.com wrote:

> On 2011/08/09 18:23:42, mike_apollinemike.com wrote:
> 
>> Good call - I've uploaded a new patch that uses "accidental-grob"
> (don't know
>> why I didn't think of that before...too much code on the brain!).
> 
> The latest patch set needs rebasing against master.
> 
> Cheers,
> Neil
> 
> http://codereview.appspot.com/4801083/

Done.

Cheers,
MS

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

[PATCH] Clean up some build warnings

[Reinhold Kainhofer]

Here is a patch that removes most of the compiler warnings on a 32-bit 
platform:
http://codereview.appspot.com/4854049


Cheers,
Reinhold
-- 
--
Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Wols Lists]

On 09/08/11 20:44, Neil Puttock wrote:
> On 9 August 2011 20:21, Reinhold Kainhofer  wrote:
> 
>> > So having only 9 warnings in our codebase (four of which are in the
>> > lexer/parser, which hardly anyone of us really understands!) is amazing.
> There are many more warnings (> 180) if you're compiling a 64-bit
> binary.  They could be silenced via casting, but Han-Wen isn't in
> favour of that approach (http://codereview.appspot.com/1724041/):
> 
> "* Why are all the casts there?  Is this a 64 bit compiler thing?  Lily 
> compiles
> virutally without warnings over here (core duo, gcc 4.4.4).  I think all the
> casting hinders readability, so I propose to not add casts unless necessary.  
> If
> the warnings bother you, add a targeted -Wno-xxx option to the Makefile.  I
> doubt that there are any cases where there is the risk of a real error."
> 
Hmmm ...

"Is this a 64 bit compiler thing?" - not really. It's an "int" thing in
all probability. sizeof(int) is explicitly undefined. As for there
aren't any cases where there is a real risk of error, well I don't know
of any compilers where sizeof(int) = 1, but that would be perfectly
legal ...

If you shouldn't use cast, you shouldn't use int. Which is a shame,
because I think int is defined to be "the natural length of the platform
you're running on" ie it's the most efficient integer variant to use.

Imho (as I've said before) *all* warnings should be explained or fixed.
Papering over the cracks by hiding them is likely to result sooner or
later in a real problem slipping through (like a compiler defining
sizeof(int) as 1 :-)

Cheers,
Wol

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fixes issue 40. (issue4801083)

[n . puttock]

On 2011/08/09 18:23:42, mike_apollinemike.com wrote:


Good call - I've uploaded a new patch that uses "accidental-grob"

(don't know

why I didn't think of that before...too much code on the brain!).


The latest patch set needs rebasing against master.

Cheers,
Neil

http://codereview.appspot.com/4801083/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Rewrite regtest mozart-hrn-3.ly (issue4811066)

[Neil Puttock]

On 9 August 2011 09:47, Phil Holmes  wrote:

> I said in a separate message that this isn't necessary.  The link is
> automatically converted to a clickable link.

This only works reliably with Adobe Reader.

Foxit produces an incorrect link: mutopia.org/It (it picks up the
start of the next line)

Neither okular nor evince produces a link.

Cheers,
Neil

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Neil Puttock]

On 9 August 2011 20:21, Reinhold Kainhofer  wrote:

> So having only 9 warnings in our codebase (four of which are in the
> lexer/parser, which hardly anyone of us really understands!) is amazing.

There are many more warnings (> 180) if you're compiling a 64-bit
binary.  They could be silenced via casting, but Han-Wen isn't in
favour of that approach (http://codereview.appspot.com/1724041/):

"* Why are all the casts there?  Is this a 64 bit compiler thing?  Lily compiles
virutally without warnings over here (core duo, gcc 4.4.4).  I think all the
casting hinders readability, so I propose to not add casts unless necessary.  If
the warnings bother you, add a targeted -Wno-xxx option to the Makefile.  I
doubt that there are any cases where there is the risk of a real error."

>> out/parser.cc:2392: warning: conversion to 'short int' from 'int'
>> may alter its value
>> /lily/lexer.ll:634: warning, rule cannot be matched
>> /lily/lexer.ll:637: warning, rule cannot be matched
>> /lily/lexer.ll:706: warning, -s option given but default rule can be
>> matched
>
> Anyone here who knows more about the lexer and the parser?

The parser.cc warning is from code generated by Bison.

I'm not sure about the two `rule cannot be matched' warnings, though
both lines have been there sice 1997 and removing them doesn't seem to
cause any problems.

Cheers,
Neil

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Graham Percival]

On Tue, Aug 09, 2011 at 09:21:26PM +0200, Reinhold Kainhofer wrote:
> Am Dienstag, 9. August 2011, 12:07:12 schrieb Phil Holmes:
> > at them. There are nine warnings from the code compiler:
> 
> And that number is really amazing and absolutely proves my point: Coders PAY 
> attention to warnings and usually fix them in the code they write.
> I would have expected way more compiler warnings in the C++ code!

We probably don't have all the gcc warning options enabled.
Hint: using -Wall does *not* get you all warnings!  (sometimes I
hate gcc)

> so, you really expect us developers to do the following when compiling:
> 
> 1) change some code
> 2) run make
> 3) WAIT until the build is finished (which takes a LONG time)
> 4) find the logfile
> 5) open the logfile
> 6) search the part of the logfile that is about the code you changed
> 7) Check the warnings
> 8) fix them  -> start at 1)

No, of course not.  I was thinking of:
1. change some code
2. make
3. make show-warnings


But more than that, I expect (or at least, hope) that everybody
will take a chill pill and stop talking about the build system.
We are **not** going to be discussing the code-building part of
make for another month or so.

After I've sent my income tax review material (apparently the
Canada Revenue Agency isn't happy with an online-only submission
which involves $20,000+ in tuition fees from a foreign
university!), I will prepare a proposal for the behaviour of
"make doc".


To clarify: we will **only** be discussing "make doc".  Not
"make bin".  Not "make clean".  Not "make".
Talking about anything else is a complete waste of time, because
we will start over from the very beginning in a month.

Cheers,
- Graham

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Sketch for fix of issue 307 (issue4813048)

[bordage . bertrand]

This needs an update to be applied.

Cheers,
Bertrand

http://codereview.appspot.com/4813048/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Line 722 of axis-group-interface.cc

[Neil Puttock]

On 8 August 2011 09:09, m...@apollinemike.com  wrote:
> Question: on line 722 of axis-group-interface.cc, I see:
>
>      while (i + 1 < elements.size ()
>             && scm_eq_p (elements[i + 1]->get_property 
> ("outside-staff-priority"), priority))
>
> Shouldn't this be:
>
>      while (i + 1 < elements.size ()
>             && to_boolean (scm_eq_p (elements[i + 1]->get_property 
> ("outside-staff-priority"), priority)))

Good catch.

The Guile docs suggest using scm_eqv_p for this case:

"Numbers and characters are not equal to any other object, but the
problem is they're not necessarily eq? to themselves either."

Cheers,
Neil

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Reinhold Kainhofer]

Am Dienstag, 9. August 2011, 12:07:12 schrieb Phil Holmes:
> I know you have many 10s of times more experience with lilypond than I do,
> and I agree with 99% of what you say...  But...
> 
> The truth is, no-one pays any attention to warnings during the build
> process.  If I grep the output of make for the word "warning" I get 380
> lines in the middle of 37,000 other lines of output.  Most of these
> warnings come from other parts of the build system, but no-one's looking
> at them. There are nine warnings from the code compiler:

And that number is really amazing and absolutely proves my point: Coders PAY 
attention to warnings and usually fix them in the code they write.
I would have expected way more compiler warnings in the C++ code!

To give you numbers: When I wrote the patch for loglevels, it took me several 
compiler errors and at least 5 compiler warnings (that I explicitly remember). 
So having only 9 warnings in our codebase (four of which are in the 
lexer/parser, which hardly anyone of us really understands!) is amazing.


> /flower/file-name.cc:100: warning: ignoring return value of 'char*
> getcwd(char*, size_t)', declared with attribute warn_unused_result
> /lily/glissando-engraver.cc:124: warning: comparison between signed and
> unsigned integer expressions
> /lily/lily-parser-scheme.cc:85: warning: ignoring return value of 'int
> chdir(const char*)', declared with attribute warn_unused_result
> /lily/lyric-combine-music-iterator.cc:224: warning: unused parameter 'sev'
> /lily/skyline.cc:395: warning: unused parameter 'a'


-) The "unused parameter" warnings are harmless and easy to fix. 
-) The "ignoring return value" ar a bit trickier, but should also be easy to
   fix. They are bad coding style, in particular:
o) the file-name.cc one returns a string with undefined contents if getcwd
   fails, which might either crash guile or lead to other bugs. 
o) the lily-parser-scheme.cc warning was also a programming error (if you
   used --output=dir/file, and dir/ had the wrong permissions, then
   lilypond would not print an error, but simply put the output file into
   the current directory!).

I'll post a patch for all of them.


> out/parser.cc:2392: warning: conversion to 'short int' from 'int'
> may alter its value
> /lily/lexer.ll:634: warning, rule cannot be matched
> /lily/lexer.ll:637: warning, rule cannot be matched
> /lily/lexer.ll:706: warning, -s option given but default rule can be
> matched 

Anyone here who knows more about the lexer and the parser?

> If warnings are there to prevent code errors, why have these not been
> fixed?

When coders work on some code, they usually only look at the warnings caused 
by their own code.


> In practice, displaying warnings on the console is a waste.  It's really
> far, far better to put them in a file, where concerned individuals can grep
> the file, open it in an editor and view it, etc.

so, you really expect us developers to do the following when compiling:

1) change some code
2) run make
3) WAIT until the build is finished (which takes a LONG time)
4) find the logfile
5) open the logfile
6) search the part of the logfile that is about the code you changed
7) Check the warnings
8) fix them  -> start at 1)

Currently, my workflow is:

1) change some code
2) run make
3) look at the console output until the file you changed is compiled (i.e.
   scrolls by)
4) check whether any warning is displayed 
   (terminal windows have toolbars, so they won't scroll away!)
5) fix them (meanwhile the build can continue!) -> start at 1)

Typically takes some 15 seconds total.

> I would have got nowhere
> understanding the build system unless I routinely redirected the build
> output to file.

For analyzing that's absolutely true.

> They're not being ignored.  They're not even being seen.  Please address my
> point of how you would see them in 37,000 lines of console output.

On the other hand: we have 37.000 lines of make output and 9 compiler warnings 
from the C-code. If each takes 2 lines, that increases the make output by 0.05 
percent (by 0.0005). Is this really a significant reduction to remove the 
warnings???


Cheers,
Reinhold

PS: I should also mention that many of the other 500 warnings from a clean 
"make" build are messages printed by lilypond in --verbose mode:
warning: Relocation: is absolute: 
argv0=/home/reinhold/lilypond/lilypond/out/bin/lilypond
PATH=/home/reinhold/lilypond/lilypond/out/bin (voranstellen)
PATH wird auf 
/home/reinhold/lilypond/lilypond/out/bin:/home/reinhold/lilypond/lilypond/lily/out:/home/reinhold/lilypond/lilypond/scripts/build/out:/home/reinhold/lilypond/lilypond/scripts/out:/home/reinhold/lilypond/lilypond/lily/out:/home/reinhold/lilypond/lilypond/scripts/build/out:/home/reinhold/lilypond/lilypond/scripts/out:/home/reinhold/.build/bin:/home/reinhold/.bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games::
 
gesetzt
Warnung: Verlagerung: Kompilier-Datenverzeichnis

Re: Progress on loose columns. (issue4841052)

[bordage . bertrand]

LGTM (still one trailing whitespace in the regtest).

Bertrand

http://codereview.appspot.com/4841052/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Should this be applied?

[Neil Puttock]

On 8 August 2011 12:43, David Kastrup  wrote:

> I have no clue what start and end are

start and end are column ranks, i.e., an index to the position of a
particular column in a score.  You can see them if you enable
debugging for columns:

\relative c' {
  c1
}

\layout {
  \context {
\Score
\override PaperColumn #'stencil = #ly:paper-column::print
\override NonMusicalPaperColumn #'stencil = #ly:paper-column::print
  }
}

> and I have no clue what pure is.

https://secure.wikimedia.org/wikipedia/en/wiki/Pure_function

> But get_pure_property takes start and end as additional arguments over
> get_property, and so does pure_is_visible.
>
> So from a mere text-matching hand-waving likelihood point of view, the
> above change seems plausible.
>
> Is there anybody that could shed light on this?

The only effect I can see is that break-visiibility won't be cached.
There are no pure break-visibility callbacks so the start/end args
will never be applied.

Cheers,
Neil

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fixes issue 40. (issue4801083)

[m...@apollinemike.com]

On Aug 9, 2011, at 7:15 PM, n.putt...@gmail.com wrote:

> http://codereview.appspot.com/4801083/diff/8001/lily/line-spanner.cc#newcode113
> lily/line-spanner.cc:113: Grob *acc = Note_column::accidentals
> (bound_grob->get_parent (X_AXIS));
> acc is an AccidentalPlacement, so should be renamed to avoid confusion.
> 
> As an AccidentalPlacement, it won't work very well with chords that have
> accidentals far enough away from the head the glissando ends on, or
> chord glissandos where some heads don't have accidentals:
> 
> \relative c, {
>  \clef bass
>  \override Glissando #'minimum-length = #6
>  \override Glissando #'springs-and-rods = #ly:spanner::set-spacing-rods
>  8[\glissando ]
> }
> 

Good call - I've uploaded a new patch that uses "accidental-grob" (don't know 
why I didn't think of that before...too much code on the brain!).
I've taken out all of that minimum-distance stuff.  The way the regtest is 
designed, it does not reproduce the snippet in Issue 40 to the letter but it 
reproduces it in spirit (I think) - it uses whole notes so that the glissando 
is visible.

Cheers,
MS


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Prevents lilypond from segfaulting with add-footer = ##f (issue4832046)

[n . puttock]

On 2011/08/09 06:47:11, MikeSol wrote:


Fix pushed as 435b36a3e6576cebf794d815ae7cc78b652180f9.


The warning message still needs tweaking:

warning (_ ("must have a footer to add footnotes"))

http://codereview.appspot.com/4832046/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fixes issue 40. (issue4801083)

[n . puttock]

http://codereview.appspot.com/4801083/diff/8001/input/regression/glissando-accidental.ly
File input/regression/glissando-accidental.ly (right):

http://codereview.appspot.com/4801083/diff/8001/input/regression/glissando-accidental.ly#newcode8
input/regression/glissando-accidental.ly:8: a1 \glissando cis
a1\glissando

http://codereview.appspot.com/4801083/diff/8001/lily/line-spanner.cc
File lily/line-spanner.cc (right):

http://codereview.appspot.com/4801083/diff/8001/lily/line-spanner.cc#newcode113
lily/line-spanner.cc:113: Grob *acc = Note_column::accidentals
(bound_grob->get_parent (X_AXIS));
acc is an AccidentalPlacement, so should be renamed to avoid confusion.

As an AccidentalPlacement, it won't work very well with chords that have
accidentals far enough away from the head the glissando ends on, or
chord glissandos where some heads don't have accidentals:

\relative c, {
  \clef bass
  \override Glissando #'minimum-length = #6
  \override Glissando #'springs-and-rods = #ly:spanner::set-spacing-rods
  8[\glissando ]
}

http://codereview.appspot.com/4801083/diff/8001/lily/line-spanner.cc#newcode380
lily/line-spanner.cc:380: "minimum-distance "
The docstring for this will need changing.

http://codereview.appspot.com/4801083/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Fixes issue 40. (issue4801083)

[Neil Puttock]

On 9 August 2011 07:41,   wrote:
> On 2011/08/09 05:08:56, hanwenn wrote:
>>
>> LGTM
>
>> note that image of the issue will also require a minimum distance
>
> setting,
>>
>> otherwise, the glissando will be shortened into a dot?
>
> Done.  New patchset uploaded.

This doesn't ensure the glissando's visible (and messes up two
tablature regtests).  Han-Wen probably means minimum-length (which
requires a springs-and-rods setting); alternatively, you could set
ragged-right = ##f.

Cheers,
Neil

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Carl Sorensen]

On 8/9/11 9:34 AM, "Phil Holmes"  wrote:

> - Original Message -
> From: "Wols Lists" 
> To: 
> Sent: Tuesday, August 09, 2011 4:26 PM
> Subject: Re: GOP-PROP 5: build system output (final)
> 
> 
>> On 09/08/11 11:07, Phil Holmes wrote:
>>> - Original Message - From: "Reinhold Kainhofer"
>>> 
>>> 

>>> 
>>> In practice, displaying warnings on the console is a waste.  It's really
>>> far, far better to put them in a file, where concerned individuals can
>>> grep the file, open it in an editor and view it, etc.  I would have got
>>> nowhere understanding the build system unless I routinely redirected the
>>> build output to file.
>> 
>> I'm afraid I'm with Reinhold. As a *programmer*, I consider it very bad
>> practice to ignore warnings. For the system to hide them from me, well !!!
> 
> 
> They're not being ignored.  They're not even being seen.  Please address my
> point of how you would see them in 37,000 lines of console output.

I think that what Reinhold and Wols are asking for is that *compiler*
warnings should be sent to the terminal under default behavior, not that all
37,000 lines of console output be sent to the terminal.

I hope we all agree that the result of make is currently too verbose for
effective use.

Phil wants to make it useful.  I hope we all agree that is a *good* thing.

I think the main disagreement is with the statement in GOP_prop_5 that "only
the results of make(1) will be echoed to the terminal."

I'd propose a different statement.  Something along the lines of

"Only the results of make(1) and the binary build section will be echoed to
the terminal.  The binary build section includes all of the gcc codebase, as
well as the automatically created python scripts."

I think I'd even be fine with defining the binary build section as anything
that is not the result of a call to lilypond or lilypond-book.

Thanks,

Carl



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Phil Holmes]

- Original Message - 
From: "Wols Lists" 

To: 
Sent: Tuesday, August 09, 2011 4:26 PM
Subject: Re: GOP-PROP 5: build system output (final)



On 09/08/11 11:07, Phil Holmes wrote:

- Original Message - From: "Reinhold Kainhofer"


[snip very well argued case]

Reinhold,

I know you have many 10s of times more experience with lilypond than I
do, and I agree with 99% of what you say...  But...

The truth is, no-one pays any attention to warnings during the build
process.  If I grep the output of make for the word "warning" I get 380
lines in the middle of 37,000 other lines of output.  Most of these
warnings come from other parts of the build system, but no-one's looking
at them. There are nine warnings from the code compiler:

/flower/file-name.cc:100: warning: ignoring return value of 'char*
getcwd(char*, size_t)', declared with attribute warn_unused_result
/lily/glissando-engraver.cc:124: warning: comparison between signed and
unsigned integer expressions
/lily/lily-parser-scheme.cc:85: warning: ignoring return value of 'int
chdir(const char*)', declared with attribute warn_unused_result
/lily/lyric-combine-music-iterator.cc:224: warning: unused parameter 
'sev'

/lily/skyline.cc:395: warning: unused parameter 'a'
/lily/lexer.ll:634: warning, rule cannot be matched
/lily/lexer.ll:637: warning, rule cannot be matched
/lily/lexer.ll:706: warning, -s option given but default rule can be
matched
out/parser.cc:2392: warning: conversion to 'short int' from 'int' may
alter its value

If warnings are there to prevent code errors, why have these not been
fixed?


Because they're known about and discounted as not very important? Nine
warnings, imho, is not very many for a code base the size of lilypond.


In practice, displaying warnings on the console is a waste.  It's really
far, far better to put them in a file, where concerned individuals can
grep the file, open it in an editor and view it, etc.  I would have got
nowhere understanding the build system unless I routinely redirected the
build output to file.


I'm afraid I'm with Reinhold. As a *programmer*, I consider it very bad
practice to ignore warnings. For the system to hide them from me, well !!!



They're not being ignored.  They're not even being seen.  Please address my 
point of how you would see them in 37,000 lines of console output.


--
Phil Holmes



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Wols Lists]

On 09/08/11 11:07, Phil Holmes wrote:
> - Original Message - From: "Reinhold Kainhofer"
> 
> 
> [snip very well argued case]
> 
> Reinhold,
> 
> I know you have many 10s of times more experience with lilypond than I
> do, and I agree with 99% of what you say...  But...
> 
> The truth is, no-one pays any attention to warnings during the build
> process.  If I grep the output of make for the word "warning" I get 380
> lines in the middle of 37,000 other lines of output.  Most of these
> warnings come from other parts of the build system, but no-one's looking
> at them. There are nine warnings from the code compiler:
> 
> /flower/file-name.cc:100: warning: ignoring return value of 'char*
> getcwd(char*, size_t)', declared with attribute warn_unused_result
> /lily/glissando-engraver.cc:124: warning: comparison between signed and
> unsigned integer expressions
> /lily/lily-parser-scheme.cc:85: warning: ignoring return value of 'int
> chdir(const char*)', declared with attribute warn_unused_result
> /lily/lyric-combine-music-iterator.cc:224: warning: unused parameter 'sev'
> /lily/skyline.cc:395: warning: unused parameter 'a'
> /lily/lexer.ll:634: warning, rule cannot be matched
> /lily/lexer.ll:637: warning, rule cannot be matched
> /lily/lexer.ll:706: warning, -s option given but default rule can be
> matched
> out/parser.cc:2392: warning: conversion to 'short int' from 'int' may
> alter its value
> 
> If warnings are there to prevent code errors, why have these not been
> fixed?

Because they're known about and discounted as not very important? Nine
warnings, imho, is not very many for a code base the size of lilypond.
> 
> In practice, displaying warnings on the console is a waste.  It's really
> far, far better to put them in a file, where concerned individuals can
> grep the file, open it in an editor and view it, etc.  I would have got
> nowhere understanding the build system unless I routinely redirected the
> build output to file.

I'm afraid I'm with Reinhold. As a *programmer*, I consider it very bad
practice to ignore warnings. For the system to hide them from me, well !!!

You can't always get rid of warnings, but when I was setting guidelines
for junior programmers I said

(a) The warning level should be jacked up to max
(b) if you don't understand a warning message, then you investigate
until you know whether it's safe to ignore or not, AND IF YOU CAN GET
RID OF IT, YOU DO.

Given (a), why bother turning *up* the warning level, if you're then
going to hide the warnings? As for (b), ime some warnings can't be
suppressed, I know we had a vicious circle where trying to fix one
warning just caused another one.

And backing up Reinhold about the other point of "warnings usually
indicate a problem" I couldn't agree more. I took over maintenance of a
program that "mostly worked but was flaky". It had no warning level set,
and so defaulted to almost none. Over a period of a few months (it was a
lot or work) I jacked that up to max and fixed all the warnings. Yes
they were nearly all bugs. And while I could never confirm that this
programming mistake caused that problem, the program became much more
reliable as a result. That "out/parser" is a perfect example - it *may*
be innocuous, or it *may* be a serious problem. It really ought to be
checked out and fixed, and if I were a boss, I would have no qualms
whatever about saying "fix it". In fact, looking closer, I would say
*most* of those warnings should be fixed - if they're innocuous they're
easy to fix the code to make them go away, and if they're not innocuous
- well, lily could easily blow up.

Cheers,
Wol

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Progress on loose columns. (issue4841052)

[m...@apollinemike.com]

On Aug 9, 2011, at 4:30 PM, bordage.bertr...@gmail.com wrote:

> What if we want a larger padding?
> In your regtest, the clef collides with the third notehead when we set
> the padding with:
> \override Score.SpacingSpanner #'left-loose-column-padding = #2
> 

Hm...good question!  I've uploaded a new patch set that addresses this.
That said, if you want more padding, use \override Score.NonMusicalPaperColumn 
#'allow-loose-spacing = ##f.  It is a more elegant way to get a column into the 
spacing engine.


> There's also two trailing whitespaces.
> 

Thanks for spotting these!  I'll scrub them.

Cheers,
MS
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Progress on loose columns. (issue4841052)

[bordage . bertrand]

What if we want a larger padding?
In your regtest, the clef collides with the third notehead when we set
the padding with:
\override Score.SpacingSpanner #'left-loose-column-padding = #2

There's also two trailing whitespaces.

Regards,
Bertrand


http://codereview.appspot.com/4841052/diff/6001/input/regression/loose-column-spacing.ly
File input/regression/loose-column-spacing.ly (right):

http://codereview.appspot.com/4841052/diff/6001/input/regression/loose-column-spacing.ly#newcode13
input/regression/loose-column-spacing.ly:13: \clef treble g'' fis,
trailing whitespace.

http://codereview.appspot.com/4841052/diff/6001/lily/spacing-loose-columns.cc
File lily/spacing-loose-columns.cc (right):

http://codereview.appspot.com/4841052/diff/6001/lily/spacing-loose-columns.cc#newcode140
lily/spacing-loose-columns.cc:140:
trailing whitespace.

http://codereview.appspot.com/4841052/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Progress on loose columns. (issue4841052)

[mtsolo]

Hey all,

I'm pretty sure this is the best way to get around the loose column
issue for now.

The problem (I think) is that some invisible BarLine grobs act as
alignment markers and thus can be slid well left of the two grobs they
were originally supposed to fall between.  A distinction needs to be
made between loose columns that are actually seen on the page and loose
columns that are used as spacing aids.  The current code assumes that
bar-lines are the only loose columns that can fall into this spacing-aid
category and simply omits them from the new algorithm.  It may be better
down the road to change the way volta brackets are attached to bounds
(there is a note in the volta-engraver to do this).

Cheers,
MS

http://codereview.appspot.com/4841052/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Search box changes

[Wols Lists]

On 09/08/11 09:51, Phil Holmes wrote:
> - Original Message - From: "Wols Lists" 
> To: 
> Sent: Tuesday, August 09, 2011 12:12 AM
> Subject: Re: Search box changes
> 
> 
>> On 08/08/11 16:04, Phil Holmes wrote:
>>> - Original Message - From: "Graham Percival"
>>> 
>>> To: "Phil Holmes" 
>>> Cc: 
>>> Sent: Sunday, August 07, 2011 11:45 PM
>>> Subject: Re: Search box changes
>>>
>>>
 On Sun, Aug 07, 2011 at 05:33:45PM +0100, Phil Holmes wrote:
> I've prepared some changes for the web search box in line with what
> I suggested in
> http://code.google.com/p/lilypond/issues/detail?id=1806 - screenshot
> attached.

 I'd need to look at the effects of the patch to see how it behaves
 in a 800-pixel wide screen.
>>>
>>> I didn't expect it to, but it looks fine. Image attached.  I would
>>> actually propose not supporting screens this narrow - even netbooks
>>> typically run 1024x600.
>>
>> And then people like me get royally pee-d off!
>>
>> I HATE HATE HATE screens that only work when maximised. I typically have
>> a whole bunch of programs running at once, and try and arrange my screen
>> so that *all* of them are partially visible *all* the time.
>>
>> Anything that tries to force me to run full-screen generally annoys me
>> so much that I stop using it.
>>
>> If it needs that much screen estate, fair enough, but if it doesn't need
>> it don't use it! Let the USER decide how much screen they want to give
>> it, don't demand that it be allowed to take the lot!
>>
>> Cheers,
>> Wol
> 
> 
> It doesn't only work when maximised.  Like just about every windowed
> application on the planet, as you reduce the size of the window, the
> furniture is squeezed until, eventually, much of it becomes unreadable
> or disappears from view.  You choose the play-off between how much real
> estate it takes and how much detail you want displayed.  But I remain of
> the opinion that we should not make design decisions based on the need
> to support extremely outdated hardware.
> 
I know. I'm not saying you should support outdated hardware, and I'm
quite happy for you to take all the space you need. BUT what I AM saying
is "don't assume you've got the entire screen to yourself!"

The number of pixels you have on screen is irrelevant. What matters is
the number of pixels the user can spare. When I'm "pond"ing, typically I
will have a kate window with my .ly, one okular window with my pdf
output, maybe another window with the scanned original. And with my
modern widescreen monitor, that gives them an average of 400 pixels
each. Now you're saying that I really need at least a thousand to view
your screen!? WTF!?

As I say, use all the space you need. But just remember, on my *modern*
*widescreen* monitor, I've only got 300 pixels to spare to you ... :-)

Cheers,
Wol

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 5: build system output (final)

[Phil Holmes]

- Original Message - 
From: "Reinhold Kainhofer" 


[snip very well argued case]

Reinhold,

I know you have many 10s of times more experience with lilypond than I do, 
and I agree with 99% of what you say...  But...


The truth is, no-one pays any attention to warnings during the build 
process.  If I grep the output of make for the word "warning" I get 380 
lines in the middle of 37,000 other lines of output.  Most of these warnings 
come from other parts of the build system, but no-one's looking at them. 
There are nine warnings from the code compiler:


/flower/file-name.cc:100: warning: ignoring return value of 'char* 
getcwd(char*, size_t)', declared with attribute warn_unused_result
/lily/glissando-engraver.cc:124: warning: comparison between signed and 
unsigned integer expressions
/lily/lily-parser-scheme.cc:85: warning: ignoring return value of 'int 
chdir(const char*)', declared with attribute warn_unused_result

/lily/lyric-combine-music-iterator.cc:224: warning: unused parameter 'sev'
/lily/skyline.cc:395: warning: unused parameter 'a'
/lily/lexer.ll:634: warning, rule cannot be matched
/lily/lexer.ll:637: warning, rule cannot be matched
/lily/lexer.ll:706: warning, -s option given but default rule can be matched
out/parser.cc:2392: warning: conversion to 'short int' from 'int' may alter 
its value


If warnings are there to prevent code errors, why have these not been fixed?

In practice, displaying warnings on the console is a waste.  It's really 
far, far better to put them in a file, where concerned individuals can grep 
the file, open it in an editor and view it, etc.  I would have got nowhere 
understanding the build system unless I routinely redirected the build 
output to file.


That said, I am absolutely and completely in agreement with you that, should 
there be a build error, it should be immediately apparent that it has failed 
and what caused it.


Can I take this opportunity to request specific feedback on viewing the 
output of make?  It's trivially simple to turn off with make -s, so it would 
be my proposal not to try to hide it in any other way, but to ask anyone who 
wished to lose it to use the -s option.  Does that sound sensible?


--
Phil Holmes



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

RE: New Binary for LilyPond on MacOSX 10.7 available

[James Lowe]

Hello,

)-Original Message-
)From: Hans Aberg [mailto:haber...@telia.com]
)Sent: 08 August 2011 23:12
)To: James Lowe
)Cc: lilypond-devel (lilypond-devel@gnu.org); LilyPond User (lilypond-
)u...@gnu.org)
)Subject: Re: New Binary for LilyPond on MacOSX 10.7 available
)Importance: Low
)
)On 8 Aug 2011, at 23:11, James Lowe wrote:
)
)> For some reason Hans won't email the group ;)
)
)So then don't forward my mail to the lists! - The reason is that I can't verify
)it, so I do not want unreliable information spread around. Wait until you
)find someone who can do it.

Well don't reply to me directly when they come from the lists then!

I was just trying to help. 

I wasn't sure if

1. You had seen the response. This upload had been available for a while and we 
had had no confirmation.
2. You had made a mistake by sending to me directly. This happens all the time.

All we needed was for someone who had the problem to run the new binary and see 
if they still had the problem. That was all.

However next time I might not bother.

Thanks

James




___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Search box changes

[Phil Holmes]

- Original Message - 
From: "Wols Lists" 

To: 
Sent: Tuesday, August 09, 2011 12:12 AM
Subject: Re: Search box changes



On 08/08/11 16:04, Phil Holmes wrote:

- Original Message - From: "Graham Percival"

To: "Phil Holmes" 
Cc: 
Sent: Sunday, August 07, 2011 11:45 PM
Subject: Re: Search box changes



On Sun, Aug 07, 2011 at 05:33:45PM +0100, Phil Holmes wrote:

I've prepared some changes for the web search box in line with what
I suggested in
http://code.google.com/p/lilypond/issues/detail?id=1806 - screenshot
attached.


I'd need to look at the effects of the patch to see how it behaves
in a 800-pixel wide screen.


I didn't expect it to, but it looks fine. Image attached.  I would
actually propose not supporting screens this narrow - even netbooks
typically run 1024x600.


And then people like me get royally pee-d off!

I HATE HATE HATE screens that only work when maximised. I typically have
a whole bunch of programs running at once, and try and arrange my screen
so that *all* of them are partially visible *all* the time.

Anything that tries to force me to run full-screen generally annoys me
so much that I stop using it.

If it needs that much screen estate, fair enough, but if it doesn't need
it don't use it! Let the USER decide how much screen they want to give
it, don't demand that it be allowed to take the lot!

Cheers,
Wol



It doesn't only work when maximised.  Like just about every windowed 
application on the planet, as you reduce the size of the window, the 
furniture is squeezed until, eventually, much of it becomes unreadable or 
disappears from view.  You choose the play-off between how much real estate 
it takes and how much detail you want displayed.  But I remain of the 
opinion that we should not make design decisions based on the need to 
support extremely outdated hardware.


--
Phil Holmes



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Rewrite regtest mozart-hrn-3.ly (issue4811066)

[Phil Holmes]

- Original Message - 
From: "Graham Percival" 

To: "James Lowe" 
Cc: "Phil Holmes" ; ; 
; ; 
; 

Sent: Monday, August 08, 2011 10:32 PM
Subject: Re: Rewrite regtest mozart-hrn-3.ly (issue4811066)



Since you pushed it, James, could you correct the \typewriter to
\with-url ?  Before you push, compile it on the command-line to
make sure you have the right syntax and you have a clickable link.

Cheers,
- Graham



I said in a separate message that this isn't necessary.  The link is 
automatically converted to a clickable link.   \with-url allows you to make 
a link with text different from the link, which we don't want here.


--
Phil Holmes


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Adds a glyph for tied lyrics. (issue4808074)

[bordage . bertrand]

So do I, but a shorter tie would collide with commas. The best solution
is probably to shorten the tie and lower it according to the Y-extent of
the covered letters. But this requires much more work.
Maybe having two sizes of lyric ties would also be a good idea: a short
one and the default one.

Bertrand

http://codereview.appspot.com/4808074/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Ugliness in the Learning Manual

[Phil Holmes]

- Original Message - 
From: "Maximilian Albert" 

To: "Phil Holmes" 
Cc: "Graham Percival" ; "David Kastrup" 
; 

Sent: Monday, August 08, 2011 9:36 PM
Subject: Re: Ugliness in the Learning Manual


Hi all,


2) list all environment variables used (both internally and
externally) in the build system in the CG.

Advantage: at least this knowledge is written down somewhere.
Disadvantage: the list will not be kept up-to-date. (don't argue;
there's absolutely nothing you can say that will make me believe
that everybody will keep it up-to-date over the next 20 years)


Think it's quite a bit of work, too.


I don't know anything about the build system, but isn't it possible to
do this automatically? Say, if you rename all lilypond-related
environment variables so that they start with LILY_ then why can't you
say something like 'env | grep LILY_' and put the output into the CG?
Please ignore if this doesn't make any sense, I haven't followed the
thread closely so I may be missing something.

==

The issue arose because texi2pdf has its own environment variable pointing 
to where texinfo.tex is located.  So you have to use the exact name expected 
by texi2pdf - preceding it with LILY_ would mean it would ignore the 
variable.


--
Phil Holmes



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: GOP-PROP 8: issue priorities (radical update)

[Trevor Daniels]

Jan Warchoł wrote Tuesday, August 09, 2011 12:27 AM



2011/8/8 Trevor Daniels :


Graham Percival wrote Monday, August 08, 2011 6:06 AM

Type-critical:
* anything which stops contributors from helping out (e.g.
lily-git.tcl not working, source tree(s) not being
available). To limit this scope of this point, we will
assume that the contributor is using the latest lilydev and
has read the relevant part(s) of the Contributor’s Guide.


I agree this is important, but I don't see why it
should prevent a new release per se.


Because having some issue officially block stable release is the 
only

way of seriously pushing developers to fix it?


Wouldn't work.  Few, if any, developers use lily-git.tcl so
are unlikely to be in a position to fix it.

Developers seem to become more productive when
releases are make frequently.  If releases stop, for whatever
reason, development slows down. So if this is intended as
a means of coercion I think it is misguided.

Trevor



-
No virus found in this message.
Checked by AVG - www.avg.com
Version: 10.0.1392 / Virus Database: 1520/3822 - Release Date: 08/08/11


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel