Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
https://codereview.appspot.com/549840044/diff/567430043/lily/grob-scheme.cc File lily/grob-scheme.cc (left): https://codereview.appspot.com/549840044/diff/567430043/lily/grob-scheme.cc#oldcode327 lily/grob-scheme.cc:327: LY_DEFINE (ly_grob_basic_properties, "ly:grob-basic-properties", There is r

Re: Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
The description says "[ly:grob-properties] will not return the properties that were \overridden." Aren't you confusing this with ly:grob-basic-properties ? I think ly:grob-properties will actually _only_ return the properties that were overriden. https://codereview.appspot.com/549840044/

Re: Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
On 2020/04/11 16:32:58, thomasmorley651 wrote: > On 2020/04/11 16:31:48, thomasmorley651 wrote: > > > Btw, it's used in > > input/regression/multi-measure-rest-reminder.ly > > input/regression/scheme-text-spanner.ly > > as well. > > No, that's ly:grob-properties? > Bad grepping ... Also icky nam

Re: Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
On 2020/04/11 16:38:28, hanwenn wrote: > On 2020/04/11 16:02:31, dak wrote: > > The description says "[ly:grob-properties] will not return the properties that > > were \overridden." > > > > Aren't you confusing this with ly:grob-basic-properties ? I

Re: Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
On 2020/04/11 17:44:11, hanwenn wrote: > On 2020/04/11 17:19:19, dak wrote: > > > I thought of removing the other function too, and I agree in principle, but > it > > > can only break more user files. As long as we're not considering > reorganizing > > >

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-12 Thread dak
On 2020/04/12 10:35:24, hanwenn wrote: > On Sat, Apr 11, 2020 at 3:55 PM David Kastrup wrote: > > >> You haven't understood the scheme. A grob only contains the properties > > >> seminal to its type. That's why there is a double indirection. A full > > >> array of 416 entri

Re: Define Scheme markups using define-public (issue 577720043 by hanw...@gmail.com)

2020-04-12 Thread dak
On 2020/04/12 10:59:21, hanwenn wrote: > On 2020/03/30 12:20:17, dak wrote: > > > > Ugly and a maintenance burden since the code is used twice. Anything > > > > wrong with my proposal? > > > > > > I didn't understand your proposal. >

Re: Use composition for embedding PangoMatrix in Transform (issue 555610043 by hanw...@gmail.com)

2020-04-13 Thread dak
"PangoMatrix is a struct, so there is no value in using inheritance" is a non-sequitur. A struct just is a class with public members. The motivation for choosing inheritance here was that this allows passing Transform directly into Pango for operations. This advantage is not realised in our curr

Optimize get_path_list. (issue 579570043 by hanw...@gmail.com)

2020-04-13 Thread dak
There is scm_c_memq for avoiding hare&tortoise, but for this use case, the explicit code seems like the saner option. Even without hare&tortoise, the list would be consed together each time without further measures. https://codereview.appspot.com/579570043/diff/577750045/lily/stencil-integral.cc

Move get_normal to Offset::normal (issue 576000043 by hanw...@gmail.com)

2020-04-13 Thread dak
https://codereview.appspot.com/57643/diff/577750046/flower/include/offset.hh File flower/include/offset.hh (right): https://codereview.appspot.com/57643/diff/577750046/flower/include/offset.hh#newcode123 flower/include/offset.hh:123: Offset normal() const { It's kind of unusual to make a

Re: Move get_normal to Offset::normal (issue 576000043 by hanw...@gmail.com)

2020-04-13 Thread dak
.hh:123: Offset normal() const { > On 2020/04/13 18:24:47, dak wrote: > > It's kind of unusual to make a constructing function a (non-static) member > > function unless this is necessitated for operator semantics, because different > > conversion rules apply and there

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread dak
https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh File lily/include/mutable-properties.hh (right): https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh#newcode31 lily/include/mutable-properties.hh:31: class Iterator {

Re: Rewrite Mutable_properties based on vector (issue 575990043 by hanw...@gmail.com)

2020-04-13 Thread dak
https://codereview.appspot.com/575990043/diff/577740043/lily/mutable-properties.cc File lily/mutable-properties.cc (right): https://codereview.appspot.com/575990043/diff/577740043/lily/mutable-properties.cc#newcode50 lily/mutable-properties.cc:50: if (SCM_EQ_P (key (i), k)) scm_is_eq https://co

Re: Rewrite Mutable_properties based on vector (issue 575990043 by hanw...@gmail.com)

2020-04-13 Thread dak
On 2020/04/13 20:21:29, dak wrote: > https://codereview.appspot.com/575990043/diff/577740043/lily/mutable-properties.cc#newcode109 > lily/mutable-properties.cc:109: Mutable_properties::mark () const > This can segfault in GUILEv2 and/or depending on some other circumstances sinc

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread dak
On 2020/04/13 20:27:34, hanwenn wrote: > On 2020/04/13 20:10:35, dak wrote: > > > https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh > > File lily/include/mutable-properties.hh (right): > > > > > https://codereview.appspot.

Re: Rewrite Mutable_properties based on vector (issue 575990043 by hanw...@gmail.com)

2020-04-13 Thread dak
On 2020/04/13 21:13:42, hanwenn wrote: > > > > You need to sort the C++ structures potentially accessed by the GC hook into a > > Preinit class (grep for Preinit for existing uses) and inherit from that > before > > inheriting from whatever operates the Smob. Then C++ guarantees an > > initialis

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-13 Thread dak
On 2020/04/13 23:07:57, Dan Eble wrote: > This change per se LGTM. I remember discussing this syntactic change briefly on > the list a few(?) months ago, so this is not surprising. > > I'm quite pleased with this change, actually. I remember how I felt the first > time I came across klass->a_mac

Re: Rewrite Mutable_properties based on vector (issue 575990043 by hanw...@gmail.com)

2020-04-14 Thread dak
On 2020/04/13 17:43:17, hanwenn wrote: > > Actually, it looks like I have been measuring noise. Thermal throttling? > > It looks like I can get reproducible timings if I downclock the CPU from 2.6 to > 2.0Ghz Put the following line in the config /etc/modprobe.d/acpi.conf:options thinkpad-acpi

Re: Use Interval for representing skyline X coordinates (issue 571980043 by hanw...@gmail.com)

2020-04-16 Thread dak
Purpose is not clear since the code does not appear to use anything other than the explicit endpoints. Also, intervals are indexed by LEFT/RIGHT rather than START/STOP. Values may be the same, but the former is a whole lot more mnemonic. https://codereview.appspot.com/571980043/

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 05:40:53, lemzwerg wrote: > LGTM - but I don't know the internals well enough to really decide that. It's really not as much about the internals rather than about how to express their use. GET_LISTENER provides an SCM-callable memoized function port into a C++ member function. The

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 10:26:01, dak wrote: > On 2020/04/18 05:40:53, lemzwerg wrote: > > > What about using two macros instead of one? The first would take a context as a > > first argument (as it does now), the second one uses 'this' in the macro body, > > omitting tha

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 10:32:50, lemzwerg wrote: > > I agree with the sentiment, but I fail to come up > > with a naming choice that does not make the cure > > worse than the problem. > > I would simply use GET_LISTENERX (or GET_LISTENER1) for the longer call. That's being obtuse. Requires learning two t

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 10:30:27, hanwenn wrote: > https://codereview.appspot.com/549890043/diff/579610043/lily/include/listener.hh > File lily/include/listener.hh (left): > > https://codereview.appspot.com/549890043/diff/579610043/lily/include/listener.hh#oldcode140 > lily/include/listener.hh:140: #define

Re: get_path_list: use loop instead of tail recursion (issue 579570046 by d...@gnu.org)

2020-04-18 Thread dak
Reviewers: hanwenn, Message: On 2020/04/17 09:12:18, hanwenn wrote: > LGTM > > https://codereview.appspot.com/579570046/diff/579580044/lily/stencil-integral.cc > File lily/stencil-integral.cc (right): > > https://codereview.appspot.com/579570046/diff/579580044/lily/stencil-integral.cc#newcode111

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
https://codereview.appspot.com/549890043/diff/573710043/lily/global-context.cc File lily/global-context.cc (right): https://codereview.appspot.com/549890043/diff/573710043/lily/global-context.cc#newcode49 lily/global-context.cc:49: event_source ()->add_listener (GET_LISTENER (static_cast (this),

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 14:43:24, Dan Eble wrote: > On 2020/04/18 14:31:33, dak wrote: > > If we consider type_traits as a sunk cost, I'll see whether I can find > anything > > in it to address the inheritance wart you complained of. > > As you already put in some time there,

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-19 Thread dak
Reviewers: lemzwerg, Message: On 2020/04/19 05:53:34, lemzwerg wrote: > Nice! I don't really understand the C++ wizardry, but having to type less > macros in the normal code is certainly beneficial. The wizardry is comparatively confined and mainly consists of a more polished version of the mfp_

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-19 Thread dak
https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh File lily/include/callback.hh (right): https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh#newcode166 lily/include/callback.hh:166: typedef typename remove_pointer (nullptr)))>::type typ

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-19 Thread dak
On 2020/04/19 09:55:03, hahnjo wrote: > On 2020/04/19 09:39:00, dak wrote: > > > https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh > > File lily/include/callback.hh (right): > > > > > https://codereview.appspot.com/551780043/diff/5

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-19 Thread dak
On 2020/04/19 11:03:35, hahnjo wrote: > On 2020/04/19 10:55:52, dak wrote: > I have the vague > > impression that the ability to do template specialisation on typedefs would > also > > be able to solve part of the job here in a more elegant manner but right now > my >

Re: Rewrite Skyline code (issue 547980044 by hanw...@gmail.com)

2020-04-19 Thread dak
On 2020/04/19 21:43:26, hanwenn wrote: > (this still needs some work, but the speedup might be worth an early look) * Buildings store Y coordinate of the left edge, rather than the intercept at x==0.0. This avoid excessive rounding problems when X is large, and lets us compute building.height

Issue #1204: fix font-name-add-files regtest (issue 573730044 by v.villen...@gmail.com)

2020-04-20 Thread dak
https://codereview.appspot.com/573730044/diff/583810043/input/regression/font-name-add-files.ly File input/regression/font-name-add-files.ly (right): https://codereview.appspot.com/573730044/diff/583810043/input/regression/font-name-add-files.ly#newcode25 input/regression/font-name-add-files.ly:

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-20 Thread dak
On 2020/04/20 23:26:07, Dan Eble wrote: > On 2020/04/20 19:36:39, dak wrote: > > Polish and extend tools in callback.hh > > LGTM. > > I have a hunch that the MFPn_WRAP macros could be turned into something more > typical of C++, but I don't blame you for resting at t

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-20 Thread dak
On 2020/04/21 00:43:02, dak wrote: > On 2020/04/20 23:26:07, Dan Eble wrote: > > On 2020/04/20 19:36:39, dak wrote: > > > Polish and extend tools in callback.hh > > > > LGTM. > > > > I have a hunch that the MFPn_WRAP macros could be turned into some

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

2020-04-21 Thread dak
I am rather skeptical about the usefulness of such microoptimizations without influence on algorithmic complexity. In return for better memory locality you buy into quite larger memory fragmentation, and we have scores of comparatively modest size already exhausting memory. All that exhausted mem

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

2020-04-23 Thread dak
On 2020/04/22 07:43:17, hanwenn wrote: > current master: > > 4f906b95aea99f2a47d5ba037f7421e5bb933c42 (origin/master) Issue 5908: > get_path_list: use loop instead of tail recursion > > Command being timed: "out/bin/lilypond.4f906b95ae -I carver MSDM" > User time (seconds): 34.30 > Maximum reside

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

2020-04-23 Thread dak
On 2020/04/23 16:01:20, dak wrote: > On 2020/04/22 07:43:17, hanwenn wrote: > > * The linked list has an overhead of 2 pointers, on a 4x Real > > datastructure, ie. 50% overhead. By the way: C++11 has Forward_list which would be another plugin container type with less overhead.

Re: make_draw_bezier_boxes: save work if thickness == 0.0 (issue 551730043 by hanw...@gmail.com)

2020-04-24 Thread dak
https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc#newcode496 lily/stencil-integral.cc:496: break; I think that is less readable than desirable.

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

2020-04-24 Thread dak
On 2020/04/24 21:18:12, dak wrote: > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc > File lily/stencil-integral.cc (right): > > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc#newcode465 > lily/stencil-integral.

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

2020-04-24 Thread dak
https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc#newcode465 lily/stencil-integral.cc:465: // more convoluted, but it's fairly hot path. Sorry fo

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

2020-04-24 Thread dak
On 2020/04/24 21:33:12, Carl wrote: > On 2020/04/24 21:19:57, dak wrote: > > On 2020/04/24 21:18:12, dak wrote: > > > > > > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc > > > File lily/stencil-integral.

Re: Rewrite Skyline code (issue 547980044 by hanw...@gmail.com)

2020-04-24 Thread dak
On 2020/04/19 22:35:39, dak wrote: > On 2020/04/19 21:43:26, hanwenn wrote: > > (this still needs some work, but the speedup might be worth an early look) > > * Buildings store Y coordinate of the left edge, rather than the > intercept at x==0.0. This avoid excessive roundi

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

2020-04-25 Thread dak
On 2020/04/25 07:29:06, hanwenn wrote: > On 2020/04/24 22:04:52, dak wrote: > > On 2020/04/24 21:33:12, Carl wrote: > > > On 2020/04/24 21:19:57, dak wrote: > > > > On 2020/04/24 21:18:12, dak wrote: > > > > > > > > > > > > > >

Re: Fix warnings related to po (issue 551830043 by jonas.hahnf...@gmail.com)

2020-04-25 Thread dak
On 2020/04/25 08:36:02, hahnjo wrote: > On 2020/04/24 20:14:36, hahnjo wrote: > > On 2020/04/24 18:25:33, lemzwerg wrote: > > > LGTM, thanks a lot! > > > > > > https://codereview.appspot.com/551830043/diff/573740043/po/GNUmakefile > > > File po/GNUmakefile (right): > > > > > > > > > https://coder

Re: Add a script for running timing benchmarks (issue 545950043 by hanw...@gmail.com)

2020-04-25 Thread dak
On 2020/04/25 17:07:17, hahnjo wrote: > I strongly object to adding more random scripts to the source tree. There are > already far too many unmaintained in scripts/auxiliar/ with no documentation at > all. How about approaching this in a different manner then? Adding instructions to the CG about

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

2020-04-26 Thread dak
On 2020/04/24 13:15:10, dak wrote: > mailto:hanw...@gmail.com writes: > > > Here is how I have experienced this discussion: > > > > DAK: this is micro-optimization that causes memory fragmentation. > > > > HW: No, it's not; here is a benchmark th

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-04-27 Thread dak
Without having looked all that much at the code: When the parser sees some \blabla it will generally first have to check for a keyword and (when it has no match) afterwards for a variable with that name, and a lot of those are actually music functions these days that used to be keyword. Wouldn't

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-04-27 Thread dak
On 2020/04/27 09:11:16, dak wrote: > Han-Wen Nienhuys <mailto:hanw...@gmail.com> writes: > > > On Mon, Apr 27, 2020 at 10:59 AM <mailto:d...@gnu.org> wrote: > >> When the parser sees some \blabla it will generally first have to check > >> for a keyword a

Re: Thread skyline construction through stencil interpretation (issue 581960043 by hanw...@gmail.com)

2020-04-27 Thread dak
That's the kind of stuff that warrants compiling a few scores with -ddebug-skylines and looking rather closely for kinks. https://codereview.appspot.com/581960043/diff/561710044/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/581960043/diff/56171004

Re: Use Scheme_hash_table for keyword handling (issue 577840053 by d...@gnu.org)

2020-04-27 Thread dak
https://codereview.appspot.com/577840053/diff/561760044/lily/lily-lexer.cc File lily/lily-lexer.cc (right): https://codereview.appspot.com/577840053/diff/561760044/lily/lily-lexer.cc#newcode97 lily/lily-lexer.cc:97: Protected_scm Lily_lexer::keytable_; On 2020/04/27 21:22:59, hanwenn wrote: > co

Re: Use Scheme_hash_table for keyword handling (issue 577840053 by d...@gnu.org)

2020-04-27 Thread dak
On 2020/04/27 22:57:06, dak wrote: > Move keytable init out of constructor. Frankly, I don't consider this an > improvement. As anecdotal data, it took me about as long to get this working as the whole other patch, and I had it segfault about half a dozen times and had another

Re: Use Scheme_hash_table for keyword handling (issue 577840053 by d...@gnu.org)

2020-04-27 Thread dak
On 2020/04/27 23:07:17, dak wrote: > On 2020/04/27 22:57:06, dak wrote: > > Move keytable init out of constructor. Frankly, I don't consider this an > > improvement. > > As anecdotal data, it took me about as long to get this working as the whole > other patch, and I

Re: Use Scheme_hash_table for keyword handling (issue 577840053 by d...@gnu.org)

2020-04-27 Thread dak
https://codereview.appspot.com/577840053/diff/561760044/lily/lexer.ll File lily/lexer.ll (right): https://codereview.appspot.com/577840053/diff/561760044/lily/lexer.ll#newcode928 lily/lexer.ll:928: // use more SCM for this. On 2020/04/27 23:24:19, Dan Eble wrote: > Is this what you've just done,

Issue 5934: remove Repeated_music::folded_music_length (issue 561670043 by nine.fierce.ball...@gmail.com)

2020-04-28 Thread dak
Interesting. In 2.18 it appears to be used as length-callback in TremoloRepeatedMusic. Was that wrong or did something change in unfolded_music_length ? https://codereview.appspot.com/561670043/

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-04-28 Thread dak
Regarding the "Get rid of unused function ly:lexer-keywords": that one was added with commit 1ae6f5710e714f13da1bfbfbd840835316618d1f Author: Han-Wen Nienhuys Date: Thu Dec 14 14:10:56 2006 +0100 add ly:lexer-keywords to make keyword extraction easier and exact. but there appears to be no

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-04-30 Thread dak
On 2020/04/30 07:42:16, hahnjo wrote: > On 2020/04/27 11:58:11, dak wrote: > > Tracker issue: 5946 (https://sourceforge.net/p/testlilyissues/issues/5946/) > > Rietveld issue: 577840053 (https://codereview.appspot.com/577840053) > > Issue description: > > Use Sc

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-04-30 Thread dak
On 2020/04/30 19:15:23, hahnjo wrote: > This probably goes without saying, but touching such core functionality should > receive very rigid testing. I hope I got most things covered, but especially > manual feedback would be very much appreciated! Sounds pretty good. A great long-term perspective

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 07:58:32, hahnjo wrote: > disclaimer: I'm not a lawyer, this is just my understanding of the licenses. > > On 2020/05/01 06:28:56, hanwenn wrote: > > Technologically speaking, this is a brilliant idea, and I am all in favor it. > > > > However, I think we can't enable this by defaul

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 11:15:29, hanwenn wrote: > On 2020/04/30 07:58:09, dak wrote: > > On 2020/04/30 07:42:16, hahnjo wrote: > > > On 2020/04/27 11:58:11, dak wrote: > > > > Tracker issue: 5946 > (https://sourceforge.net/p/testlilyissues/issues/5946/) > &g

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 11:18:11, hahnjo wrote: > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc > File lily/general-scheme.cc (right): > > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc#newcode783 > lily/general-scheme.cc:783: command += "(" +

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 11:44:05, hahnjo wrote: > On 2020/05/01 11:41:13, dak wrote: > > On 2020/05/01 11:18:11, hahnjo wrote: > > > > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc > > > File lily/general-scheme.cc (right): > > > >

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 22:19:38, hanwenn wrote: > On Fri, May 1, 2020 at 11:58 PM <mailto:v.villen...@gmail.com> wrote: > > > > On 2020/05/01 12:12:40, dak wrote: > > > That being said, the situation regarding Scorio, a proprietary entity > > using Free > > >

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-02 Thread dak
On 2020/05/02 09:49:44, hahnjo wrote: > On 2020/05/01 06:28:56, hanwenn wrote: > > I suggest we make this an option that you have enable explicitly. > > done > > > If it is enabled, we'd have to change the --license output to say AGPL as > well. > > I thought about this and decided against it: >

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-03 Thread dak
On 2020/05/03 09:46:56, hahnjo wrote: > rebase + allow -dgs-api=#f to still fork I decided to look up the terms of the Affero GPL version 3. The relevant differences are: > Notwithstanding any other provision of this License, if you modify the Program, your modified version must prominently offe

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-03 Thread dak
On 2020/05/02 10:22:15, hahnjo wrote: > https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc > File lily/general-scheme.cc (right): > > https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc#newcode778 > lily/general-scheme.cc:778: free (a); > On 2

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-03 Thread dak
On 2020/05/03 10:46:49, hahnjo wrote: > On 2020/05/03 10:40:25, dak wrote: > > On 2020/05/03 09:46:56, hahnjo wrote: > > > rebase + allow -dgs-api=#f to still fork > > > > I decided to look up the terms of the Affero GPL version 3. The relevant > > difference

Re: Split glyph contours in up/down segments for skylines (issue 569700043 by hanw...@gmail.com)

2020-05-08 Thread dak
On 2020/05/08 14:14:47, hahnjo wrote: > assert(false) that none of the previous cases was true? That should detect a > breaking change in FT_Outline pretty quickly. else if (condition) { ... } else assert(false); seems to make the assertion output comparatively useless. Wouldn't it be better to

Re: define-markup-command-internal -> module-define-markup-command! (issue 547920045 by d...@gnu.org)

2020-05-08 Thread dak
Reviewers: hanwenn, Message: On 2020/05/08 17:12:41, hanwenn wrote: > I don't understand how this approach could ever help byte-compiling the markup > scheme files. This still uses module-define! , so the guile2 compilation step > will be oblivious to markup functions. It makes it easier to crea

Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

2020-05-09 Thread dak
Rationale? This negates work done as part of issue 5347 with the long-term goal of making transforms a better integrated and accessible part of stencils. This will be increasingly important when we move to Cairo since Cairo cannot represent rotations by multiples of 90 degrees cleanly other than

Re: Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

2020-05-09 Thread dak
On 2020/05/09 12:00:01, hanwenn wrote: > On 2020/05/09 11:03:12, dak wrote: > > Rationale? This negates work done as part of issue 5347 with the long-term > goal > > of making transforms a better integrated and accessible part of stencils. > This > > will be increasi

Re: Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

2020-05-09 Thread dak
On 2020/05/09 16:19:59, hanwenn wrote: > On Sat, May 9, 2020 at 4:56 PM <mailto:d...@gnu.org> wrote: > > > > On 2020/05/09 12:00:01, hanwenn wrote: > > > On 2020/05/09 11:03:12, dak wrote: > > > > Rationale? This negates work done as part of issue 5347 wi

Re: Add on-page-greater-than, -less-than (on-the-fly) (issue 74540044)

2017-06-09 Thread dak
Ok, I've taken another look at something that should help with this amount of fine-grained definitions. Do you think that the following macro markup-when would be fine-grained enough to forego these kind of definition? I demonstrate it for implementing on-page-greater-than but of course one may

Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)

2017-06-12 Thread dak
https://codereview.appspot.com/321930043/diff/160001/Documentation/notation/simultaneous.itely File Documentation/notation/simultaneous.itely (right): https://codereview.appspot.com/321930043/diff/160001/Documentation/notation/simultaneous.itely#newcode917 Documentation/notation/simultaneous.ite

Re: Add on-page-greater-than, -less-than (on-the-fly) (issue 74540044)

2017-06-12 Thread dak
On 2017/06/10 19:36:30, pwm wrote: That's a nice consistency. I tried it out and the following works too: #(define (onpage proc nmbr) (markup-when ((page:page-number -1)) (proc page:page-number nmbr))) \paper { #(set-paper-size "a7landscape") oddFooterMarkup = \markup \on-the

Re: Various chain-assoc-get -> #:properties (issue 323940043 by d...@gnu.org)

2017-06-16 Thread dak
https://codereview.appspot.com/323940043/diff/20001/Documentation/snippets/new/three-sided-box.ly File Documentation/snippets/new/three-sided-box.ly (right): https://codereview.appspot.com/323940043/diff/20001/Documentation/snippets/new/three-sided-box.ly#newcode36 Documentation/snippets/new/thr

Re: Change all instances of \partcombine to \partCombine in the documentation (issue 326870043 by pkx1...@gmail.com)

2017-07-10 Thread dak
https://codereview.appspot.com/326870043/diff/1/Documentation/snippets/changing-partcombine-texts.ly File Documentation/snippets/changing-partcombine-texts.ly (right): https://codereview.appspot.com/326870043/diff/1/Documentation/snippets/changing-partcombine-texts.ly#newcode24 Documentation/sni

Re: Change all instances of \partcombine to \partCombine in the documentation (issue 326870043 by pkx1...@gmail.com)

2017-07-10 Thread dak
On 2017/07/10 15:38:52, Carl wrote: On 2017/07/10 14:09:53, dak wrote: > https://codereview.appspot.com/326870043/diff/1/Documentation/snippets/changing-partcombine-texts.ly > File Documentation/snippets/changing-partcombine-texts.ly (right): > > https://codereview.appspot.c

Re: Centering bass figures on whole notes and longer (issue 325070043 by pkx1...@gmail.com)

2017-07-15 Thread dak
On 2017/07/15 07:51:11, thomasmorley651 wrote: On 2017/07/15 07:40:01, thomasmorley651 wrote: > > In > << > \relative c' { c1 c1 } > \figures { <6+>2 <6+>2 <6+>1 } > >> > I would expect same aligning for first and second BassFigure related to the > note. Aargh, should read: I would expec

Re: Change \note markup command to get a duration (issue 328050043 by d...@gnu.org)

2017-07-25 Thread dak
On 2017/07/25 10:03:08, thomasmorley651 wrote: LGTM Though, wouldn't the rest-markup-command needed to be changed accordingly? Needed? No. The note-markup-command did not need to be changed either. But rest-markup-command _wants_ to be changed accordingly of course. You'll find that I hav

Re: Allow defining markup commands in LilyPond syntax (issue 324140043 by d...@gnu.org)

2017-07-30 Thread dak
Reviewers: lemzwerg, thomasmorley651, Message: On 2017/07/30 15:41:56, thomasmorley651 wrote: If I understand correctly the main advantage for the user will be the possibility to define custom markup-commands with LilyPond-syntax. Collecting a subset of preexisting commands to substitute tedio

Re: Change \note markup command to get a duration (issue 328050043 by d...@gnu.org)

2017-07-30 Thread dak
On 2017/07/25 10:44:09, dak wrote: On 2017/07/25 10:03:08, thomasmorley651 wrote: > LGTM > > Though, wouldn't the rest-markup-command needed to be changed accordingly? Needed? No. The note-markup-command did not need to be changed either. But rest-markup-command _wants_

Re: Output better message for wrong argument of dash-definiton (issue 321400043 by thomasmorle...@gmail.com)

2017-08-02 Thread dak
On 2017/08/02 20:44:07, thomasmorley651 wrote: adds list-of-lists? in lilypond-scheme-predicates Frankly, I find that sort-of pointless: this just replaces one insufficient predicate with a slightly less insufficient one. It's nice that this triggers an error at the point of call rather than e

Re: Support for English note names in Arabic Music (issue 322510043 by pkx1...@gmail.com)

2017-08-13 Thread dak
On 2017/08/13 19:17:09, pkx166h wrote: This fails on 'make' and I don't know why... --snip-- Effective prefix: "/home/james/lilypond-git/build/out/share/lilypond/current" PATH="/home/james/lilypond-git/build/out/bin/../bin:/home/james/lilypond-git/build/out/bin:/home/james/lilypond-git/bu

Re: Issue 4678: Fix spaces in metronome mark (issue 323420043 by beauleetien...@gmail.com)

2017-08-16 Thread dak
Uh, isn't that just skirting the issue? SVG text generation has a bug swallowing spaces and instead of fixing that, one instance of space use rather uses something else? That sounds like a bottomless barrel. Shouldn't we rather try to fix the actual problem? I mean, it will occur for user-gene

Re: Issue 4678: Fix spaces in metronome mark (issue 323420043 by beauleetien...@gmail.com)

2017-08-17 Thread dak
On 2017/08/17 19:59:30, Ebe123 wrote: On 2017/08/16 22:03:31, dak wrote: > Uh, isn't that just skirting the issue? SVG text generation has a bug > swallowing spaces and instead of fixing that, one instance of space use rather > uses something else? > > That sounds like

Re: Issue 4678: Fix spaces in metronome mark (issue 323420043 by beauleetien...@gmail.com)

2017-08-18 Thread dak
On 2017/08/18 03:50:30, Ebe123 wrote: We are not losing spaces, the standard is to ignore trailing whitespace, such as line-breaks and (breaking) spaces at the beginning of tabs. If we represent a space in markup by something in XML (never mind whether or not it looks like a space character)

Doc: Issue 4993 Different time signatures in alternative repeats (issue 323450043 by tdanielsmu...@googlemail.com)

2017-08-20 Thread dak
https://codereview.appspot.com/323450043/diff/1/Documentation/notation/repeats.itely File Documentation/notation/repeats.itely (right): https://codereview.appspot.com/323450043/diff/1/Documentation/notation/repeats.itely#newcode172 Documentation/notation/repeats.itely:172: @warning{If there are

Typo: Fix incorrect \language string from previous (issue 328350043 by pkx1...@gmail.com)

2017-08-26 Thread dak
https://codereview.appspot.com/328350043/diff/1/ly/hel-arabic.ly File ly/hel-arabic.ly (right): https://codereview.appspot.com/328350043/diff/1/ly/hel-arabic.ly#newcode333 ly/hel-arabic.ly:333: major = #`( Any reason to add all standard modes here again? I suppose they'd already be defined anyw

Re: Let Merge_rests_engraver deal with dotted rests (issue 324310043 by thomasmorle...@gmail.com)

2017-08-28 Thread dak
https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm File scm/scheme-engravers.scm (right): https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm#newcode121 scm/scheme-engravers.scm:121: (define-public (Merge_rests_engraver context) Stupid question: is ther

Re: Let Merge_rests_engraver deal with dotted rests (issue 324310043 by thomasmorle...@gmail.com)

2017-08-28 Thread dak
On 2017/08/28 16:01:36, thomasmorley651 wrote: On 2017/08/28 15:18:06, dak wrote: > https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm > File scm/scheme-engravers.scm (right): > > https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm#newco

Re: Let Merge_rests_engraver deal with dotted rests (issue 324310043 by thomasmorle...@gmail.com)

2017-08-29 Thread dak
https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/simultaneous.itely File Documentation/notation/simultaneous.itely (right): https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/simultaneous.itely#newcode10 Documentation/notation/simultaneous.itely:

Re: Let Merge_rests_engraver deal with dotted rests (issue 324310043 by thomasmorle...@gmail.com)

2017-08-29 Thread dak
:10: @c \version "2.19.29" On 2017/08/29 07:39:45, thomasmorley651 wrote: On 2017/08/29 07:31:20, dak wrote: > Version warrants changing to the version where the engraver got register. At > least once you actually make use of the registration... Not sure to which version I

Re: Doc: Add section on SVG to Contributor (issue 328450043 by beauleetien...@gmail.com)

2017-09-04 Thread dak
On 2017/09/04 19:11:57, Ebe123 wrote: While not technically a programming language, PostScript is also listed :) Uh, you are aware that PostScript _is_ a programming language? In contrast, PDF is a file format. That's why there are rather few programs working with or displaying PostScript (yo

Re: Standardize format of `in_color` (issue 329140043 by beauleetien...@gmail.com)

2017-09-10 Thread dak
https://codereview.appspot.com/329140043/diff/1/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): https://codereview.appspot.com/329140043/diff/1/lily/axis-group-interface.cc#newcode991 lily/axis-group-interface.cc:991: .in_color (1, 0, 1)); It's likely mostly a matter of t

Re: EG: a bit of grob-transformer documentation (issue 321620043 by d...@gnu.org)

2017-09-11 Thread dak
https://codereview.appspot.com/321620043/diff/1/Documentation/extending/programming-interface.itely File Documentation/extending/programming-interface.itely (right): https://codereview.appspot.com/321620043/diff/1/Documentation/extending/programming-interface.itely#newcode1386 Documentation/exte

Allow a markup to replace the default LyricHyphen (issue 325470043 by knup...@gmail.com)

2017-09-14 Thread dak
https://codereview.appspot.com/325470043/diff/1/lily/lyric-hyphen.cc File lily/lyric-hyphen.cc (right): https://codereview.appspot.com/325470043/diff/1/lily/lyric-hyphen.cc#newcode55 lily/lyric-hyphen.cc:55: bool use_markup = to_boolean (me->get_property ("use-markup")); That property seems spur

Re: Allow a markup to replace the default LyricHyphen (issue 325470043 by knup...@gmail.com)

2017-09-16 Thread dak
Rats, forgot to publish my code comments. Indepent of those: I don't actually see this addressing issue 1255, so maybe create a new issue in the Sourceforge issue tracker for it and attach this Rietveld issue? It seems at best loosely related to issue #1255 but seems to be desirable nevertheless

Re: Allow a markup to replace the default LyricHyphen (issue 325470043 by knup...@gmail.com)

2017-09-17 Thread dak
On 2017/09/17 10:02:41, knupero wrote: On 2017/09/16 19:53:57, dak wrote: > Indepent of those: I don't actually see this addressing issue 1255, so maybe > create a new issue in the Sourceforge issue tracker for it and attach this > Rietveld issue? It seems at best loos

Re: Allow a markup to replace the default LyricHyphen (issue 325470043 by knup...@gmail.com)

2017-09-17 Thread dak
right): https://codereview.appspot.com/325470043/diff/20001/scm/define-grobs.scm#newcode1403 scm/define-grobs.scm:1403: (text . '()) On 2017/09/17 10:16:01, knupero wrote: On 2017/09/16 19:53:56, dak wrote: > That's the default when unset. For properties that may optionally be set

Re: Add alpha transparency to SVG backend (issue 330300043 by beauleetien...@gmail.com)

2017-09-22 Thread dak
On 2017/09/22 11:19:08, Ebe123 wrote: Add to regression tests I think it is a bad idea to change rgb-color here: this is a recipe for wide-reaching incompatibilities for the sake of limited applications. It would make more sense to meddle with the type of the "transparent" property, making it a

<    1   2   3   4   5   6   7   8   9   10   >