Re: Clean up and fix glyph contour generation nits. (issue 566080043 by hanw...@gmail.com)

*Much* better to read and understand, thanks! LGTM now. https://codereview.appspot.com/566080043/

Re: Clean up and fix glyph contour generation nits. (issue 566080043 by hanw...@gmail.com)

> > I don't fully understand this code. > > that is an ominous comment to come from the Freetype > author. I had assumed you reviewed the existing code > as well. I have two difficulties: I'm not good at C++ and Guile stuff, and it's not completely clear to me what the code wants to achieve. I

Remove deprecated context properties (issue 560030044 by v.villen...@gmail.com)

LGTM. This issue also adds some scheme code and a conversion rule – they are different commits, right? https://codereview.appspot.com/560030044/diff/557800043/scm/define-context-properties.scm File scm/define-context-properties.scm (right): https://codereview.appspot.com/560030044/diff/55780004

Re: Clean up and fix glyph contour generation nits. (issue 566080043 by hanw...@gmail.com)

I don't fully understand this code. Please add more comments. A quite well documented routine to walk over a contour is function `FT_Outline_Decompose` in file `src/base/ftoutln.c` of the FreeType source code. https://codereview.appspot.com/566080043/diff/560030043/lily/freetype.cc File lily/f

Prevent race condition in `-dfont-ps-resdir` (issue 561810045 by truer...@gmail.com)

LGTM, with minor nits. https://codereview.appspot.com/561810045/diff/551840043/scm/backend-library.scm File scm/backend-library.scm (right): https://codereview.appspot.com/561810045/diff/551840043/scm/backend-library.scm#newcode143 scm/backend-library.scm:143: ;; When the directory already exist

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

[forgot to send it previously] https://codereview.appspot.com/569700043/diff/582060043/lily/freetype.cc File lily/freetype.cc (right): https://codereview.appspot.com/569700043/diff/582060043/lily/freetype.cc#newcode126 lily/freetype.cc:126: else if (outline->tags[j] & 1) On 2020/05/08 08:15:58,

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

https://codereview.appspot.com/569700043/diff/573820043/lily/freetype.cc File lily/freetype.cc (right): https://codereview.appspot.com/569700043/diff/573820043/lily/freetype.cc#newcode129 lily/freetype.cc:129: else if (ctags[j] & 1) 1 → FT_CURVE_TAG_ON https://codereview.appspot.com/569700043/d

Issue 4182: avoid checking the offset of cross-staff stems too early (issue 554030043 by barr...@gmail.com)

LGTM https://codereview.appspot.com/554030043/diff/582040043/input/regression/cross-staff-stem-offset.ly File input/regression/cross-staff-stem-offset.ly (right): https://codereview.appspot.com/554030043/diff/582040043/input/regression/cross-staff-stem-offset.ly#newcode5 input/regression/cross-s

Remove define causing a warning (issue 561790043 by jonas.hahnf...@gmail.com)

LGTM https://codereview.appspot.com/561790043/

Re: Add OTC font support for `-dfont-ps-resdir` (issue 548080043 by truer...@gmail.com)

Thanks for the changes. Here the next round of comments :-) https://codereview.appspot.com/548080043/diff/567530058/lily/open-type-font-scheme.cc File lily/open-type-font-scheme.cc (right): https://codereview.appspot.com/548080043/diff/567530058/lily/open-type-font-scheme.cc#newcode467 lily/ope

Add OTC font support for `-dfont-ps-resdir` (issue 548080043 by truer...@gmail.com)

LGTM, with some nits https://codereview.appspot.com/548080043/diff/573790043/Documentation/usage/running.itely File Documentation/usage/running.itely (right): https://codereview.appspot.com/548080043/diff/573790043/Documentation/usage/running.itely#newcode663 Documentation/usage/running.itely:66

Remove ly:lexer-keywords command (issue 559960060 by d...@gnu.org)

LGTM https://codereview.appspot.com/559960060/

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

LGTM https://codereview.appspot.com/569700043/

Calculate skylines only once. (issue 553980043 by hanw...@gmail.com)

LGTM. The figured bass stuff is a separate commit, I guess... https://codereview.appspot.com/553980043/

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

LGTM, thanks! Looks very good, indeed. https://codereview.appspot.com/548030043/

Doc: mention tuplet-slur in Changes and in NR 1.2.1.2 "Tuplets" (issue 559930043 by v.villen...@gmail.com)

LGTM, thanks! https://codereview.appspot.com/559930043/diff/549940043/Documentation/notation/rhythms.itely File Documentation/notation/rhythms.itely (right): https://codereview.appspot.com/559930043/diff/549940043/Documentation/notation/rhythms.itely#newcode275 Documentation/notation/rhythms.ite

Issue 3778: Use bounding box as skylines for markup in svg backend (issue 582010043 by barr...@gmail.com)

LGTM https://codereview.appspot.com/582010043/

Add `-dfont-ps-resdir` option to embed fonts to PDFs later (issue 577900043 by truer...@gmail.com)

Very nice, thanks! There are some grammar glitches in the documentation – I hope that a native English speaker can improve that. https://codereview.appspot.com/577900043/diff/577910043/Documentation/usage/running.itely File Documentation/usage/running.itely (right): https://codereview.appspot.c

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

LGTM https://codereview.appspot.com/577840053/

Transform: add print_smob to aid debugging (issue 561680043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/561680043/

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

LGTM, thanks a lot! https://codereview.appspot.com/551830043/diff/573740043/po/GNUmakefile File po/GNUmakefile (right): https://codereview.appspot.com/551830043/diff/573740043/po/GNUmakefile#newcode14 po/GNUmakefile:14: sed-makestuff = -e 's/[a-zA-Z_/]*make\[[0-9]*\].*//' shouldn't this be rathe

Re: Make dblatex an optional dependency (issue 567480043 by jonas.hahnf...@gmail.com)

LGTM https://codereview.appspot.com/567480043/

stepmake: Remove defunct test template (issue 565960043 by jonas.hahnf...@gmail.com)

LGTM https://codereview.appspot.com/565960043/

Don't use C++ operator synonym "not" (issue 551820043 by d...@gnu.org)

LGTM https://codereview.appspot.com/551820043/

Use binary search in Axis_group_interface::combine_pure_heights. (issue 573730043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/573730043/

Re: Documentation: Add dependency notation.texi -> internals.texi (issue 557720043 by jonas.hahnf...@gmail.com)

LGTM https://codereview.appspot.com/557720043/diff/581940043/Documentation/GNUmakefile File Documentation/GNUmakefile (right): https://codereview.appspot.com/557720043/diff/581940043/Documentation/GNUmakefile#newcode304 Documentation/GNUmakefile:304: $(outdir)/contributor.texi: $(outdir)/ly-gram

Remove unnecessary includes of dispatcher.hh (issue 555700043 by d...@gnu.org)

LGTM https://codereview.appspot.com/555700043/

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

LGTM https://codereview.appspot.com/549920043/

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

Nice! I don't really understand the C++ wizardry, but having to type less macros in the normal code is certainly beneficial. https://codereview.appspot.com/551780043/

stepmake: Remove unused tex template (issue 563870043 by jonas.hahnf...@gmail.com)

LGTM https://codereview.appspot.com/563870043/

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

> 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. https://codereview.appspot.com/549890043/

Issue 5917: ly:performance-headers and \midi { after-writing } (issue 567450043 by nine.fierce.ball...@gmail.com)

LGTM https://codereview.appspot.com/567450043/diff/555680043/lily/performance-scheme.cc File lily/performance-scheme.cc (left): https://codereview.appspot.com/567450043/diff/555680043/lily/performance-scheme.cc#oldcode31 lily/performance-scheme.cc:31: LY_DEFINE (ly_performance_set_header_x, "ly:

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

LGTM - but I don't know the internals well enough to really decide that. 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 that as a macro argument. The former seems to be a rare

Fix comment for Box::unite (issue 559870043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/559870043/diff/553910043/lily/include/box.hh File lily/include/box.hh (right): https://codereview.appspot.com/559870043/diff/553910043/lily/include/box.hh#newcode38 lily/include/box.hh:38: /// smallest box enclosing `this` and `b` Why '///' instead of '//'? h

Re: midi: convert data to bigendian encoding directly (issue 565920043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/565920043/

stencil-integral: factor out repeated scm_car(expr) calls (issue 579590044 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/579590044/

python: Fix compile for in-tree builds (issue 581910043 by jonas.hahnf...@gmail.com)

LGTM https://codereview.appspot.com/581910043/

Execute build scripts directly from source (issue 545870043 by jonas.hahnf...@gmail.com)

LGTM, with some nits. https://codereview.appspot.com/545870043/diff/549860043/make/doc-i18n-root-rules.make File make/doc-i18n-root-rules.make (right): https://codereview.appspot.com/545870043/diff/549860043/make/doc-i18n-root-rules.make#newcode10 make/doc-i18n-root-rules.make:10: find $(outdir)

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

>From visual inspection, LGTM. I only wonder whether we should use if { foo(...); } or if foo(...); for single statements ­– right now, the source code contains both variants... https://codereview.appspot.com/561640043/

Re: Revert "Load only the default font for System_start_delimiter" (issue 557670043 by thomasmorle...@gmail.com)

some nits https://codereview.appspot.com/557670043/diff/569610043/input/regression/system-start-brace-style.ly File input/regression/system-start-brace-style.ly (right): https://codereview.appspot.com/557670043/diff/569610043/input/regression/system-start-brace-style.ly#newcode4 input/regression

Reduce number of calls to Bezier::dir_at_point by 2x (issue 583750044 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/583750044/

Shortcut Rational addition if either operand is zero (issue 551690046 by hanw...@gmail.com)

LGTM. Maybe you can add a comment w.r.t. speed improvement. https://codereview.appspot.com/551690046/

Re: Use python3 in website.make (issue 581880043 by hanw...@gmail.com)

Maybe this can be short-tracked to quickly get rid of the faulty webpage? https://codereview.appspot.com/581880043/

Re: Simplify mf-to-table (issue 549840043 by hanw...@gmail.com)

LGTM (without testing) https://codereview.appspot.com/549840043/

slur-configuration: fix size_t conversion warning (issue 545860043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/545860043/

Re: Cleanup many unused global variables (issue 547890043 by jonas.hahnf...@gmail.com)

LGTM, thanks. BTW, it's 'clean up' if you use it as a verb'. https://codereview.appspot.com/547890043/

Re: Fix line comments for new snippets (issue 549820043 by d...@gnu.org)

> I'd rather not have 2.21.0 in a state inconsistent > with its makelsr program. So if I were to push, > I'd need to push the results of a new makelsr run > as well. OK, I misunderstood. https://codereview.appspot.com/549820043/

Fix line comments for new snippets (issue 549820043 by d...@gnu.org)

LGTM, and please push directly. https://codereview.appspot.com/549820043/

Regtests cleanup: fix texidoc strings (issue 571950043 by v.villen...@gmail.com)

LGTM, thanks! https://codereview.appspot.com/571950043/diff/545810043/input/regression/auto-beam-exceptions.ly File input/regression/auto-beam-exceptions.ly (right): https://codereview.appspot.com/571950043/diff/545810043/input/regression/auto-beam-exceptions.ly#newcode7 input/regression/auto-be

Issue #1204: Document, and add regtest for, external fonts. (issue 557640051 by v.villen...@gmail.com)

Nice idea, thanks. Some nits and questions. https://codereview.appspot.com/557640051/diff/571940043/Documentation/notation/text.itely File Documentation/notation/text.itely (right): https://codereview.appspot.com/557640051/diff/571940043/Documentation/notation/text.itely#newcode1552 Documentati

Re: add property label-alignments to OttavaBracket (issue 575330043 by lilyp...@maltemeyn.de)

Minor doc nits. https://codereview.appspot.com/575330043/diff/549780043/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/575330043/diff/549780043/Documentation/changes.tely#newcode67 Documentation/changes.tely:67: @item The labels can be changed

Re: mf: use python scripting for generating Emmentaler fonts (issue 553580043 by hanw...@gmail.com)

https://codereview.appspot.com/553580043/diff/571810043/mf/gen-emmentaler-brace.fontforge.py File mf/gen-emmentaler-brace.fontforge.py (right): https://codereview.appspot.com/553580043/diff/571810043/mf/gen-emmentaler-brace.fontforge.py#newcode40 mf/gen-emmentaler-brace.fontforge.py:40: # mergeF

Re: Add dynamic-interface to keepAliveInterfaces (issue 553760043 by j...@abou-samra.fr)

LGTM https://codereview.appspot.com/553760043/diff/565830044/Documentation/notation/staff.itely File Documentation/notation/staff.itely (right): https://codereview.appspot.com/553760043/diff/565830044/Documentation/notation/staff.itely#newcode801 Documentation/notation/staff.itely:801: rests, sk

Re: Issue 5864: improve Rational infinity initialization (issue 577710043 by nine.fierce.ball...@gmail.com)

LGTM https://codereview.appspot.com/577710043/

Re: Inline executable-* stepmake templates in lily/GNUmakefile (issue 577690043 by hanw...@gmail.com)

> > What's the reason this isn't called `LOADLIBS`? > > you should ask the GNU project. Aah, indeed, thanks. Well, `LOADLIBES` is deprecated, and the new name is `LDLIBS`. So maybe use `LDLIBS`? https://codereview.appspot.com/577690043/

Re: Issue 5036: 128 beaming output not producing output as expected (?) (issue 559700043 by torsten.haemme...@web.de)

LGTM https://codereview.appspot.com/559700043/diff/545760043/input/regression/beaming-more-than-4-beams-normal-size.ly File input/regression/beaming-more-than-4-beams-normal-size.ly (right): https://codereview.appspot.com/559700043/diff/545760043/input/regression/beaming-more-than-4-beams-normal

Cleanup flower/ makefile (issue 577700045 by hanw...@gmail.com)

LGTM, with nits. https://codereview.appspot.com/577700045/diff/547810068/flower/GNUmakefile File flower/GNUmakefile (right): https://codereview.appspot.com/577700045/diff/547810068/flower/GNUmakefile#newcode17 flower/GNUmakefile:17: TEST_LOADLIBES = $(LIBRARY) $(CXXABI_LIBS) I suggest s/TEST_LOA

Trim unused toplevel targets. (issue 547810069 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/547810069/diff/575870045/GNUmakefile.in File GNUmakefile.in (right): https://codereview.appspot.com/547810069/diff/575870045/GNUmakefile.in#newcode26 GNUmakefile.in:26: RELEASE_FILES = RELEASE-COMMIT Many GNU packages auto-generate a ChangeLog file from the gi

Inline scm stepmake templates (issue 575870044 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/575870044/

Inline elisp stepmake templates. (issue 563780044 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/563780044/

Stop installing TFM and Type1 fonts. (issue 577700043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/577700043/

\compressFullBarRests should be renamed (issue 553750044 by v.villen...@gmail.com)

LGTM, thanks! I have some nits here and there, though. https://codereview.appspot.com/553750044/diff/561590043/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/553750044/diff/561590043/Documentation/changes.tely#newcode66 Documentation/changes.t

Inline executable-* stepmake templates in lily/GNUmakefile (issue 577690043 by hanw...@gmail.com)

>From visual checking I'm not sure whether your changes work as expected... https://codereview.appspot.com/577690043/diff/581870043/lily/GNUmakefile File lily/GNUmakefile (right): https://codereview.appspot.com/577690043/diff/581870043/lily/GNUmakefile#newcode15 lily/GNUmakefile:15: LOADLIBES =

Re: aclocal.m4: Support GUILE_CONFIG, document GUILE_FLAVOR (issue 575860044 by d...@gnu.org)

LGTM, and some minor nits. https://codereview.appspot.com/575860044/diff/547790043/aclocal.m4 File aclocal.m4 (right): https://codereview.appspot.com/575860044/diff/547790043/aclocal.m4#newcode625 aclocal.m4:625: AC_ARG_VAR(GUILE_FLAVOR, AS_HELP_STRING([], What about breaking the line here to ge

Re: flower: Get rid of libc-extension (issue 553740043 by jonas.hahnf...@gmail.com)

https://codereview.appspot.com/553740043/diff/547780043/input/regression/tie-single-manual.ly File input/regression/tie-single-manual.ly (right): https://codereview.appspot.com/553740043/diff/547780043/input/regression/tie-single-manual.ly#newcode19 input/regression/tie-single-manual.ly:19: \ove

Re: scripts/build/scan-mf-deps: script to generate MF dependencies (issue 553700043 by hanw...@gmail.com)

https://codereview.appspot.com/553700043/diff/555460043/mf/invoke-mf2pt.sh File mf/invoke-mf2pt.sh (right): https://codereview.appspot.com/553700043/diff/555460043/mf/invoke-mf2pt.sh#newcode18 mf/invoke-mf2pt.sh:18: # which no longer dump a .mem file > > We could probably get rid of the .mem fil

Re: scripts/build/scan-mf-deps: script to generate MF dependencies (issue 553700043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/553700043/diff/555460043/mf/invoke-mf2pt.sh File mf/invoke-mf2pt.sh (right): https://codereview.appspot.com/553700043/diff/555460043/mf/invoke-mf2pt.sh#newcode18 mf/invoke-mf2pt.sh:18: # which no longer dump a .mem file We could probably get rid of the .mem fi

Remove compat hack for Solaris7 and HP-UX (issue 579480047 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/579480047/

Re: Fix output-distance tests (issue 569540043 by hanw...@gmail.com)

> Are there still systems that run without bash that we care about? I guess all BSD variants might miss bash since it is evil, evil GNU software. https://codereview.appspot.com/569540043/

Re: scripts/build/scan-mf-deps: script to generate MF dependencies (issue 553700043 by hanw...@gmail.com)

https://codereview.appspot.com/553700043/diff/545710043/mf/invoke-mf2pt.sh File mf/invoke-mf2pt.sh (right): https://codereview.appspot.com/553700043/diff/545710043/mf/invoke-mf2pt.sh#newcode4 mf/invoke-mf2pt.sh:4: mf2pt1=$(realpath $1) Second try. According to https://unix.stackexchange.com/

Re: Fix output-distance tests (issue 569540043 by hanw...@gmail.com)

Just skimming the code: LGTM https://codereview.appspot.com/569540043/diff/555440061/scripts/build/output-distance.py File scripts/build/output-distance.py (right): https://codereview.appspot.com/569540043/diff/555440061/scripts/build/output-distance.py#newcode66 scripts/build/output-distance.py

Re: scripts/build/scan-mf-deps: script to generate MF dependencies (issue 553700043 by hanw...@gmail.com)

this looks like a good idea, thanks! https://codereview.appspot.com/553700043/diff/547740044/mf/invoke-mf2pt.sh File mf/invoke-mf2pt.sh (right): https://codereview.appspot.com/553700043/diff/547740044/mf/invoke-mf2pt.sh#newcode4 mf/invoke-mf2pt.sh:4: mf2pt1=$(realpath $1) I only know `realpath`

Re: Added transition lines for lyrics (issue 565750043 by davidgrant...@gmail.com)

LGTM now, thanks! https://codereview.appspot.com/565750043/diff/557620047/scm/define-music-types.scm File scm/define-music-types.scm (right): https://codereview.appspot.com/565750043/diff/557620047/scm/define-music-types.scm#newcode685 scm/define-music-types.scm:685: . ((description . "A transit

Re: configure.ac: Restore check for 'libguile18.h'. (issue 557580043 by lemzw...@googlemail.com)

> configure.ac:195: save_CPPFLAGS="$CXXFLAGS" > Took me some time to find this two-character typo :-( > fixed in staging with commit 7fddbc0ff1 Thanks a lot! https://codereview.appspot.com/557580043/

Re: Added transition lines for lyrics (issue 565750043 by davidgrant...@gmail.com)

> Gould talks specifically about vowels, but I don't see > any reason why it shouldn't apply to sh->ss, or even > from vowel to closed mouth. How about > gradual-syllable-change-event? Mhmm, what about simply `vowel-transition-event`? IMHO it's not necessary to invent new names. https://coderev

Re: Added transition lines for lyrics (issue 565750043 by davidgrant...@gmail.com)

Some more nits :-) https://codereview.appspot.com/565750043/diff/557610044/Documentation/music-glossary.tely File Documentation/music-glossary.tely (right): https://codereview.appspot.com/565750043/diff/557610044/Documentation/music-glossary.tely#newcode415 Documentation/music-glossary.tely:415:

Re: Added transition lines for lyrics (issue 565750043 by davidgrant...@gmail.com)

Looks very nice, thanks! I must admit that I've never seen such a feature before, so I can't really comment on the actual implementation; my nits are just to improve the documentation. However, I wonder why you call this transition *line* and not transition *arrow* ... https://codereview.appspo

Re: Issue 5829: Re-indent all mf files (issue 573610048 by torsten.haemme...@web.de)

Too lazy to check every file, but according to the few samples I looked at: LGTM, thanks! https://codereview.appspot.com/573610048/

Fix Windows resource to enable manifest (issue 551580044 by truer...@gmail.com)

LGTM https://codereview.appspot.com/551580044/

Re: aclocal.m4: Update PKG_* macros to pkg-config 0.29.2 (issue 551590044 by lemzw...@googlemail.com)

Reviewers: hanwenn, hahnjo, Message: > > can we include this rather than copy & paste? > > This would require generating aclocal.m4 by aclocal as > part of generating configure. I'm all for doing this, > but I'd avoid doing this level of tooling changes > before 2.21.0 Seconded. Description: a

Move metafont stepmake templates into mf/GNUmakefile (issue 577610043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/577610043/

make: remove unused mutopia templates (issue 559600043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/559600043/

Calculate download sizes rather than hardcoding them (issue 567340043 by d...@gnu.org)

Very nice, thanks! From visual inspection, LGTM, with only minor nits. https://codereview.appspot.com/567340043/diff/565730043/scripts/build/fix-docsize.sh File scripts/build/fix-docsize.sh (right): https://codereview.appspot.com/567340043/diff/565730043/scripts/build/fix-docsize.sh#newcode37 s

Add some const declarations to page breaking code (issue 569490044 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/569490044/

Use $(MAKE) instead of 'make' throughout (issue 565720043 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/565720043/

Don't add . to PATH in Make (issue 563650043 by d...@gnu.org)

LGTM https://codereview.appspot.com/563650043/

Remove more unused bits from aclocal.m4 (issue 549680043 by jonas.hahnf...@gmail.com)

LGTM https://codereview.appspot.com/549680043/

Re: Issue 5806: Tweak mf files to avoid FontForge internal overlap error (issue 571780043 by torsten.haemme...@web.de)

LGTM, thanks! https://codereview.appspot.com/571780043/

Re: Issue 5806: Tweak mf files to avoid FontForge internal overlap error (issue 571780043 by torsten.haemme...@web.de)

not happy with indentation yet... https://codereview.appspot.com/571780043/diff/549620043/mf/feta-scripts.mf File mf/feta-scripts.mf (left): https://codereview.appspot.com/571780043/diff/549620043/mf/feta-scripts.mf#oldcode999 mf/feta-scripts.mf:999: .. z2l > Nevertheless, I've changed the inden

Re: mf: use python scripting for generating Emmentaler fonts (issue 553580043 by hanw...@gmail.com)

> > Can we assume that FontForge's python support and is > > always enabled? Shall we check this? > > the FF page doesn't say that python is optional. It's a build option in both the old (configure) and new (cmake) builds... https://codereview.appspot.com/553580043/

Re: build cleanups. (issue 547690053 by hanw...@gmail.com)

LGTM https://codereview.appspot.com/547690053/

Re: mf: use python scripting for generating Emmentaler fonts (issue 553580043 by hanw...@gmail.com)

LGTM; some minor nits https://codereview.appspot.com/553580043/diff/571780045/mf/GNUmakefile File mf/GNUmakefile (right): https://codereview.appspot.com/553580043/diff/571780045/mf/GNUmakefile#newcode130 mf/GNUmakefile:130: $(outdir)/emmentaler-brace.otf: emmentaler-brace.fontforge.py $(outdir)/

Re: Issue 5806: Tweak mf files to avoid FontForge internal overlap error (issue 571780043 by torsten.haemme...@web.de)

Very nice, thanks! LGTM. https://codereview.appspot.com/571780043/diff/549620043/mf/feta-arrowheads.mf File mf/feta-arrowheads.mf (right): https://codereview.appspot.com/571780043/diff/549620043/mf/feta-arrowheads.mf#newcode58 mf/feta-arrowheads.mf:58: y5 = 0; Please use an indentation of 8 spa

Add comments to code related to page breaking/layout (issue 563630043 by hanw...@gmail.com)

LGTM. Please feel free to ignore (most of) my remarks if you consider such nitpicking as unnecessary :-) https://codereview.appspot.com/563630043/diff/571770043/lily/include/page-breaking.hh File lily/include/page-breaking.hh (right): https://codereview.appspot.com/563630043/diff/571770043/lily

Fixes for cross-compilation to x86_64-w64-mingw32 (issue 579340043 by jonas.hahnf...@gmail.com)

LGTM https://codereview.appspot.com/579340043/

aclocal.m4: also recognize guile2.2 (issue 555370043 by hanw...@gmail.com)

LGTM. https://codereview.appspot.com/555370043/

Re: Issue 5788: New French Beamimg Approach (issue 557500043 by torsten.haemme...@web.de)

> How about > * beamed-stem-french-adjustment > * french-beaming-stem-adjustment > …? I like the second one. https://codereview.appspot.com/557500043/

Re: aclocal.m4 (STEPMAKE_GUILE_DEVEL): Fix logic and improve diagnostics. (issue 573570044 by lemzw...@googlemail.com)

Reviewers: hahnjo, Message: > Do we really want that many "no"s in the output Currently, you get this (in one long line): checking for guile-1.8 >= 1.8.2... checking for guile-2.2 >= 2.2.0... checking for guile-2.0 >= 2.0.7... Package guile-2.0 was not found in the pkg-config search path. Perh

Re: Issue 5788: New French Beamimg Approach (issue 557500043 by torsten.haemme...@web.de)

> It sounds like actual manipulation of that property > by the user would interfere with the implementation > of french-beaming. So maybe just mark/sort it as an > internal property and only regtest french-beaming as > such? If this is the intention, yes. https://codereview.appspot.com/557500043

Remove outdated compiler checks in configure (issue 551510043 by jonas.hahnf...@gmail.com)

LGTM https://codereview.appspot.com/551510043/

  1   2   3   4   5   6   7   8   >