Re: Remove references to test-output-distance.ly. (issue 549780044 by hanw...@gmail.com)

[Dan Eble]

On Mar 30, 2020, at 17:34, Han-Wen Nienhuys  wrote:
> 
> On Sun, Mar 29, 2020 at 11:45 PM Dan Eble  wrote:
>> 
>> On Mar 29, 2020, at 17:39, Han-Wen Nienhuys  wrote:
 test-output-distance was removed on the grounds that the self-test
 serves the same purpose, but I don't see how it does.
>>> 
>>> Could you elaborate? What failure scenario are you worried about?
>> 
>> My question is, how does the self-test "verify that the regression tests 
>> have, in fact, run"?  I don't see how it could do more than verify that 
>> itself has run.
> 
> How does test-output-distance verify that the regression tests have run?
> 
> The verification doesn't come from the test file. It comes from the
> fact that someone is looking at the test output.

(Rhetorically) How much verification comes from looking at the output since you 
removed the test?

> The self-test gives us assurance that the test result index.html file
> is making sense.

Yes, and that does not overlap with the purpose of test-output-distance.

> You can verify that the tests were run by checking
> that the index.html file is there.

I think the only thing that the presence of index.html implies is that 
output-distance.py was run.

> For further context, I am interested in automating our test setup
> further: I want to have CI that can post feedback ("test failed!") on
> code reviews automatically. test-output-distance is a test that always
> fails, which makes that more complicated.

This is what the description of the commit that removed test-output-distance 
ought to have been.

I've been trying to point out that something of value was lost.  This last 
statement of yours finally gives something to weigh that value against.
-- 
Dan

Re: Remove references to test-output-distance.ly. (issue 549780044 by hanw...@gmail.com)

[Han-Wen Nienhuys]

On Sun, Mar 29, 2020 at 11:45 PM Dan Eble  wrote:
>
> On Mar 29, 2020, at 17:39, Han-Wen Nienhuys  wrote:
> >> test-output-distance was removed on the grounds that the self-test
> >> serves the same purpose, but I don't see how it does.
> >
> > Could you elaborate? What failure scenario are you worried about?
>
> My question is, how does the self-test "verify that the regression tests 
> have, in fact, run"?  I don't see how it could do more than verify that 
> itself has run.

How does test-output-distance verify that the regression tests have run?

The verification doesn't come from the test file. It comes from the
fact that someone is looking at the test output.

The self-test gives us assurance that the test result index.html file
is making sense. You can verify that the tests were run by checking
that the index.html file is there.

For further context, I am interested in automating our test setup
further: I want to have CI that can post feedback ("test failed!") on
code reviews automatically. test-output-distance is a test that always
fails, which makes that more complicated.

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: Remove all uses of markup macro from scm/*.scm (issue 575930043 by d...@gnu.org)

[dak]

https://codereview.appspot.com/575930043/diff/561620044/scm/time-signature.scm
File scm/time-signature.scm (right):

https://codereview.appspot.com/575930043/diff/561620044/scm/time-signature.scm#newcode23
scm/time-signature.scm:23: (mup (if (procedure? proc)
On 2020/03/30 21:15:15, hanwenn wrote:
> nitpick: how about 'fraction-markup' or 'fraction' ?

Done.

Strictly speaking, the change was likely not necessary in the first
place, but I'd hate keeping (markup ... around in a file, even though it
means something entirely different here.

https://codereview.appspot.com/575930043/diff/561620044/scm/titling.scm
File scm/titling.scm (right):

https://codereview.appspot.com/575930043/diff/561620044/scm/titling.scm#newcode102
scm/titling.scm:102: (mup (ly:output-def-lookup layout what)))
On 2020/03/30 21:15:15, hanwenn wrote:
> title-markup ?
> 

Done.

https://codereview.appspot.com/575930043/

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

[hanwenn]

On 2020/03/30 12:20:17, dak wrote:
> On 2020/03/29 14:35:43, hanwenn wrote:
> > On Sun, Mar 29, 2020 at 4:12 PM  wrote:
> > >
> > > On 2020/03/29 13:55:41, hanwenn wrote:
> > > > On 2020/03/29 13:52:48, hanwenn wrote:
> > > > > retain existing
> > > >
> > > > how's this? This leaves things backward compatible.
> > >
> > > Ugly and a maintenance burden since the code is used twice. 
Anything
> > > wrong with my proposal?
> > 
> > I didn't understand your proposal.
> > 
> > > It does not have duplicate code, makes
> > > define-markup-command compilable (while requiring its toplevel
use) and
> > > provides a way of doing the same consistently for module-specific
rather
> > > than toplevel use.
> > >
> > > It sacrifices, like your proposal, non-toplevel-performance for
the sake
> > > of compilability in Guilev2.  It's just that what the parser then
uses
> > > is in a form that could also be used in a reasonably natural
manner from
> > > Scheme.
> > >
> > > Should I write up a patch doing that?
> > 
> > Yes please.
> > 
> 
> Working on it.  By the way, it just occured to me that a whole lot of
trouble
> with byte-compilation is caused by the markup macro.

Yes, it was the next thing I wanted to look at, but you beat me to the
punch.


https://codereview.appspot.com/577720043/

Re: Remove all uses of markup macro from scm/*.scm (issue 575930043 by d...@gnu.org)

[hanwenn]

thanks!

Harm, the changes only need to be done in .scm, because it's only those
files that we'd be precompiling


https://codereview.appspot.com/575930043/diff/561620044/scm/time-signature.scm
File scm/time-signature.scm (right):

https://codereview.appspot.com/575930043/diff/561620044/scm/time-signature.scm#newcode23
scm/time-signature.scm:23: (mup (if (procedure? proc)
nitpick: how about 'fraction-markup' or 'fraction' ?

https://codereview.appspot.com/575930043/diff/561620044/scm/titling.scm
File scm/titling.scm (right):

https://codereview.appspot.com/575930043/diff/561620044/scm/titling.scm#newcode102
scm/titling.scm:102: (mup (ly:output-def-lookup layout what)))
title-markup ?

https://codereview.appspot.com/575930043/

Re: Remove all uses of markup macro from scm/*.scm (issue 575930043 by d...@gnu.org)

[hanwenn]

LGTM

https://codereview.appspot.com/575930043/

Remove all uses of markup macro from scm/*.scm (issue 575930043 by d...@gnu.org)

[thomasmorley65]

patch itself LGTM

Ofcourse there are other folders than scm/*.scm, like snippets,
snippets/new, ly-examples, Documentation...
Follow up planned?

https://codereview.appspot.com/575930043/

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

[v . villenave]

On 2020/03/30 18:56:27, thomasmorley651 wrote:
> tmpnam is deprecated in guile-3.0.2

(sigh) Nice catch!

> Why introducing it, we would need to move away from it later anyway?

Well, I *would* have used mkstemp, but I need to create a directory
(rather than a file), and there’s no mkdtemp in Guile.  Plus, I wanted
the code to be supported by both Guile 1.8 and 2.  Plus, we’re already
using tmpnam somewhere else in the codebase.  Plus…

> Also, the link you provided speaks about security risks...

Yes, so I’ve seen.  I certainly *can* imagine how that could be
exploited in a critical case.  (And by letting users have access to a
Turing-complete language within LilyPond files, our exposure to security
faults is already quite wide.)  Then again, that’s a regtest we’re
talking about, not a snippet or a doc example, not something we’re
actively encouraging people to do at home.

Frankly, if anyone can think of a solution to do the exact same thing
with mkstemp (and to remove the temporary files after compilation), I’m
all ears.  As of right now, I know I can’t :-)

V.

https://codereview.appspot.com/557640051/

Re: GSoC Proposal - SVG Export

[Étienne Beaulé]

To re-open this thread:

Sorry if I wasn’t clear enough about LY’s export formats. I thought I was clear 
with the distinction I drew between my terminology of « application » and « 
program » from my first message. Sadly Ghostscript has deprecated its SVG 
capabilities.

I was sadly unavailable last year for GSoC, but I have submitted my proposal 
for this year, with the addition of improving SVG WOFF through SMuFL.

I seem to recall another student post here with initiative for that part, and 
would be glad to work with them, or focus on how to get his work on the SVG 
backend.

Cordially,
Étienne

> Le 4 déc. 2018 à 09:39, Paul Morris  a écrit :
> 
> On 12/3/18 10:07 AM, Richard Shann wrote:
> 
>> what seems strange is that there seems to be no concept of "backgound
>> color" in SVG (logical in a way as SVG is about things you can draw)
>> allowing for the possibility of drawing white on white.
> 
> 
> Easily setting a background color would be a nice option to have.  There's 
> this snippet, and it works for SVG (but there's room for improvement):
> 
> http://lsr.di.unimi.it/LSR/Item?id=699
> 
> 
>> yes, you add stuff to the LilyPond input to color everything explicitly
>> black. IIRC you can specify extra stuff to be included from the command
>> line too.
> 
> 
> Ah, right, like this: http://lsr.di.unimi.it/LSR/Item?id=443
> 
> That wraps everything in  tags with a color property, like so:
> 
> 
>   
> 
> 
> 
> Would probably be better if it did this instead:
> 
> 
> 
Precisely. I’ve already stated that: « […] In terms of colour, it currently 
wraps objects of a colour in a 

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

[thomasmorley65]

On 2020/03/30 18:29:37, Valentin Villenave wrote:
> On 2020/03/30 16:06:08, lemzwerg wrote:
> > Is it guaranteed that we can create this directory?  What about
srcdir !=
> > builddir?
> 
> Yes.  It’s a (tmpnam) name, located in /tmp or $TMPDIR or whatever:
>
https://www.gnu.org/software/guile/manual/html_node/File-System.html#index-tmpnam
> That’s the mechanism we already use for intermediate-files.

Well, see 
https://lists.gnu.org/archive/html/guile-devel/2020-03/msg00046.html
tmpnam is deprecated in guile-3.0.2
Why introducing it, we would need to move away from it later anyway?
Also, the link you provided speaks about security risks...


https://codereview.appspot.com/557640051/

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

[v . villenave]

Reviewers: lemzwerg,

Message:
On 2020/03/30 16:06:08, lemzwerg wrote:
> Is it guaranteed that we can create this directory?  What about srcdir
!=
> builddir?

Yes.  It’s a (tmpnam) name, located in /tmp or $TMPDIR or whatever:
https://www.gnu.org/software/guile/manual/html_node/File-System.html#index-tmpnam
That’s the mechanism we already use for intermediate-files.

The alternative would be to create the font files in the cwd, like what
markup-eps.ly does.  I decided against it because creating a file is
already intrusive, creating a subdir is somehow worse.

Now, ideally these should be removed after the PDF is generated.  The
problem is that (like intermediate-files, precisely) they need to be
deleted only after GhostScript’s done with them… and I’m not sure we’re
able to overwrite, for example, postprocess-output or output-framework
on-the-fly.  I don’t think the codebase needs additional hooks only for
one regtest to clean after itself, but it *could* be a problem for
systems that run the regtests continuously.

Thanks for the other nits, I’ve addressed them.

V.

Description:
Issue #1204: Document, and add regtest for, external fonts.

Add a few details in NR 1.8.3.1 "Fonts explained", and
a regtest with base64-embedded minimal OpenType fonts.
(I didn’t want to use musical symbols since the whole
point is that these glyphs are not to be confused with
whatever’s installed on the OS.)
The regtest has been tested with all major versions
since 2.12, and current master with and without Guile2.

Please review this at https://codereview.appspot.com/557640051/

Affected files (+256, -2 lines):
  M Documentation/notation/text.itely
  A input/regression/font-name-add-files.ly

Re: OT: maillist changes width, messes lines

[Jean-Charles Malahieude]

Le 30/03/2020 à 17:31, Francisco Vila a écrit :
At some point in the email path from sender to recipients, our list 
incorrectly word wraps some long lines. See attachment. It's in the 
actual email code, too.


I was pretty much ignoring it until I couldn't help reading

   "Anything and rather sake uses from"

which clearly tends to distort the message.


It happens mostly when it comes from Rietveld. What I do in such case is 
opening the discussion directly on codereview.appspot.


Cheers,
--
Jean-Charles

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

[lemzwerg--- via Discussions on LilyPond development]

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
Documentation/notation/text.itely:1552: (and thus available in LilyPond
scores), through the following commands:
no comma

https://codereview.appspot.com/557640051/diff/571940043/Documentation/notation/text.itely#newcode1558
Documentation/notation/text.itely:1558: 
I would remove this empty line in the @example environment.

https://codereview.appspot.com/557640051/diff/571940043/input/regression/font-name-add-files.ly
File input/regression/font-name-add-files.ly (right):

https://codereview.appspot.com/557640051/diff/571940043/input/regression/font-name-add-files.ly#newcode14
input/regression/font-name-add-files.ly:14: dummyfontfile = 
#(string-append (tmpnam) "-dummyfont.otf")
s/  / /

https://codereview.appspot.com/557640051/diff/571940043/input/regression/font-name-add-files.ly#newcode210
input/regression/font-name-add-files.ly:210: #(mkdir dummyfontdir)
Is it guaranteed that we can create this directory?  What about srcdir
!= builddir?

https://codereview.appspot.com/557640051/

Re: Anything missing for 2.21.0?

[Valentin Villenave]

On 3/29/20, Dan Eble  wrote:
>str = re.sub (r"\\(compress|expand)FullBarRests)", r"\\\1EmptyMeasures",

That’s on me; sorry about that.
https://codereview.appspot.com/553750044/diff/561590043/python/convertrules.py
I was convinced it had successfully gone through the docs and
regtests, which is why I didn’t look twice.  Sh*t.

Cheers,
V.

OT: maillist changes width, messes lines

[Francisco Vila]

At some point in the email path from sender to recipients, our list 
incorrectly word wraps some long lines. See attachment. It's in the 
actual email code, too.


I was pretty much ignoring it until I couldn't help reading

  "Anything and rather sake uses from"

which clearly tends to distort the message.
--
Francisco Vila, Ph.D. - Badajoz (Spain)
paconet.org , lilypond.es

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

[dak]

On 2020/03/29 14:35:43, hanwenn wrote:
> On Sun, Mar 29, 2020 at 4:12 PM  wrote:
> >
> > On 2020/03/29 13:55:41, hanwenn wrote:
> > > On 2020/03/29 13:52:48, hanwenn wrote:
> > > > retain existing
> > >
> > > how's this? This leaves things backward compatible.
> >
> > Ugly and a maintenance burden since the code is used twice. 
Anything
> > wrong with my proposal?
> 
> I didn't understand your proposal.
> 
> > It does not have duplicate code, makes
> > define-markup-command compilable (while requiring its toplevel use)
and
> > provides a way of doing the same consistently for module-specific
rather
> > than toplevel use.
> >
> > It sacrifices, like your proposal, non-toplevel-performance for the
sake
> > of compilability in Guilev2.  It's just that what the parser then
uses
> > is in a form that could also be used in a reasonably natural manner
from
> > Scheme.
> >
> > Should I write up a patch doing that?
> 
> Yes please.
> 

Working on it.  By the way, it just occured to me that a whole lot of
trouble with byte-compilation is caused by the markup macro.

git grep '(macro\($| \)' scm |wc -l

counts 65 occurences.  Those could certainly be replaced with
make-...-markup functions without causing too much trouble, leaving the
markup macro as only a user-level "feature".  That should significantly
cut down on our problems, right?  The markup macro framework appears to
be one of the worst corners for byte-compilation.

The performance loss due to run-time argument checking instead of
compile-time argument checking (which is likely the motivation for the
markup macro design) is likely insignificant compared to the cost of
actually typesetting markup.

https://codereview.appspot.com/577720043/