https://codereview.appspot.com/357770043/diff/20001/configure.ac
File configure.ac (right):
https://codereview.appspot.com/357770043/diff/20001/configure.ac#newcode220
configure.ac:220: CXXFLAGS=-Wcast-function-type
This is more a sort of logic nitpick than of actual relevance given how
GCC is
On 2018/08/15 21:51:46, Dan Eble wrote:
On 2018/08/15 16:43:27, dak wrote:
> This is more a sort of logic nitpick than of actual relevance given
how GCC is
> wired, but if we want to be using -Wno-cast-function-type , that's
what we
> should announce and check for rather than -
https://codereview.appspot.com/348990043/diff/1/lily/includable-lexer.cc
File lily/includable-lexer.cc (right):
https://codereview.appspot.com/348990043/diff/1/lily/includable-lexer.cc#newcode163
lily/includable-lexer.cc:163: for (size_t i = 0; i < count; ++i)
Factoring this out into a separate
Reviewers: thomasmorley651,
Message:
On 2018/10/03 16:56:27, thomasmorley651 wrote:
Even without your patch one can do:
part.1,1 = "foo"
part.1,2 = "bar"
part.1,3 = "buzz"
part.2,1 = "xx"
part.2,2 = "yy"
part.2,3 = "zz"
part.3 = "whatever"
#(format #t "\n\"part\" contains:\n~y\n" part)
#
On 2018/10/03 16:56:27, thomasmorley651 wrote:
Even without your patch one can do:
Oh, I forgot: "even without your patch" points to a misunderstanding.
The sole purpose of this patch is to remove a previously accepted
construct for assignment because, well, I was troubled by a discussion
point
All three look indeed like typos, but all three look like they will have
significant effects warranting individual testing and likely a regtest.
So while that looks bothersome, it might make sense turning this into
three separate issues.
https://codereview.appspot.com/365800043/
I wonder whether it might be reasonable to just have all markup commands
with a last argument of markup? produce their last (recursively treated)
argument as default, and possibly all markup list commands with a last
argument of markup-list? similarly produce their last argument? That
leaves fewe
On 2018/11/11 10:57:31, thomasmorley651 wrote:
On 2018/11/10 12:53:18, dak wrote:
> I wonder whether it might be reasonable to just have all markup
commands with
a
> last argument of markup? produce their last (recursively treated)
argument as
> default, and possibly all markup list
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" }
This looks out of whack with the prece
https://codereview.appspot.com/348050043/diff/1/ly/property-init.ly
File ly/property-init.ly (right):
https://codereview.appspot.com/348050043/diff/1/ly/property-init.ly#newcode34
ly/property-init.ly:34: ambitusAfterKeySignature = {
On 2018/12/15 10:40:36, Malte Meyn wrote:
Is this the correct
: 1-\markup { \super "5" }
On 2018/12/13 11:32:34, dak wrote:
> The way this is arranged currently, it reads jarringly.
I agree, there are a few things that look quite bad: among others,
\partialJazzMusic should be rewritten (and its use is hardly
documented);
there’s a TOD
https://codereview.appspot.com/369930043/diff/20001/python/convertrules.py
File python/convertrules.py (right):
https://codereview.appspot.com/369930043/diff/20001/python/convertrules.py#newcode3977
python/convertrules.py:3977: str = re.sub (r"\\partcombine",
r"\\partCombine", str)
Uh, you real
https://codereview.appspot.com/345160043/diff/1/lily/relocate.cc
File lily/relocate.cc (right):
https://codereview.appspot.com/345160043/diff/1/lily/relocate.cc#newcode376
lily/relocate.cc:376: if (command == "set?")
Where/how is that being used? Wouldn't this want documenting?
https://codere
https://codereview.appspot.com/365840043/diff/60001/scm/modal-transforms.scm
File scm/modal-transforms.scm (right):
https://codereview.appspot.com/365840043/diff/60001/scm/modal-transforms.scm#newcode335
scm/modal-transforms.scm:335: (ly:music-transpose
Shouldn't this also set/adjust the octava
https://codereview.appspot.com/365840043/diff/80001/scm/modal-transforms.scm
File scm/modal-transforms.scm (right):
https://codereview.appspot.com/365840043/diff/80001/scm/modal-transforms.scm#newcode324
scm/modal-transforms.scm:324: (sorted (sort pitches ly:pitchhttps://codereview.appspot.com/
On 2019/01/28 20:02:53, Valentin Villenave wrote:
I can’t figure out how to make it work without resetting the
EventChord’s
elements list. How would you proceed?
Without any pointer to what you are having problems with, this is
essentially "do it yourself". Sigh. I don't even understand wh
On 2019/01/28 21:53:04, dak wrote:
On 2019/01/28 20:02:53, Valentin Villenave wrote:
> I can’t figure out how to make it work without resetting the
EventChord’s
> elements list. How would you proceed?
Without any pointer to what you are having problems with, this is
essentially
https://codereview.appspot.com/365840043/diff/130001/ly/music-functions-init.ly
File ly/music-functions-init.ly (right):
https://codereview.appspot.com/365840043/diff/130001/ly/music-functions-init.ly#newcode609
ly/music-functions-init.ly:609: (let loop ((num num) (music music))
Come to think o
https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc
File lily/relocate.cc (right):
https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc#newcode128
lily/relocate.cc:128: TOPLEVEL_VERSION, package_datadir));
Why no .c_str () here (and lots of other places)?
https://coder
On 2019/02/08 09:49:58, lemzwerg wrote:
On 2019/02/08 09:42:27, dak wrote:
> https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc
> File lily/relocate.cc (right):
>
>
https://codereview.appspot.com/347070043/diff/1/lily/relocate.cc#newcode128
> lily/
On 2019/02/08 09:46:17, Valentin Villenave wrote:
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
On 2019/02/08 11:41:45, lemzwerg wrote:
> Can you give any reason why it _should_ work rather than "I tried"?
flower/include/international.hh:46
Oh good grief.
string _f (char const *format, ...)
__attribute__ ((format (printf, 1, 2)));
string _f (char const *format, const string &s, const
On 2019/02/08 12:45:43, Valentin Villenave wrote:
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 th
On 2019/02/08 17:05:46, Valentin Villenave wrote:
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 :-)
(not (null? xxx)) vs (not-n
https://codereview.appspot.com/221710044/diff/60001/lily/general-scheme.cc
File lily/general-scheme.cc (right):
https://codereview.appspot.com/221710044/diff/60001/lily/general-scheme.cc#newcode150
lily/general-scheme.cc:150: LY_DEFINE (ly_rassoc, "ly:rassoc",
I am not sure this is wanted (ther
On 2019/02/12 16:24:37, Valentin Villenave wrote:
On 2019/02/11 22:02:11, dak wrote:
> Doing a single call of
> make_markup_concat (scm_list_3 (markup1, markup2, markup3));
> does not seem too bad and the export/import is a one-time cost for
all
> prospective uses. I am surpri
https://codereview.appspot.com/353880043/diff/2/lily/beam-quanting.cc
File lily/beam-quanting.cc (right):
https://codereview.appspot.com/353880043/diff/2/lily/beam-quanting.cc#newcode1039
lily/beam-quanting.cc:1039: string card = best->score_card_ + to_string
(" c%d/%lu", completed, configs.siz
, dak wrote:
> This code is insane. Maybe just outcomment the whole else if branch
instead?
Well, option `--relocate' should (1) be recognized (i.e., it shouldn't
produce
an error), but at the same time it should (2) be a no-op. Removing
the branch
would disable (1)...
Where d
https://codereview.appspot.com/348120043/diff/1/input/regression/script-stencil.ly
File input/regression/script-stencil.ly (right):
https://codereview.appspot.com/348120043/diff/1/input/regression/script-stencil.ly#newcode31
input/regression/script-stencil.ly:31: #(append! default-script-alist
/02/22 20:41:08, Valentin Villenave wrote:
On 2019/02/22 16:19:53, dak wrote:
> There is no point in renaming this into get_glyph_or_stencil if it
cannot
return
> anything but a stencil.
The point was to avoid any risk of confusion with Grob::get_stencil
(). If you
find it unnecessary
On 2019/02/22 21:51:39, Valentin Villenave wrote:
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 w
On 2019/02/24 08:48:22, Valentin Villenave wrote:
I’m still bothered by the way script-interface.cc is written,
specifically the
hardcoded "feta" reference which I tried to address; do you have any
thoughts on
that? (Other than that, it appears there’s very little to salvage from
my patch
https://codereview.appspot.com/351880043/diff/20001/lily/page-turn-page-breaking.cc
File lily/page-turn-page-breaking.cc (right):
https://codereview.appspot.com/351880043/diff/20001/lily/page-turn-page-breaking.cc#newcode37
lily/page-turn-page-breaking.cc:37: bool page_breakable = scm_is_symbol
https://codereview.appspot.com/572520044/diff/544550044/Documentation/notation/input.itely
File Documentation/notation/input.itely (right):
https://codereview.appspot.com/572520044/diff/544550044/Documentation/notation/input.itely#newcode246
Documentation/notation/input.itely:246: \paper @{ @do
Thanks for looking at this, Keith! I've tried to make a version that
does the right thing with regard to the assertion failure. That does
not address the root cause of the problem, nor does it fix the centering
thing you pointed out. Quite lacklustre.
https://codereview.appspot.com/572550043/
LGTM
https://codereview.appspot.com/554590044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Multimeasure rests can be split into multiple rests and across lines.
That's what makes then a Spanner rather than an Item and is the main
cause of this complication. How are you dealing with such a split? A
fermata only makes sense on the last such rest, most text markups make
sense only on the
https://codereview.appspot.com/560670043/diff/550690043/lily/line-spanner.cc
File lily/line-spanner.cc (right):
https://codereview.appspot.com/560670043/diff/550690043/lily/line-spanner.cc#newcode395
lily/line-spanner.cc:395: "Sets the Y-coordinate of the end point, in
staff-spaces"
On 2019/05/
https://codereview.appspot.com/231680043/diff/11/lily/lily-guile.cc
File lily/lily-guile.cc (right):
https://codereview.appspot.com/231680043/diff/11/lily/lily-guile.cc#newcode459
lily/lily-guile.cc:459: // unsmob delivers true. This means that
unsmob is a
The resulting comment text is
Reviewers: benko.pal, lemzwerg,
https://codereview.appspot.com/58343/diff/568950043/Documentation/snippets/figured-bass-headword.ly
File Documentation/snippets/figured-bass-headword.ly (right):
https://codereview.appspot.com/58343/diff/568950043/Documentation/snippets/figured-bass-headw
https://codereview.appspot.com/581110043/diff/569040043/input/regression/tuplet-slur-tweaks.ly
File input/regression/tuplet-slur-tweaks.ly (right):
https://codereview.appspot.com/581110043/diff/569040043/input/regression/tuplet-slur-tweaks.ly#newcode1
input/regression/tuplet-slur-tweaks.ly:1: \v
On 2019/10/21 09:52:41, thomasmorley651 wrote:
Does this one need a convert-rule?
If so, I'd need some help. My python-skill is more or less zero.
I've just taken some look. It would appear that the only other markup
commands using the "offset" property are the \tie/\overtie\undertie
family
On 2019/10/21 11:02:12, thomasmorley651 wrote:
On 2019/10/21 10:31:32, dak wrote:
> On 2019/10/21 09:52:41, thomasmorley651 wrote:
> > Does this one need a convert-rule?
> >
> > If so, I'd need some help. My python-skill is more or less zero.
>
> I've just t
On 2019/10/21 11:13:17, dak wrote:
On 2019/10/21 11:02:12, thomasmorley651 wrote:
> On 2019/10/21 10:31:32, dak wrote:
> > On 2019/10/21 09:52:41, thomasmorley651 wrote:
> > > Does this one need a convert-rule?
> > >
> > > If so, I'd need some h
On 2019/10/21 15:36:04, thomasmorley651 wrote:
Then we would have the following properties
((thickness 1) (offset 2) (underline-shift 0) (underline-offset 2))
A user-override for underline-shift would disturb things, though.
Maybe some
words in the doc-string about it?
Having both offset
On 2019/10/26 14:57:32, Dan Eble wrote:
I don't have time right now to test that this still works when tidy is
installed, but since it's blocking others, I see no problem with
pushing it to
staging.
It's not just that there "is no problem". Without working test
procedures, the state of this
On 2019/10/29 22:00:09, Dan Eble wrote:
On 2019/10/29 19:38:39, hahnjo wrote:
> Anyway, for now building the "latest" Python 2.4.6 is actually not
too hard.
...
> This can be passed via PYTHON= to LilyPond's configure -
> without any weird environment variables like LD_LIBRARY_PATH.
Should t
On 2019/11/14 09:11:35, lemzwerg wrote:
> And yes, this works for me, at least on Linux (didn't test FreeBSD
yet and
don't
> own a Mac). Plus it passes a full check with a test-baseline
generated by GCC,
> so I'm pretty confident it's also working correctly.
Ho humm :-) David K?
This is
On 2019/12/11 13:55:56, Dan Eble wrote:
On 2019/12/11 07:17:30, hahnjo wrote:
> On 2019/12/10 22:28:46, Dan Eble wrote:
> > I don't know if the compiler that GUB uses properly supports
C++11,
> > and I'm not yet interested in spending my own time to investigate.
>
> GUB builds GCC 4.9.4 which i
On 2019/12/14 10:18:33, hahnjo wrote:
Fabulous, thanks for putting this together!
I don't want to get your enthusiasm up prematurely, but I think that the
GUB GCC versions we need(?)/use for PowerPC might not be entirely great
with C++11 though they'll likely support it when given explicitly on
On 2019/12/15 14:40:08, dan_faithful.be wrote:
On Dec 15, 2019, at 06:31, mailto:lilyp...@de-wolff.org wrote:
> It is not the commit title, but I do think that this is not a part
of issue
5639: compile with --std=c11
> The reason that I think it is important to keep this separated is
that the
On 2019/12/16 16:51:59, Dan Eble wrote:
On 2019/12/15 06:29:59, lemzwerg wrote:
> Well, the cis–trans pairing is IMHO only understandable for people
who have a
> Latin background (or come from Austria, since it was divided into
> »Cisleithanien« and »Transleithanien« in the K. und K. era)... A
On 2019/12/17 17:57:52, Dan Eble wrote:
This is ready for a full review now.
I don't think any new test cases are required.
I haven't looked very deeply into the potential use of an unpure-pure
container.
It looks like unpure-pure containers are used for vertical placement,
They are f
On 2019/12/20 00:01:59, lemzwerg wrote:
> > There should always a comma after 'i.e.', as far as I know.
>
> Nope. It's grammatical dogma (in the same vein as not
> starting sentences with the words 'And' or 'Because).
> I think commas after these abbreviations look ugly.
I'm not a native spea
Frankly, I am disconcerted that the original code runs through a float
conversion in the first place. This is not as much about removing a
warning rather than fixing bad code since the warning had a point.
https://codereview.appspot.com/581400043/diff/577250043/flower/rational.cc
File flower/ra
https://codereview.appspot.com/561290043/diff/583280043/lily/context.cc
File lily/context.cc (right):
https://codereview.appspot.com/561290043/diff/583280043/lily/context.cc#newcode116
lily/context.cc:116: if (Context *score = gthis->get_score_context ())
On 2020/01/03 16:18:09, lemzwerg wrote:
https://codereview.appspot.com/363880043/diff/80001/python/convertrules.py
File python/convertrules.py (right):
https://codereview.appspot.com/363880043/diff/80001/python/convertrules.py#newcode3980
python/convertrules.py:3980: if re.search
(r"#[banter|jazz]-chord-names", str):
I have no idea w
https://codereview.appspot.com/549350043/diff/581410043/configure.ac
File configure.ac (right):
https://codereview.appspot.com/549350043/diff/581410043/configure.ac#newcode7
configure.ac:7: AC_INIT([LilyPond], [2.21.0], [bug-lilyp...@gnu.org],
[lilypond], [http://lilypond.org/])
Hardwiring the
On 2020/01/08 17:48:36, hahnjo wrote:
On 2020/01/08 16:43:24, dak wrote:
> mailto:jonas.hahnf...@gmail.com writes:
>
> > Reviewers: dak,
> >
> > Message:
> > On 2020/01/08 16:18:25, dak wrote:
> >>
https://codereview.appspot.com/549350043/diff/5814100
On 2020/01/08 18:03:36, dak wrote:
>
> But re-reading your question, you're maybe proposing to have a file
that is
> included by Autoconf when generating configure? I think that's not
possible
with
> the documented functionality of Autoconf because you can only do
ver
On 2020/01/08 19:49:22, hahnjo wrote:
On 2020/01/08 19:40:06, c_sorensen wrote:
> How about
>
> AC_INIT([LilyPond],m4_esyscmd(echo `VERSION.AC'))
>
> The documentation says it is permissible to use m4_esyscmd as part
of the
> package information strings in AC_INIT.
>
> Carl
Not quite, but a
On 2020/01/08 20:30:20, hahnjo wrote:
On 2020/01/08 20:14:20, dak wrote:
> On 2020/01/08 19:49:22, hahnjo wrote:
> > On 2020/01/08 19:40:06, c_sorensen wrote:
> > > How about
> > >
> > > AC_INIT([LilyPond],m4_esyscmd(echo `VERSION.AC'))
> > >
>
Reviewers: hahnjo,
https://codereview.appspot.com/583290043/diff/557180043/python/convertrules.py
File python/convertrules.py (right):
https://codereview.appspot.com/583290043/diff/557180043/python/convertrules.py#newcode1774
python/convertrules.py:1774: str += "'" * (o + 1)
On 2020/01/08 08:17
https://codereview.appspot.com/566920044/diff/549120043/python/convertrules.py
File python/convertrules.py (right):
https://codereview.appspot.com/566920044/diff/549120043/python/convertrules.py#newcode3898
python/convertrules.py:3898: mods = str.join (re.findall ("\n(" +
wordsyntax + r")\s*=\s
On 2020/01/12 01:13:51, dak wrote:
https://codereview.appspot.com/566920044/diff/549120043/python/convertrules.py
File python/convertrules.py (right):
https://codereview.appspot.com/566920044/diff/549120043/python/convertrules.py#newcode3898
python/convertrules.py:3898: mods = str.join
On 2020/01/14 13:52:02, hahnjo wrote:
Is this ok to push in its current revision?
I am not overly happy that it makes for inoperative code in the case of
a read-only repository that autogen.sh tries catering for. Can/should
autogen.sh pass some sort of environment variable that this can be
pic
On 2020/01/14 14:12:47, hahnjo wrote:
On 2020/01/14 13:59:43, dak wrote:
> On 2020/01/14 13:52:02, hahnjo wrote:
> > Is this ok to push in its current revision?
>
> I am not overly happy that it makes for inoperative code in the case
of a
> read-only repository that autogen
On 2020/01/14 14:48:44, Dan Eble wrote:
On 2020/01/14 14:43:34, hahnjo wrote:
> On 2020/01/14 14:29:38, Dan Eble wrote:
> > On 2020/01/14 13:59:43, dak wrote:
> > > On 2020/01/14 13:52:02, hahnjo wrote:
> > > I am not overly happy that it makes for inoperative code in
On 2020/01/14 15:07:48, dak wrote:
On 2020/01/14 14:48:44, Dan Eble wrote:
> On 2020/01/14 14:43:34, hahnjo wrote:
> > On 2020/01/14 14:29:38, Dan Eble wrote:
> > > On 2020/01/14 13:59:43, dak wrote:
> > > > On 2020/01/14 13:52:02, hahnjo wrote:
> > > &g
On 2020/01/14 15:19:09, dak wrote:
On 2020/01/14 15:07:48, dak wrote:
> On 2020/01/14 14:48:44, Dan Eble wrote:
> > On 2020/01/14 14:43:34, hahnjo wrote:
> > > On 2020/01/14 14:29:38, Dan Eble wrote:
> > > > On 2020/01/14 13:59:43, dak wrote:
> > > &
On 2020/01/14 16:35:50, Dan Eble wrote:
On 2020/01/14 15:54:28, dak wrote:
> +AC_INIT([LilyPond],
> +[m4_esyscmd_s([. ${SRCDIR:-.}/VERSION; echo
I was about to test it, but the main patch no longer applies to
master. I'll
try against an older version.
I do wonder whet
https://codereview.appspot.com/549350043/diff/545440043/configure.ac
File configure.ac (right):
https://codereview.appspot.com/549350043/diff/545440043/configure.ac#newcode8
configure.ac:8: [m4_esyscmd([. ${SRCDIR:-.}/VERSION; echo -n
$MAJOR_VERSION.$MINOR_VERSION.$PATCH_LEVEL])],
The proposal
On 2020/01/16 11:22:24, hahnjo wrote:
On 2020/01/16 10:57:46, dak wrote:
> https://codereview.appspot.com/549350043/diff/545440043/configure.ac
> File configure.ac (right):
>
>
https://codereview.appspot.com/549350043/diff/545440043/configure.ac#newcode8
> configure.ac
LGTM
https://codereview.appspot.com/545450043/diff/583370043/lily/lily-guile.cc
File lily/lily-guile.cc (right):
https://codereview.appspot.com/545450043/diff/583370043/lily/lily-guile.cc#newcode55
lily/lily-guile.cc:55: scm_write(s, port);
Well, I was "what the Dickens?" reading this. I suppo
https://codereview.appspot.com/555160043/diff/569260043/lily/slur.cc
File lily/slur.cc (left):
https://codereview.appspot.com/555160043/diff/569260043/lily/slur.cc#oldcode576
lily/slur.cc:576: "inspect-index "
Is the removal of the inspect-index property intentional? It appears
unused, but at
LGTM
https://codereview.appspot.com/555170043/
On 2020/01/22 18:51:07, Erlend Aasland wrote:
> On 2020/01/22 16:20:21, Dan Eble wrote:
> > Veto! I've had more than one bad experience with lowercase ell.
> >
> > In one case--I think it was elsewhere in LilyPond--it was purely a
readability
> > issue, but reason enough to avoid it.
> >
> > In t
On 2020/01/23 11:33:11, lemzwerg wrote:
> I'm quite sure that you have more experience with C++ than me,
however, I'm not
> really happy with this change since it makes the code substantially
uglier.
> Isn't there another solution for the problem?
>
>
https://codereview.appspot.com/579240043/diff
On 2020/01/23 12:43:34, lemzwerg wrote:
> > Why is this ugly? std::string is really the name of the class.
>
> No question, but I would prefer
>
> struct xxx {
> using std:string;
>
> string foo;
> string bar;
> string baz;
> }
> }
>
> to
>
> struct xxx {
> std::strin
On 2020/01/23 14:36:26, dak wrote:
> On 2020/01/23 12:43:34, lemzwerg wrote:
> > > Why is this ugly? std::string is really the name of the class.
> >
> > No question, but I would prefer
> >
> > struct xxx {
> > using std:string;
> >
> &
https://codereview.appspot.com/575540045/diff/571410043/scm/documentation-generate.scm
File scm/documentation-generate.scm (right):
https://codereview.appspot.com/575540045/diff/571410043/scm/documentation-generate.scm#newcode98
scm/documentation-generate.scm:98: (if (defined? 'set-port-encoding
Not sure about the speedup (we might have small sizes often enough that
the constant factor becomes relevant), but the code is quite a net win
with regard to obviously doing what it should do. It probably takes a
whole lot more allocation calls, but at least the impact on memory is
just temporary.
On 2020/01/24 13:39:15, hahnjo wrote:
> On 2020/01/24 11:22:43, Dan Eble wrote:
> > I'd like special permission to skip the countdown and push this
today instead
> of
> > tomorrow. Would that be OK?
>
> Not my call, but maybe you can give a reason? Is it the expected merge
> conflicts? If so, it'
On 2020/01/24 14:11:59, Dan Eble wrote:
> On 2020/01/24 14:06:22, hanwenn wrote:
> > If the allocation cost becomes problematic, we can use another
hashmap
> instead.
>
> "We" could profile it and know before pushing whether it is worth it.
I don't think that this is really performance-critical.
https://codereview.appspot.com/583390043/diff/551390044/lily/grob.cc
File lily/grob.cc (right):
https://codereview.appspot.com/583390043/diff/551390044/lily/grob.cc#newcode971
lily/grob.cc:971: int j = 0;
vsize j = 0;
:-)
https://codereview.appspot.com/583390043/
On 2020/01/24 13:57:33, Dan Eble wrote:
> On 2020/01/24 13:45:50, dak wrote:
> > Hoo yes. That kind of extensive change is going to hurt anyway
given the
> > current burst activity level. It will likely suck either way. Do
you have
> > particular dependencies yourself, Dan
On 2020/01/24 15:26:06, dak wrote:
> On 2020/01/24 13:57:33, Dan Eble wrote:
> > On 2020/01/24 13:45:50, dak wrote:
> > > Hoo yes. That kind of extensive change is going to hurt anyway
given the
> > > current burst activity level. It will likely suck either way. D
On 2020/01/24 15:50:22, Dan Eble wrote:
> On 2020/01/24 15:17:02, dak wrote:
> > On 2020/01/24 14:11:59, Dan Eble wrote:
> > > On 2020/01/24 14:06:22, hanwenn wrote:
> > > > If the allocation cost becomes problematic, we can use another
hashmap
> > > instead.
That seems weird for me. The topic states "this provides a way to run
LilyPond with Guile 2.2" but garbage collection with Guile 2.2 works out
of the box already. This patch only causes extra work and will slow
down garbage collection further.
The only way in which it could make sense is if in r
https://codereview.appspot.com/575530107/diff/581530050/lily/spaceable-grob.cc
File lily/spaceable-grob.cc (right):
https://codereview.appspot.com/575530107/diff/581530050/lily/spaceable-grob.cc#newcode92
lily/spaceable-grob.cc:92: break;
That looks still too complicated. Just do return *next_s
On 2020/01/26 10:45:11, thomasmorley651 wrote:
> I don't see any difference to checking for a number either. Thus
"distinguish"
> feels strange.
There was a time when music function predicates determined syntactic
decisions based on what function they pointed to rather than what they
evaluated to.
https://codereview.appspot.com/571430046/diff/551390052/lily/main.cc
File lily/main.cc (right):
https://codereview.appspot.com/571430046/diff/551390052/lily/main.cc#newcode830
lily/main.cc:830: catch (std::exception e)
On 2020/01/26 14:24:02, hahnjo wrote:
> On 2020/01/26 14:18:20, hanwenn wrote
.com/575540045/diff/571410043/scm/documentation-generate.scm#newcode98
> > scm/documentation-generate.scm:98: (if (defined?
'set-port-encoding!)
> > On 2020/01/24 11:08:51, dak wrote:
> > > I think we usually do this as
> > >
> > > (if (guile-v2) ...
>
https://codereview.appspot.com/577410045/diff/571430047/lily/include/parse-scm.hh
File lily/include/parse-scm.hh (right):
https://codereview.appspot.com/577410045/diff/571430047/lily/include/parse-scm.hh#newcode30
lily/include/parse-scm.hh:30: SCM parse_embedded_scheme (Input *i, bool
safe, Lily
On 2020/01/29 00:06:14, Dan Eble wrote:
Pretty sure that the trigger was
commit ed86e71eb0f17e93036142938d8c490a3e2fcbea
Author: Valentin Villenave
Date: Mon Feb 25 17:24:11 2019 +0100
Use "simple strings" rather than #"hash-prefixed Scheme strings"
Now that the user-exposed synt
On 2020/01/29 03:08:48, Dan Eble wrote:
> change the test, not the display function
Oh, I am sorry to have been misleading with my comment: I think your
original patch was the right thing to do. I just remarked that what
display-lily printed would have also worked as input so this was not
really
On 2020/01/29 06:36:07, hanwenn wrote:
>
> BTW - I don't want to tell a C++ expert how to use the language in
general. If
> we were in an alternate reality where we could start from scratch we
could
> reconsider the decision to not use non-const references in structs and
function
> arguments. As
https://codereview.appspot.com/577410045/diff/551410043/lily/parse-scm.cc
File lily/parse-scm.cc (right):
https://codereview.appspot.com/577410045/diff/551410043/lily/parse-scm.cc#newcode59
lily/parse-scm.cc:59:
Pouring oil on the fire... Rietveld highlighting indicates non-empty
lines consist
On 2020/01/29 12:20:10, mail5 wrote:
> Unfortunately I haven't set up a build system on my new computer yet,
so this
> patch is not tested locally at all, so I'm humbly waiting for the
automated
> tests to succeed or fail ...
I don't like the "use-modules clauses adjusted accordingly". I don't
th
https://codereview.appspot.com/572600048/diff/548660043/Documentation/snippets/new/non-traditional-key-signatures.ly
File Documentation/snippets/new/non-traditional-key-signatures.ly
(right):
https://codereview.appspot.com/572600048/diff/548660043/Documentation/snippets/new/non-traditional-key-s
301 - 400 of 1900 matches
Mail list logo
