indent indent indent
http://codereview.appspot.com/4490045/diff/25003/lily/tie-engraver.cc
File lily/tie-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/25003/lily/tie-engraver.cc#newcode189
lily/tie-engraver.cc:189: /*
On 2011/06/14 23:07:54, Graham Percival wrote:
these
I've identified a few more questionable lines, although maybe you should
wait for a lilypond C++ expert to look at them.
Now do you see why I was apologizing so much for not starting the C++
GOP question this week? this stuff is ridiculous. :/
2011/6/15 percival.music...@gmail.com:
I've identified a few more questionable lines, although maybe you should
wait for a lilypond C++ expert to look at them.
Well then, i'm stuffed.
If you don't want to make an exception to the indentation rules for
this issue, i'm not going to touch
All indentation should be done with spaces.
This is why I changed some tabs to spaces - sorry if this caused
confusion. I suggest that someone who knows the unwritten rules of
indentation finalizes the format issues.
http://codereview.appspot.com/4490045/
W dniu 15 czerwca 2011 00:49 użytkownik Carl Sorensen
c_soren...@byu.edu napisał:
On 6/14/11 4:19 PM, Janek Warchoł lemniskata.bernoull...@gmail.com
wrote:
I'm disheartened by the idea of reverting tab-space conversion,
because CG 10.3.2 says All indentation should be done with spaces.
On Wed, Jun 15, 2011 at 01:02:07PM +0200, Janek Warchoł wrote:
2011/6/15 percival.music...@gmail.com:
I've identified a few more questionable lines, although maybe you should
wait for a lilypond C++ expert to look at them.
Well then, i'm stuffed.
yeah.
Enough screwing around. Send me
2011/6/15 Graham Percival gra...@percival-music.ca:
On Wed, Jun 15, 2011 at 01:02:07PM +0200, Janek Warchoł wrote:
2011/6/15 percival.music...@gmail.com:
I've identified a few more questionable lines, although maybe you should
wait for a lilypond C++ expert to look at them.
Well then, i'm
On Wed, Jun 15, 2011 at 02:05:10PM +0200, Janek Warchoł wrote:
Patches attached. Rebased from 12 to 5 commits; i kept
aaa!! indent commit for historical reasons. I can
rebase more if you want.
Nah, I'm fine. Rebased down to 1, including some more whitespace
changes. Doing a
I've pushed a modified version of this.
Given the confusion about our C++ style, I removed a few changes which
were probably good. In some places I added an extra (pointless) space;
in other places I changed a tab character to a space to match the
existing (broken) indentation, etc. The goal
2011/6/15 percival.music...@gmail.com:
I've pushed a modified version of this.
wOOt!
YEEHAW!
Hooray!
Given the confusion about our C++ style, I removed a few changes which
were probably good. In some places I added an extra (pointless) space;
in other places I changed a tab character to a
New patch set uploaded, i think all formatting issues are resolved.
Should i run the regtests again?
http://codereview.appspot.com/4490045/diff/20001/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
On 2011/06/14 20:28:14, Janek Warchol wrote:
New patch set uploaded, i think all formatting issues are resolved.
Should i run
the regtests again?
LGTM, but yes please, run the regtests.
http://codereview.appspot.com/4490045/
___
lilypond-devel
2011/6/14 percival.music...@gmail.com:
On 2011/06/14 20:28:14, Janek Warchol wrote:
New patch set uploaded.
Should i run the regtests again?
LGTM, but yes please, run the regtests.
Regtests clean.
http://codereview.appspot.com/4490045/
___
http://codereview.appspot.com/4490045/diff/30001/lily/tie-engraver.cc
File lily/tie-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/30001/lily/tie-engraver.cc#newcode157
lily/tie-engraver.cc:157: maybe should check positions too.
looks like tab - space conversion going on
Hi Janek,
To save time fixing indentation, if you have emacs installed there's a
script which will nitpick the code:
scripts/auxiliar/fixcc.py
http://lilypond.org/doc/v2.15/Documentation/contributor/indentation
Cheers,
Neil
http://codereview.appspot.com/4490045/
http://codereview.appspot.com/4490045/diff/30001/lily/tie-engraver.cc
File lily/tie-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/30001/lily/tie-engraver.cc#newcode157
lily/tie-engraver.cc:157: maybe should check positions too.
On 2011/06/14 21:36:59, Neil Puttock wrote:
On Tue, Jun 14, 2011 at 09:43:56PM +, n.putt...@gmail.com wrote:
To save time fixing indentation, if you have emacs installed there's a
script which will nitpick the code:
scripts/auxiliar/fixcc.py
http://lilypond.org/doc/v2.15/Documentation/contributor/indentation
Unfortunately that
On 14 June 2011 22:57, Graham Percival gra...@percival-music.ca wrote:
On Tue, Jun 14, 2011 at 09:43:56PM +, n.putt...@gmail.com wrote:
To save time fixing indentation, if you have emacs installed there's a
script which will nitpick the code:
scripts/auxiliar/fixcc.py
2011/6/14 percival.music...@gmail.com
http://codereview.appspot.com/4490045/diff/30001/lily/tie-engraver.cc#newcode157
lily/tie-engraver.cc:157: maybe should check positions too.
On 2011/06/14 21:36:59, Neil Puttock wrote:
looks like tab - space conversion going on here (and below);
please
On Wed, Jun 15, 2011 at 12:19:01AM +0200, Janek Warchoł wrote:
2011/6/14 percival.music...@gmail.com
and Janek: you are probably quite
confused (and maybe disheartened) by this indentation problem, because
the current indentation policy used in lilypond is completely braindead
and
Thanks to Neil's help, it's done.
http://codereview.appspot.com/4490045/diff/30001/lily/tie-engraver.cc
File lily/tie-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/30001/lily/tie-engraver.cc#newcode157
lily/tie-engraver.cc:157: maybe should check positions too.
On 2011/06/14
On 6/14/11 4:19 PM, Janek Warchoł lemniskata.bernoull...@gmail.com
wrote:
2011/6/14 percival.music...@gmail.com
and Janek: you are probably quite
confused (and maybe disheartened) by this indentation problem, because
the current indentation policy used in lilypond is completely braindead
and
I have send the patch set to Janek for upload.
http://codereview.appspot.com/4490045/diff/20001/lily/tie-engraver.cc
File lily/tie-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/20001/lily/tie-engraver.cc#newcode163
lily/tie-engraver.cc:163: /*
On 2011/06/01 21:21:25, Neil
http://codereview.appspot.com/4490045/diff/20001/lily/tie-engraver.cc
File lily/tie-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/20001/lily/tie-engraver.cc#newcode163
lily/tie-engraver.cc:163: /*
On 2011/06/13 22:44:37, karin.hoethker wrote:
On 2011/06/01 21:21:25, Neil
LGTM.
http://codereview.appspot.com/4490045/diff/20001/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/20001/lily/completion-note-heads-engraver.cc#newcode204
lily/completion-note-heads-engraver.cc:204:
New patch set from Karin uploaded.
http://codereview.appspot.com/4490045/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc#newcode207
lily/completion-note-heads-engraver.cc:207:
LGTM
Carl
http://codereview.appspot.com/4490045/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On 2011/05/29 18:36:20, Janek Warchol wrote:
New patch set from Karin uploaded.
thanks, looks marvelous to me.
Pal
http://codereview.appspot.com/4490045/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
On 2011/05/28 16:13:43, benko.pal wrote:
aargh, that's not too readable.
what I actually suggest is replacing lines 204-207 of
http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
204 if
http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc#newcode207
lily/completion-note-heads-engraver.cc:207:
New patches from Karin uploaded.
I've got a warning while applying patch 0003 (about autosplit-remainder)
to my repository, but it looks like its not fatal... I don't understand
what it means, can anyone more experienced take a look and say whether
there is anything to worry about?
aargh, that's not too readable.
what I actually suggest is replacing lines 204-207 of
http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
204 if ((left_to_do_ - note_dur.get_length ()) Rational (0))
On 2011/05/29 08:53:30, benko.pal wrote:
I must miss something, to me it's still a boolean.
and (still to me) it's not an inline conditional, but
an assignment of a boolean expression to a boolean
variable.
No, it is I that missed something. I'm sorry for the noise.
Thanks,
Carl
http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc#newcode207
lily/completion-note-heads-engraver.cc:207:
2011/5/28 carl.d.soren...@gmail.com:
On 2011/05/28 16:13:43, benko.pal wrote:
aargh, that's not too readable.
what I actually suggest is replacing lines 204-207 of
http://codereview.appspot.com/4490045/diff/12001/lily/completion-note-heads-engraver.cc
File
On 2011/05/29 12:14:18, Carl wrote:
On 2011/05/29 08:53:30, benko.pal wrote:
I must miss something, to me it's still a boolean.
and (still to me) it's not an inline conditional, but
an assignment of a boolean expression to a boolean
variable.
I just used inline conditionals as an example
karin.hoeth...@googlemail.com writes:
I just used inline conditionals as an example of a code style where
conditions are inlined. More generally, there seem to be two views on
readability:
One could be summarized as Don't do more than one thing in one line
(for example inline conditions
You are right, in version 2.13 even shifted tuplets like c'8 \times 2/3
{ c'4 c'4 c'4} c'4. are split into correct durations.
It seems to me that people are rather in favour of the shorter solution
using a boolean flag instead of the duration autosplit-remainder.
On 2011/05/25 13:25:27, karin.hoethker wrote:
You are right, in version 2.13 even shifted tuplets like c'8 \times
2/3 { c'4
c'4 c'4} c'4. are split into correct durations.
It seems to me that people are rather in favour of the shorter
solution using a
boolean flag instead of the duration
2011/5/9 karin.hoeth...@googlemail.com:
http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc#newcode204
http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc#newcode204
lily/completion-note-heads-engraver.cc:204:
http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc#newcode204
lily/completion-note-heads-engraver.cc:204:
LGTM, except for non-standard indentation which should be
corrected, although I've tested it only on the examples in
the regression tests.
Trevor
http://codereview.appspot.com/4490045/diff/1/lily/completion-note-heads-engraver.cc
File lily/completion-note-heads-engraver.cc (right):
Reviewers: karin_hoethker.de, Graham Percival, lemzwerg,
carl.d.sorensen_gmail.com,
Message:
Finally i'm back in Warsaw and i'm uploading Karin's patch. Sorry for
the delay :(
Here's an archive with test file and output pdfs from before and after
the patch:
45 matches
Mail list logo
