[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: some comments and complaints on the code (issue 5651069)

From: janek . lilypond
Subject: Re: some comments and complaints on the code (issue 5651069)
Date: Sat, 11 Feb 2012 10:31:57 +0000

On 2012/02/10 23:49:24, Carl wrote:
Thanks for taking this on, Janek.

I don't know what the response will be to for_UP_and_DOWN(d).  The
last time
somebody proposed a change, it was resisted because the do{}
flip(d)!=UP idiom
seemed simple enough to be acceptable.

It took us a while to figure out what's going on with the do{}
flip(d)!=UP thing.
If it was up to me, i'd just write everything twice, following the rule
"1, 2, many" :)

Some of your comments will be better addressed by Luke - i leave them to

Thanks for review!
File lily/ (right):
lily/ * @return A vector of
On 2012/02/11 00:09:00, joeneeman wrote:
Please don't just comment on the type (unless it's SCM in which case
you can say
which scheme type it is).

Well, the main point of the comment is not describing parameters, but
the function itself.  Believe me or not, we spent 10 minutes figuring
out _what the hell_ are apes doing here and whether there are any
bananas involved.
It's stupid, i know.  But there must exist a way of writing code that is
understandable for the newcomers after second reading, not after 10
File lily/ (right):
lily/ shift_amount *= 1.25;
this must be a mistake actually.  We didn't intend to do anything with
the working code yet.
lily/ ->set_property ("direction", scm_from_int
On 2012/02/10 23:49:24, Carl wrote:
I think the indentation in the old code is correct.  The -> should be
since it's not the start of a statement.


But if this spacing is what's produced by, then we should
keep it.

It was done by fixcc indeed.
lily/ for_UP_and_DOWN (d) // please, make a
comment to this loop (better than the above one...)
On 2012/02/10 23:49:24, Carl wrote:
To whom is the "please" directed?  Is there anybody who is better than
you right
now to comment the loop?

Yes: the author of the code.  There are also other people in our team
who will understand this at second reading; i still don't understand how
exactly this works.
(of course i don't demand that anyone does anything about it)
lily/ {
On 2012/02/10 23:49:24, Carl wrote:
I don't know that we have a specification for this, or if it can be
handled by, but for consistency with the way we indent braces with loops
and if,
the braces should be indented two spaces, IMO.


However, the result was produced with fixcc.  At the moment i don't know
how to fix fixcc; i've looked at its code and it contains a lot of

('([\w\)\]]) *(--|\+\+)', '\\1\\2')

which look nice but i have absolutely no idea how they work, and no time
to learn at the moment :(

reply via email to

[Prev in Thread] Current Thread [Next in Thread]