On 2020/05/09 18:25:03, lemzwerg wrote:
>
https://codereview.appspot.com/560030044/diff/557800043/scm/define-context-properties.scm#newcode609
> scm/define-context-properties.scm:609: merge rests when this is set to
true.")
> While you are at it, please fix indentation (i.e., no indentation for
Reviewers: lemzwerg, thomasmorley651,
Message:
On 2020/05/09 20:17:09, thomasmorley651 wrote:
> Not sure about chordNameExceptionsFull and chordNameExceptionsPartial.
> I thought our deprecating policy would be to wait for 1 or even 2
stable
> versions before deleting from source.
Actually, all
On 2020/05/09 18:30:04, Dan Eble wrote:
> Yes, that's what I was trying to explain.
> internal_line_count() is relevant to one branch only.
Oh, I see your point now. I thought I could get away with line_count
only, but indeed this fails to account for (hypothetical) cases where
the line-positions
On 2020/05/09 12:47:29, Dan Eble wrote:
>
https://codereview.appspot.com/576090043/diff/569740045/lily/staff-symbol.cc
> File lily/staff-symbol.cc (right):
>
>
https://codereview.appspot.com/576090043/diff/569740045/lily/staff-symbol.cc#newcode116
> lily/staff-symbol.cc:116: return
On 2020/05/09 01:42:54, Dan Eble wrote:
> I looked around. This is the only place I found using just the size
of the
> vector. I don't think it's worth adding and maintaining a
> Staff_symbol::line_count just for this case.
Well, it wouldn’t be "adding and maintaining" since it already exists
https://codereview.appspot.com/576090043/diff/557790043/lily/multi-measure-rest.cc
File lily/multi-measure-rest.cc (right):
https://codereview.appspot.com/576090043/diff/557790043/lily/multi-measure-rest.cc#newcode268
lily/multi-measure-rest.cc:268: bool oneline = (!staff) ||
Reviewers: lemzwerg,
Message:
On 2020/04/30 19:33:03, lemzwerg wrote:
> LGTM, thanks!
Thanks Werner! I added your suggestions and pushed it as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=ee56da0f7a68cb6d6c5cd8531a566d81abdc5e59
V.
Description:
Doc: mention tuplet-slur in Changes
On 2020/05/01 12:12:40, dak wrote:
> That being said, the situation regarding Scorio, a proprietary entity
using Free
> Software as a component of delivering a web-based service with
non-disclosed
> components, is _exactly_ the reason Artifex chose the AGPL as a basis
for their
> business model
Thanks, this should be pushed immediately I think.
(Man, did this regtest give us a lot of headaches.)
https://codereview.appspot.com/567500043/diff/577810043/input/regression/font-name-add-files.ly
File input/regression/font-name-add-files.ly (right):
On 2020/04/24 07:20:55, hahnjo wrote:
> If there are no technical objections, I think we should move
> forward and let something as big sit around for too long.
Just for clarification, did you miss a "not" in that sentence?
Cheers,
- V.
https://codereview.appspot.com/573670043/
On 2020/04/21 07:29:27, hahnjo wrote:
> If nobody disagrees, I'd like to push this rather sooner than later
LGTM! ("We" should have thought of it even sooner.)
V.
https://codereview.appspot.com/557720043/
Reviewers: dak,
Message:
On 2020/04/20 12:05:15, dak wrote:
>
https://codereview.appspot.com/573730044/diff/583810043/input/regression/font-name-add-files.ly
> File input/regression/font-name-add-files.ly (right):
>
>
On 2020/04/11 09:44:26, Valentin Villenave wrote:
> What could be the cause?
So, pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=0cfef7069e86f85ad392f5c4bc63f6c4801aa2c9
V.
https://codereview.appspot.com/557640051/
Pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=b9255b50886e5e0de802a26fd486c908b0b55c0f
https://codereview.appspot.com/551650043/
On 2020/04/09 19:52:47, c_sorensen wrote:
> I'm OK to have you push it, but I'd like to hear from those involved
in the
> release whether or not the steps in the CG were followed.
Well, I introduced new CG additions to that effect a few weeks ago:
https://codereview.appspot.com/557640051/diff/565870043/input/regression/font-name-add-files.ly
File input/regression/font-name-add-files.ly (right):
https://codereview.appspot.com/557640051/diff/565870043/input/regression/font-name-add-files.ly#newcode244
Reviewers: ,
Message:
This seems trivial enough; may I just push onto staging?
Cheers,
V.
Description:
Web: update documentation symlinks, and use https in RewriteRules
Please review this at https://codereview.appspot.com/551650043/
Affected files (+4, -4 lines):
M
Reviewers: ,
Message:
Pushed as
http://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=f5f907599ce88d3d610483fa42fa78be12f53d2e
Description:
Web: update CG instructions, `attic’ page and THANKS for 2.18
Please review this at https://codereview.appspot.com/559730043/
Affected files (+56, -45
On 2020/03/30 18:56:27, thomasmorley651 wrote:
> tmpnam is deprecated in guile-3.0.2
(sigh) Nice catch!
> Why introducing it, we would need to move away from it later anyway?
Well, I *would* have used mkstemp, but I need to create a directory
(rather than a file), and there’s no mkdtemp in
Reviewers: lemzwerg,
Message:
On 2020/03/30 16:06:08, lemzwerg wrote:
> Is it guaranteed that we can create this directory? What about srcdir
!=
> builddir?
Yes. It’s a (tmpnam) name, located in /tmp or $TMPDIR or whatever:
On 2020/03/27 19:00:08, jean wrote:
> I can see no situation where the current value of keepAliveInterfaces
does a
> better
> job than the one with dynamic-interface.
OK, you both make a valid point. Then it LGTM as well.
V.
https://codereview.appspot.com/553760043/
On 2020/03/26 12:52:29, wl_gnu.org wrote:
> Never seen that, AFAIK. Can you provide a link to such a score?
I can certainly imagine that sort of things, in an orchestral score
simple enough that all instruments share the same dynamics (as was
common until the late 18th century), and where the
Reviewers: lemzwerg, Trevor Daniels,
Message:
Thanks guys, it means a lot! (Particularly from Trevor, who opened the
tracker page in the first place.)
Since I had both your LGTMs, I’ve pushed my patch onto staging as
Reviewers: lemzwerg,
Message:
On 2019/04/26 17:13:12, lemzwerg wrote:
LGTM, thanks!
Thanks Werner, pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=bc46cc1fef7eecd0a64ac60223d40e98d79c9958
I think this sentence should end with `... be set to', dropping the
word `use'.
On 2019/04/06 22:06:22, Carl wrote:
However, this will require more refactoring. I don't believe we
should hold off
on this patch until we can get that part of it done better. This
patch has
lanquished long enough. IMO we should just push it as-is and get it
in the code
base.
Agreed.
Hi Carl,
I appreciate you taking the time to rework this patch, does it mean
you’re intending to shepherd Charles’ work until it gets merged?
In addition to Paul’s comments which you’ve nicely addressed, I had a
few additional ones below, on other aspects of Charles’ approach (and
taking into
Hi Werner, that’s a very nice patch done in a quite methodical way. I
just have a question:
https://codereview.appspot.com/564590043/diff/546570043/Documentation/notation/staff.itely
File Documentation/notation/staff.itely (left):
On 2019/03/13 08:51:52, Valentin Villenave wrote:
Further simplification; add CHANGES item.
Pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=464cf0d8111976c63fe778f358ddbe28bce532a8
Thanks!
https://codereview.appspot.com/572520044/
On 2019/03/12 22:15:48, dak wrote:
https://codereview.appspot.com/572520044/diff/544550044/Documentation/notation/input.itely#newcode246
Documentation/notation/input.itely:246: \paper @{ @dots{} @}
I don't understand these diff lines. Any idea where they are from?
I thought they were because
Reviewers: thomasmorley651,
Message:
Thanks! I actually hadn’t reviewed Johannes’ code in detail, but you
make good points.
https://codereview.appspot.com/572520044/diff/574520044/ly/swing.ly
File ly/swing.ly (right):
On 2019/03/10 13:37:42, thomasmorley651 wrote:
Oversights
Greetings Harm,
looks good; much better than the hack I had previously suggested about
#1333. However, you’re using a somewhat similar approach (although much
more fine-tuned) by overriding the stencil rather than creating an
Greetings Malte,
nice catch! I’d been struggling with it for a while.
I believe that’s the last GCC warning we had (other than -Wconversion
warnings, of which there still are many).
V.
https://codereview.appspot.com/351880043/
___
lilypond-devel
On 2019/02/19 23:25:36, Valentin Villenave wrote:
Rebase & remove now moot paragraph in CG (Thanks Carl!).
Pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=ed86e71eb0f17e93036142938d8c490a3e2fcbea
https://codereview.appspot.com/363910043/
Pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=e94db23e6c620396aafba5dc2b30a4d2e5ef70e0
Thanks everyone!
V.
https://codereview.appspot.com/353880043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
Reviewers: ,
Message:
Pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=3aeb41c27fa9db60eb6faa7c0c8532614697d06c
Description:
Mention FILE or FOLDER arg to -o command-line option.
As Federico explained in issue #5288, this is mentioned
in the Usage manual, but not in the man
Yes, I believe that’s what we were looking for. Thanks!
https://codereview.appspot.com/361790043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On 2019/02/23 22:40:44, dak wrote:
Sure, but you are trying to override stencils, aren't you? Why do you
even place an entry for "script-stencil" then instead of just placing
an
entry for "stencil"?
Oh. I’ve been an idiot from the start: it simply never occurred to me
that the script-alist
On 2019/02/22 22:11:58, dak wrote:
There is some fundamental confusion here. A stencil expression is a
list. It
never satisfies the ly:stencil? predicate and always satisfies the
pair?
predicate.
Oh, I see. I mixed up "unsmob" and "eval": a stencil expression becomes
a Stencil when it’s
On 2019/02/22 21:00:08, dak wrote:
You are confusing stencils with stencil expressions. Stencils satisfy
ly:stencil? and you can extract their stencil expression (which
usually is a
pair) with ly:stencil-expr .
Yes, but what we’re dealing with in this function is a stencil
expression, isn’t
Reviewers: dak,
Message:
Thanks David! You make valid points, I just have a couple of doubts
below.
https://codereview.appspot.com/348120043/diff/1/input/regression/script-stencil.ly
File input/regression/script-stencil.ly (right):
On 2019/02/19 22:29:34, dak wrote:
I get no warning there.
You may need to `make clean’ or `make bin-clean’ then `make’ again to
get it:
page-turn-page-breaking.cc: In instantiation of 'bool is_break(T*) [with
T = Grob]':
page-turn-page-breaking.cc:50:54: required from here
On 2019/02/19 21:40:03, Valentin Villenave wrote:
Use %zu; add warning for --relocate.
BTW, if anyone has an idea about the -Wsequence-point warning issued
about is_break () in page-turn-page-breaking.cc, I’d gladly welcome a
helping hand!
V.
https://codereview.appspot.com/353880043/
On 2019/02/08 03:48:31, dan_faithful.be wrote:
So that it’s clear: I have no firm objection to \invertChords. I just
wanted to
make sure that the names were well explored.
OK then, pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=0f5c0468117ae09916430ce22140753e9a298799
Reviewers: Dan Eble,
Message:
On 2019/02/10 13:36:00, Dan Eble wrote:
It's certainly an improvement.
A user might appreciate seeing the input location in the error
message.
OK, pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?id=88980c4f927bd1f911d66cc5352b4ac73c5eb756
Reviewers: carl.d.sorensen_gmail.com, thomasmorley651, dak,
Message:
On 2019/02/10 19:59:36, dak wrote:
>> Simple strings -- don't need #
>
> In markup-mode they do
Not in current master.
Yes; that’s what motivated this proposal in the first place.
Look, putting strings between double
On 2019/02/11 22:02:11, dak wrote:
There is alist->hash-table in scm/lily-library.scm
Which is a similar to ice-9’s alist->hashq-table…
For printing note names, there is note-name->lily-string exported from
scm/define-music-display-methods.scm which, well, actually does use
rassoc.
Yes, I
On 2019/02/10 12:23:40, dak wrote:
lily/general-scheme.cc:150: LY_DEFINE (ly_rassoc, "ly:rassoc",
I am not sure this is wanted (there are also other definitions) since
it's
comparatively slow for large sets. Can you think of a scheme using a
hashtable
as cache?
Are you sure it’s worth it?
On 2019/02/08 16:41:06, dak wrote:
For what?
For making some functions ever so slightly less cumbersome to type and
to read. But I’m gathering from your question that you’ve never felt
any need for it :-)
Since you’re complaining about LilyPond-specific functions not handled
by Guile, what
On 2019/02/08 11:51:37, dak wrote:
Frankly, I don't like it. The relevant metric here is the user time
and it's
_increased_ in your benchmark (not that it's likely significant).
Agreed.
Throwing code in that the compiler does not know about is
not likely to be much of an advantage.
In
FWIW, here’s what `make test-clean; time make-test’ returns:
Without the patch:
real10m5.239s
user17m3.656s
sys 1m2.055s
With the patch:
real9m27.001s
user17m5.396s
sys 1m0.880s
(Note that I’m not saying it has any meaning.)
V.
On 2019/02/03 14:59:03, dan_faithful.be wrote:
While it is true that this algorithm yields chords that people call
inversions,
it does not yield every chord that might be so called.
Huh. Not having learned music theory in an English-speaking environment,
I must say I’m at a bit of a loss
On 2019/02/02 20:37:16, dan_faithful.be wrote:
Isn’t the salient property of an inversion simply which note is lowest
in pitch?
I think the point of inversions is not to rearrange pitches inside a
chord, but to change the limits of the chord by changing *both* the
highest and the lowest note.
On 2019/02/01 16:18:10, dak wrote:
raising or lowering one chord note
by an octave does not guarantee that it ends up at the far end of the
chord,
like when using invertChords on a c:11 chord for the fifth inversion.
Oh. Then this becomes a whole other can of worms; what should be the
On 2019/01/28 20:07:19, Valentin Villenave wrote:
if nobody has any further objection,
I suggest pushing it onto the countdown list…
Pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?h=staging=e4996ee891128a07256e95f3c8321134eb7dd499
Thanks!
On 2019/01/28 21:53:04, dak wrote:
Without any pointer to what you are having problems with, this is
essentially
"do it yourself".
David, there seems to be some misunderstanding here (aside from your
usual snark :-)
My question was only addressing your comment with regard to (list-set! )
On 2019/01/23 22:30:25, dak wrote:
Shouldn't you consider preexisting octavation for sorting the pitches?
Duh.
That looks like a mess.
Don't you rather intend to sort the _notes_?
Sure. I figured in a chord, notes wouldn’t carry individual properties…
but they do, of course.
list-set!
On 2019/01/24 21:11:20, Valentin Villenave wrote:
Address Werner’s suggestions.
This is a rather trivial amendment, so if nobody has any further
objection, I suggesting pushing it onto the countdown list…
https://codereview.appspot.com/357930043/
Thanks a lot Werner! You caught quite a few things I should have dealt
with more rigorously.
https://codereview.appspot.com/357930043/diff/20001/Documentation/notation/text.itely
File Documentation/notation/text.itely (right):
That’s a great idea; I hadn’t considered it. I ought to add it to the
regtest then…
Oh. I’ve just noticed that although it does works with simple \dropNote
and \raiseNote commands, it makes a mess of nested commands (and
therefore \invertChords). I was careful not to overwrite a possibly
Counted down, and pushed as
https://git.savannah.gnu.org/cgit/lilypond.git/commit/?h=staging=78225bc1b386e12dc1d03a5d2c7a017c0a52a22d
Thanks!
https://codereview.appspot.com/363880043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
On 2018/12/21 17:50:33, Valentin Villenave wrote:
Correct convert rules.
OK, pushed onto staging as 72067b395d947f1349ab8010f0592d45e52b8141.
https://codereview.appspot.com/369930043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
Reviewers: dak,
https://codereview.appspot.com/348060043/diff/1/ly/chord-modifiers-init.ly
File ly/chord-modifiers-init.ly (right):
https://codereview.appspot.com/348060043/diff/1/ly/chord-modifiers-init.ly#newcode49
ly/chord-modifiers-init.ly:49: 1-\markup { \super "5" }
On 2018/12/13
On 2016/02/29 11:25:37, dak wrote:
On 2016/02/29 11:22:32, dak wrote:
> Oh, didn't see it. I definitely would refrain from modifying files
in any
way.
^This, and:
Besides, the main purpose of adding such files is that the recipient
is able to
recompile them on his own in the same
On 2016/02/28 12:18:29, dak wrote:
It seems to me (after glancing over it) like the last Rietveld review
was just
abandoned and the same state resubmitted.
Greetings David,
not entirely. I’ve removed the pointless convert-rules, among others.
Any reason the suggestions have not been
OK, new version. The new "fluid" parser variable is noticeably nicer to
work with than what we had before.
https://codereview.appspot.com/225040043/diff/120001/lily/sources.cc
File lily/sources.cc (right):
https://codereview.appspot.com/225040043/diff/120001/lily/sources.cc#newcode93
On 2016/02/23 18:07:39, Valentin Villenave wrote:
Access source files only through LilyPond.
It would be nice if we could specify an author/description for each
source file... but as far as I can see, there are a bunch of "not
implemented yet" in Ghostscript:
Reviewers: simon.albrecht,
Message:
On 2016/02/22 20:46:57, simon.albrecht wrote:
https://codereview.appspot.com/288290043/diff/1/Documentation/notation/pitches.itely
File Documentation/notation/pitches.itely (right):
On 2015/05/24 22:56:44, Keith wrote:
I think whiteout-stencil should simply call this function, with the
defaults
suggested, so that the existing
\set Xx.whiteout=##t and \markup\whiteout use this new method.
I’ve always been unhappy with the current method for whiteout (which I
have also
https://codereview.appspot.com/239930043/diff/1/Documentation/notation/pitches.itely
File Documentation/notation/pitches.itely (right):
https://codereview.appspot.com/239930043/diff/1/Documentation/notation/pitches.itely#newcode492
Documentation/notation/pitches.itely:492: @item @code{francais}
On 2015/05/18 17:38:22, dak wrote:
It does not have non-ASCII notenames, so you might end up with
español being
the only non-ASCII word in the file
How much of a potential problem is it? Even in the absence of non-ASCII
chars, users are already strongly encouraged to use UTF8 encoding in
Reviewers: ,
Message:
Greetings everybody,
here’s an itch I’ve been wanting to scratch for the past decade or so:
as a French user, having to type re instead of ré feels unnatural
(and it still does after ten years); it is not uncommon that my scores
fail to compile because I’ve inadvertently
On 2015/05/17 08:39:00, dak wrote:
Well, with notenames like re it should have been \language
francais... I think we even had this at one time. No wait, that
was
espanol.
Yes. español is aliased to espanol, and français is now aliased to
francais in the same way.
It's actually the few
Greetings everybody,
Quite a few TODO remain wherever I’m not sure exactly what to do; it
would be great if others could help me clear these out before the patch
can get merged. Thanks!
https://codereview.appspot.com/225380043/
___
lilypond-devel
On 2015/04/18 08:46:08, J_lowe wrote:
Patch on countdown for April 21st
Is there a way to check this patch on various MSwin systems (with
various file and subfolder paths) before merging it? What about older
Windows versions? (IIRC we used to support up to 98SE, which is still
somewhat
Hi Carl,
thanks for resurrecting this patch! From what I understand, the logic
behind this patch looks quite sensible (but IANAP, so please take it
with a grain of salt).
Perhaps you could add a regtest demonstrating this feature (or add a
dotted+accidental note to parenthesize.ly)? Just in case
On 2012/07/14 12:11:09, janek wrote:
this is really cool!
Hi everybody,
since this patch seems to have gone cold, I thought I’d update it a bit
and re-upload it:
https://codereview.appspot.com/226110044/
https://codereview.appspot.com/6397043/
___
On 2011/09/17 19:29:40, janek wrote:
It is ready to go except for the above.
Greetings,
I realize that this patch has been sort of forgotten, so I thought I’d
update it and re-upload it:
https://codereview.appspot.com/227070043/
https://codereview.appspot.com/4636081/
On 2013/04/26 22:38:57, janek wrote:
New patchset should be announced in the tracker issue, yes.
Hi everybody,
I realize this patch has been (sort of) forgotten, so I thought I’d
update it and re-upload it:
https://codereview.appspot.com/221710044/
Cheers,
Valentin.
On 2015/04/11 07:43:55, dak wrote:
See the Button marked Delete in the following screen shot.
Thanks.
I’ve triple-checked this patch and it doesn’t appear to be breaking
anything (I’m running make check again just in case, but I’ll feel
reassured once it’s checked by other people as well);
What a mess. I inadvertently uploaded my round 2 proposal onto this
issue where it doesn’t belong.
Since Rietveld doesn’t seem to offer a way of undoing it, please
disregard patch set #10 (#9 is the one actually being reviewed, with
all of David’s suggestions taken into account).
On 2015/04/10 09:42:38, dak wrote:
In this particular case, it would appear that
Includable_lexer::file_name_strings_ already contains properly
expanded file
names after the file name search has succeeded, so that would seem to
be the
smarter thing to reference.
Here’s what I came up with;
Reviewers: ,
Message:
Greetings everybody,
I realize that this is a rather large patch; at home I had divided it
into smaller, more manageable commits such as
0001-C-Scheme-consistency-use-ly_is_eol.patch
0002-C-Scheme-consistency-use-scm_is_false.patch
Reviewers: ,
Message:
Greetings everybody,
since there have been some discussions about updating GhostScript
lately, perhaps now would be a convenient time to resurrect this
feature...
https://lists.gnu.org/archive/html/lilypond-user/2012-07/msg00105.html
Greetings Philippe and James,
this looks good overall, I just have one minor comment.
https://codereview.appspot.com/152600043/diff/1/ly/event-listener.ly
File ly/event-listener.ly (right):
https://codereview.appspot.com/152600043/diff/1/ly/event-listener.ly#newcode129
Thanks Trevor.
https://codereview.appspot.com/151230044/diff/40001/Documentation/notation/spacing.itely
File Documentation/notation/spacing.itely (right):
https://codereview.appspot.com/151230044/diff/40001/Documentation/notation/spacing.itely#newcode1000
Greetings,
having pushed the recent string-number-styles patch, I thought another
area where we might benefit from roman numerals would be page numbers.
This is an attempt at getting there.
https://codereview.appspot.com/151230044/
___
Greetings James,
I'm far from being the most qualified person to review this, but it
looks pretty good to me. I'm not sure about the capital A in the
Articulate script but at least your patch set is a clear documentation
improvement. Cheers!
Thanks James,
since I've been living under a rock for some time, I wasn't aware of the
Google tracker/Rietveld integration. Thanks Colin!
I've slightly reworded your suggestions but hopefully this will be both
concise and clear.
Reviewers: ,
Message:
Greetings everybody,
here's a thought: the shorthand syntax for string numbers is so useful
that it shouldn't be limited to fretted strings, but could also come
handy for unfretted strings notation. This is the less dirty way I
could find of making that happen.
(Sidebar:
Slight update; thanks for your patience (I’ve never been good at this
but now I’m also quite a bit rusty).
https://codereview.appspot.com/20660044/diff/1/ly/dynamic-scripts-init.ly
File ly/dynamic-scripts-init.ly (right):
On 2011/04/12 07:04:42, Keith wrote:
This solves the critical bug,
but I would like a future patch to replace this with a mapping to
replace ( by
\( etc.
(string-delete has arguments in the order opposite to Scheme
documentation to
compensate for a bug in Guile, fixed in Guile 1.9.)
Greetings Graham,
this looks acceptable to me, although I'm certainly not the most
qualified person in this regard.
Are you quite sure this really is generic enough, though? There are a
few hardcoded things here, and more inconveniently this approach means
you have to manually choose which
LGTM. I was a bit shocked when I saw the collisions in the example, but
I think I understand Keith's point.
http://codereview.appspot.com/3782042/diff/1/Documentation/notation/keyboards.itely
File Documentation/notation/keyboards.itely (right):
On 2010/12/20 23:18:37, Neil Puttock wrote:
Simply set 'direction inside the parser
rule itself following the function evaluation.
Hi Neil,
something like that? (see new patch set)
Cheers,
Valentin.
http://codereview.appspot.com/3743043/
___
On 2010/12/20 16:22:30, Carl wrote:
IMO, go ahead and set the neutral direction. If it passes make check,
then call
it good.
Hi Carl,
actually, whilst it does pass make check, it breaks... all of my own
scores!
I use the following function:
filterDirections=
#(define-music-function
On 2010/12/21 21:24:52, Carl wrote:
I can't understand why you don't want to push a patch that breaks all
your
scores... ;-)
Does this latest patch break your scores?
It does, since the 'direction property is never left null (I can work
around it, though. But at first I did feel a bit
On 2010/12/21 22:21:38, Neil Puttock wrote:
Almost there, but you're still setting 'direction unconditionally
instead of
checking first, i.e.,
Done (new patch). However, I can't really understand how this condition
could ever be false. What kind of input could trigger the parser rule
without
On 2010/12/21 22:27:43, Neil Puttock wrote:
Use ly:dir? instead of (not (null? ...).
Duh. Nice catch, thanks!
Cheers,
V.
http://codereview.appspot.com/3743043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
On 2010/12/21 22:45:53, Neil Puttock wrote:
CENTER = 0, so is always false.
Oh, I didn't know 0 was false, in Guile you can have something like:
guile (define a 0)
guile (if a (display true\n))
true
guile
(yes it's silly, but remember that the only things about programming I
know, I learned
Reviewers: Neil Puttock,
Message:
Greetings everybody,
here's a patch that Neil suggested to me (if you remember my earlier
postfix-dynamics patch, you may guess where I'm getting at with this).
Thoughts?
Cheers,
Valentin.
Description:
Add direction-awareness ability to post-event music
1 - 100 of 156 matches
Mail list logo
