date:20200131

Urs Liska  writes:

> Am Freitag, den 31.01.2020, 18:38 +0100 schrieb:
>> Urs Liska  writes:
>> 
>> > Am Mittwoch, den 29.01.2020, 07:01 -0800 schrieb d...@gnu.org:
>> > > 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
>> > > think it makes sense readjusting use-modules clauses all the time
>> > > while
>> > > we are deciding on the final module organisation, so I'd strongly
>> > > suggest first biting the bullet and deciding on a syntax for a
>> > > user-level command able to load Scheme modules without further
>> > > options,
>> > > and then introduce that command.  In that manner, future
>> > > directory
>> > > organisations (which are almost certain to come) will not affect
>> > > the
>> > > source of user-level documents any more.
>> > > 
>> > > https://codereview.appspot.com/567140045/
>> > 
>> > Maybe I'm missing something, but AFAICS there will always be the
>> > need
>> > for a module path like (ice-9 regex), or (scm display-lily). We
>> > will
>> > have that with *any* user-facing load syntax.
>> 
>> \loadScmPackage display-lily
>> 
>> or even
>> 
>> \loadPackage display-lily.scm
>> 
>> with the last .scm getting interpreted specially.
>> 
>> #(use-modules ...) is not a user-facing load syntax.  We don't want
>> people to have to change their input when an optional package
>> migrates
>> to the system or a local package changes hierarchy.  Specific path
>> components may make sense for disambiguation, but if I take a look at
>> what LaTeX does, a flat hierarchy as first user-level approximation
>> does
>> not appear to have significant scaling problems.
>
> I'm not so sure about that. In the LaTeX world this means that you
> have to come up with file names that are unique to the whole ecosystem

Yes.

> because otherwise you're relying on luck that "your" file is found
> first in case of conflicts.  I had this several times, mostly in the
> form of requests of TeX Live maintainers to rename files which were
> considered dangerously generic.

So?  If you put 5 logical layers before "Schenker", two different
Schenker graph packages will still collide because the logical
categories are still all a match.  You are not even kicking the can down
the road that way, you are just making it bigger.

> The other thing to consider is that path components are (currently)
> not only used for disambiguation but also for the lookup. LilyPond
> starts searching for files at each include path, and it relies on
> relative paths starting from a (any) include path. LaTeX, however,
> recursively searches for files starting from the include paths. I
> think going that route would have several negative impacts
> (disadvantage/risk):
>
>  * The recursive search is potentially expensive

Which is why path databases ultimately exist.

>  * There are actual risks of loops in the search path

Only for symbolic links.

>  * Targets are not really specified, using relative paths is
>  more expressive and explicit.  * kpathsea is pretty complex and has
>  corner cases that don't work well (e.g. it doesn't always follow
>  symlinks when a directory does not contain at least one "real" file),
>  and we would knowingly have to decide creating and maintaining a
>  comparable tool

Eventually.

> So, requiring explicit paths gives clearer interfaces (although more
> typing is required) and reduces searching complexity.

I don't see at all that it is a clearer interface if you have to specify
a detailed path with information that will either be the same for
packages with potential conflicting names due to sharing functionality,
or arbitrary.

Take a look actually more or less at _any_ package system, be it TeX,
Debian, PEP, RPM, Emacs.  The namespace for requesting a package is
flat.  On all of them.  There are search hierarchies containing them,
but they don't make for unique paths in any way.

> In the case of display-lily and all other built-in modules we can
> reduce this by adding the scm directory to the include path. But for
> external packages I am pretty sure that using explicit paths from the
> include paths (like we already have it with \include) makes sense.

I don't think it makes sense to have users exposed to the internal
placement of packages organized in directory trees, and I don't know any
package system that does.

The internal scm files loaded by LilyPond don't need to be visible to
the user.  Those optionally loaded manually (for example, (scm accreg))
are a different game.

> However, I'd like to stress that we are (or at least should be) talking
> about several different things when saying "package" and "loading":
>
> 1)
> Loading a package/module *file*, parsing its code and

Re: Doc: Correct and extend infos about LilyDev setup (issue 561360043 by michael.kaepp...@googlemail.com)

2020-01-31 Thread Michael Käppler

Am 30.01.2020 um 14:17 schrieb fedel...@gmail.com:

https://codereview.appspot.com/561360043/diff/565550050/Documentation/contributor/quick-start.itexi
File Documentation/contributor/quick-start.itexi (right):

https://codereview.appspot.com/561360043/diff/565550050/Documentation/contributor/quick-start.itexi#newcode208
Documentation/contributor/quick-start.itexi:208: sufficient to choose
number@tie{}71 (Generic, 105 keys). After that,
It seems it's not necessarily number 71. I would remove the number.
keyboards are sorted alphabetically so "Generic, 105 keys" is enough.

Weird that this does show up differently. Yes, I will change this.

Here's what I see:

65. Generic 101-key PC
66. Generic 102-key (Intl) PC
67. Generic 104-key PC
68. Generic 105-key (Intl) PC

I've tried 68 but didn't seem to work. I had to go to the GUI settings
for the layout, disable system setting and remove the english from the
list, then adding new layouts worked fine. (Probably a bug in Debian or
XFCE? Honestly, I don't have time nor will to investigate.)

I cannot reproduce this. I used the v2 release you prepared, loaded
it into VirtualBox 6.0.12, Host Windows 10.
After dpkg-reconfigure keyboard-configuration, using number 68 as
keyboard type and rebooting
everything works fine.
What did not work?

Anyway, to me the point is: doing trial and error in the GUI is way
better than having to follow the same procedure with dpkg-reconfigure.
So I'm not sure adding keyboard-configuration to LilyDev was a good
idea. I thought that it would have saved the user from guessing the
right layout but that's not the case, so there's no improvement over
previous situation AFAICS.

I proposed this because the method described in the CG did not work (and
does
not work yet, too). If I add the "German" keyboard layout in the XFCE
settings panel, I have to
remove the English layout in order to make it work, and even then it
does not work for the login screen
and AFAIK generally outside the X environment.
So using dpkg-reconfigure seems to be a cleaner approach to me.

https://codereview.appspot.com/561360043/

Re: Move Guile-style modules from scm to scm-modules (issue 567140045 by m...@ursliska.de)

2020-01-31 Thread Urs Liska

Am Freitag, den 31.01.2020, 18:38 +0100 schrieb David Kastrup:
> Urs Liska  writes:
> 
> > Am Mittwoch, den 29.01.2020, 07:01 -0800 schrieb d...@gnu.org:
> > > 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
> > > think it makes sense readjusting use-modules clauses all the time
> > > while
> > > we are deciding on the final module organisation, so I'd strongly
> > > suggest first biting the bullet and deciding on a syntax for a
> > > user-level command able to load Scheme modules without further
> > > options,
> > > and then introduce that command.  In that manner, future
> > > directory
> > > organisations (which are almost certain to come) will not affect
> > > the
> > > source of user-level documents any more.
> > > 
> > > https://codereview.appspot.com/567140045/
> > 
> > Maybe I'm missing something, but AFAICS there will always be the
> > need
> > for a module path like (ice-9 regex), or (scm display-lily). We
> > will
> > have that with *any* user-facing load syntax.
> 
> \loadScmPackage display-lily
> 
> or even
> 
> \loadPackage display-lily.scm
> 
> with the last .scm getting interpreted specially.
> 
> #(use-modules ...) is not a user-facing load syntax.  We don't want
> people to have to change their input when an optional package
> migrates
> to the system or a local package changes hierarchy.  Specific path
> components may make sense for disambiguation, but if I take a look at
> what LaTeX does, a flat hierarchy as first user-level approximation
> does
> not appear to have significant scaling problems.

I'm not so sure about that. In the LaTeX world this means that you have
to come up with file names that are unique to the whole ecosystem
because otherwise you're relying on luck that "your" file is found
first in case of conflicts.
I had this several times, mostly in the form of requests of TeX Live
maintainers to rename files which were considered dangerously generic.

The other thing to consider is that path components are (currently) not
only used for disambiguation but also for the lookup. LilyPond starts
searching for files at each include path, and it relies on relative
paths starting from a (any) include path. LaTeX, however, recursively
searches for files starting from the include paths. I think going that
route would have several negative impacts (disadvantage/risk):

 * The recursive search is potentially expensive
 * There are actual risks of loops in the search path
 * Targets are not really specified, using relative paths is
   more expressive and explicit.
 * kpathsea is pretty complex and has corner cases that don't work well
   (e.g. it doesn't always follow symlinks when a directory does not
   contain at least one "real" file), and we would knowingly have to
   decide creating and maintaining a comparable tool

So, requiring explicit paths gives clearer interfaces (although more
typing is required) and reduces searching complexity.

In the case of display-lily and all other built-in modules we can
reduce this by adding the scm directory to the include path. But for
external packages I am pretty sure that using explicit paths from the
include paths (like we already have it with \include) makes sense.

###

However, I'd like to stress that we are (or at least should be) talking
about several different things when saying "package" and "loading":

1)
Loading a package/module *file*, parsing its code and making elements
available to clients

1a)
One question is how to address these includable files
1b)
The other question is where the elements and which elements of the
loaded files are visible.

2)
Loading/using a package in a more conceptual sense, like "edition-
engraver" or "scholarLY". Here all the stuff about option handling
becomes (more) relevant, and the questions of 1) are prerequisites.

###

So when talking about new commands to "load packages" we are actually
talking about two different things that *both* have to be done.

Urs

Re: Issue 5719: Tie formatting maintenance (issue 581560049 by nine.fierce.ball...@gmail.com)

2020-01-31 Thread jonas . hahnfeld

LGTM

https://codereview.appspot.com/581560049/

Re: pushed wrong branch

2020-01-31 Thread Jonas Hahnfeld

Am Freitag, den 31.01.2020, 22:45 +0100 schrieb David Kastrup:
> Jonas Hahnfeld <
> hah...@hahnjo.de
> > writes:
> 
> > I accidentally pushed my branch for issue #5702 (Disable C++
> > exceptions) instead of issue #5690 (Tiny fixes for extractpdfmark).
> > I immediately reset staging and it now has the correct commits, fingers
> > crossed that patchy did not yet pick up the wrong refs...
> > 
> > Sorry for any inconvenience this may cause!
> 
> Patchy checks before pushing that the tested version of staging is still
> in the staging branch and fails if it isn't.  So you have a timing
> window for fixing things that is at least as large as the fastest Patchy
> doing duty can process all the tests.
> 
> If you managed to get the change out before it went to master, you
> should be good.

Ah, good to know. Yes, the commits have been in staging for at most 2
minutes, so all good.

Thanks,
Jonas


signature.asc
Description: This is a digitally signed message part

Re: pushed wrong branch

2020-01-31 Thread pkx166h


On 31/01/2020 21:45, David Kastrup wrote:

Jonas Hahnfeld  writes:


I accidentally pushed my branch for issue #5702 (Disable C++
exceptions) instead of issue #5690 (Tiny fixes for extractpdfmark).
I immediately reset staging and it now has the correct commits, fingers
crossed that patchy did not yet pick up the wrong refs...

Sorry for any inconvenience this may cause!

Patchy checks before pushing that the tested version of staging is still
in the staging branch and fails if it isn't.  So you have a timing
window for fixing things that is at least as large as the fastest Patchy
doing duty can process all the tests.

If you managed to get the change out before it went to master, you
should be good.

Process and scripts have seen some refinement over the years...


All pushed :D - although mine was beaten by someone else's patchy.

James

Re: pushed wrong branch

Jonas Hahnfeld  writes:

> I accidentally pushed my branch for issue #5702 (Disable C++
> exceptions) instead of issue #5690 (Tiny fixes for extractpdfmark).
> I immediately reset staging and it now has the correct commits, fingers
> crossed that patchy did not yet pick up the wrong refs...
>
> Sorry for any inconvenience this may cause!

Patchy checks before pushing that the tested version of staging is still
in the staging branch and fails if it isn't.  So you have a timing
window for fixing things that is at least as large as the fastest Patchy
doing duty can process all the tests.

If you managed to get the change out before it went to master, you
should be good.

Process and scripts have seen some refinement over the years...

-- 
David Kastrup

Re: pushed wrong branch

2020-01-31 Thread pkx166h


On 31/01/2020 21:02, Jonas Hahnfeld wrote:

I accidentally pushed my branch for issue #5702 (Disable C++
exceptions) instead of issue #5690 (Tiny fixes for extractpdfmark).
I immediately reset staging and it now has the correct commits, fingers
crossed that patchy did not yet pick up the wrong refs...

Sorry for any inconvenience this may cause!
Jonas


We'll soon find out.

Starting a patchy-staging run now.

;)


James

pushed wrong branch

2020-01-31 Thread Jonas Hahnfeld

I accidentally pushed my branch for issue #5702 (Disable C++
exceptions) instead of issue #5690 (Tiny fixes for extractpdfmark).
I immediately reset staging and it now has the correct commits, fingers
crossed that patchy did not yet pick up the wrong refs...

Sorry for any inconvenience this may cause!
Jonas


signature.asc
Description: This is a digitally signed message part

Re: allow overriding of GUILE_AUTO_COMPILE (issue 579270043 by hanw...@gmail.com)



https://codereview.appspot.com/579270043/diff/579270044/lily/main.cc
File lily/main.cc (right):

https://codereview.appspot.com/579270043/diff/579270044/lily/main.cc#newcode760
lily/main.cc:760: sane_putenv("GUILE_AUTO_COMPILE", "1", false);  //
disable auto-compile
On 2020/01/31 19:43:34, thomasmorley651 wrote:
> Shouldn't the comment be changed as well?

Done.

https://codereview.appspot.com/579270043/

Re: allow overriding of GUILE_AUTO_COMPILE (issue 579270043 by hanw...@gmail.com)

2020-01-31 Thread thomasmorley65



https://codereview.appspot.com/579270043/diff/579270044/lily/main.cc
File lily/main.cc (right):

https://codereview.appspot.com/579270043/diff/579270044/lily/main.cc#newcode760
lily/main.cc:760: sane_putenv("GUILE_AUTO_COMPILE", "1", false);  //
disable auto-compile
Shouldn't the comment be changed as well?

https://codereview.appspot.com/579270043/

Re: Use a pointer for the output parameter of Lily_lexer::scan_word (issue 577440044 by hanw...@gmail.com)

> Stop using non-const references in function signatures

Carrying the discussion over from [1], I would like to hear a clear
decision that this is the way LilyPond is going to be coded--something
more definite than one person proposing a change and another saying it
looks good.  If this is the way things are going to be, contributors and
reviewers would also benefit from guidance on when a function should
validate the pointers it receives, and if it doesn't, how that ought to
be documented to make up for not being allowed to pass by reference.

[1] https://codereview.appspot.com/577410045/


https://codereview.appspot.com/577440044/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-31 Thread Carl . D . Sorensen

On 2020/01/31 18:33:09, hanwenn wrote:
> On 2020/01/31 18:22:47, Dan Eble wrote:
> > On 2020/01/31 17:52:45, hanwenn wrote:
> > > you can do a local alias
> > > 
> > >   vector<>  = *vec;
> > > 
> > > to aid readability.
> > 
> > The more I think about banning non-const reference parameters, the
more I'm
> > against it.  Google's coding standards may work for them, but their
rationale*
> > for this one is weak.  How can we resolve this disagreement quickly?
 Do you
> > simply have the final say as the project founder?
> 
> Can we have this discussion on a thread separate from this code
review?
> I want this code to go in.

This code is a definite improvement in my book.  I like the names of the
functions, and it seems to me that eliminating the Pars_start class is 
a good idea.

Han-Wen has responded well to comments (even making changes that are not
his preferred way of doing things).

This patch LGTM.

I would like to see some separate discussion about the status of Input
and the use of non-constant reference pointers.  But we shouldn't hold
up this patch to have that discussion.

Carl

https://codereview.appspot.com/577410045/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-31 Thread Han-Wen Nienhuys

On Fri, Jan 31, 2020 at 7:43 PM Dan Eble  wrote:
> > Can we have this discussion on a thread separate from this code review?
> > I want this code to go in.
>
> Well, you can't prevent people from replying to their email, but neither can 
> their replies prevent you from pushing commits.

Well, I'm trying to follow the established process here which involves
waiting for the code review to be LGTM'd, after which other magical
steps happen.

Or do I get to bypass process?

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

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-31 Thread Dan Eble

On Jan 31, 2020, at 13:33, hanw...@gmail.com wrote:
> 
> On 2020/01/31 18:22:47, Dan Eble wrote:
>> On 2020/01/31 17:52:45, hanwenn wrote:
>>> you can do a local alias
>>> 
>>>  vector<>  = *vec;
>>> 
>>> to aid readability.
>> 
>> The more I think about banning non-const reference parameters, the
> more I'm
>> against it.  Google's coding standards may work for them, but their
> rationale*
>> for this one is weak.  How can we resolve this disagreement quickly? 
> Do you
>> simply have the final say as the project founder?
> 
> Can we have this discussion on a thread separate from this code review?
> I want this code to go in.

Well, you can't prevent people from replying to their email, but neither can 
their replies prevent you from pushing commits.
— 
Dan

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanw...@gmail.com)

2020-01-31 Thread Carl . D . Sorensen

On 2020/01/31 18:06:00, hanwenn wrote:

>  Will james pick this up automatically now, or does it need
> an LGTM?

James should pick it up automatically now.

https://codereview.appspot.com/561340043/

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanw...@gmail.com)

On 2020/01/31 18:33:38, Carl wrote:
> IIUC, this is a .clang-format that can be (but is not required to be)
used to
> format source code and prevent comments about formatting.
> 
> At this point, we are not enforcing a shift to clang-format as the
code standard
> for LilyPond.
> 
> If this is true, LGTM.
> 
> If we are enforcing a shift to clang-format, then I think we need an
LGTM from
> David K.

correct.

https://codereview.appspot.com/561340043/

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanw...@gmail.com)

2020-01-31 Thread Carl . D . Sorensen

IIUC, this is a .clang-format that can be (but is not required to be)
used to format source code and prevent comments about formatting.

At this point, we are not enforcing a shift to clang-format as the code
standard for LilyPond.

If this is true, LGTM.

If we are enforcing a shift to clang-format, then I think we need an
LGTM from David K.

https://codereview.appspot.com/561340043/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

On 2020/01/31 18:22:47, Dan Eble wrote:
> On 2020/01/31 17:52:45, hanwenn wrote:
> > you can do a local alias
> > 
> >   vector<>  = *vec;
> > 
> > to aid readability.
> 
> The more I think about banning non-const reference parameters, the
more I'm
> against it.  Google's coding standards may work for them, but their
rationale*
> for this one is weak.  How can we resolve this disagreement quickly? 
Do you
> simply have the final say as the project founder?

Can we have this discussion on a thread separate from this code review?
I want this code to go in.




https://codereview.appspot.com/577410045/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

On 2020/01/31 17:52:45, hanwenn wrote:
> you can do a local alias
> 
>   vector<>  = *vec;
> 
> to aid readability.

The more I think about banning non-const reference parameters, the more
I'm against it.  Google's coding standards may work for them, but their
rationale* for this one is weak.  How can we resolve this disagreement
quickly?  Do you simply have the final say as the project founder?

* "References can be confusing, as they have value syntax but pointer
semantics."

https://codereview.appspot.com/577410045/

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanw...@gmail.com)

On 2020/01/31 12:35:45, Dan Eble wrote:
> On 2020/01/31 09:39:33, hanwenn wrote:
> > I've applied your suggestion; PTAL.
> 
> If Werner likes it, I'm fine with it.  I haven't tried it myself
because I want
> to avoid being drawn into discussing the details of the style, but I
like seeing
> movement toward a better process.

that's great to hear. Will james pick this up automatically now, or does
it need an LGTM?

https://codereview.appspot.com/561340043/

Re: Introduce ly_scm2utf8_string, and use it for printing text (issue 577440043 by hanw...@gmail.com)

On 2020/01/31 18:01:32, hanwenn wrote:
> > The second underscore makes this stand out from the other functions
in this
> > group.
> 
> it does. Do you have a suggestion to improve?

ly_scm2utf8string?  ly_scm2utf8?


https://codereview.appspot.com/577440043/

Re: Use a pointer for the output parameter of Lily_lexer::scan_word (issue 577440044 by hanw...@gmail.com)

On 2020/01/31 12:22:22, Dan Eble wrote:
> On 2020/01/29 06:43:19, hanwenn wrote:
> > score performer
> 
> Does this relate to the lexer change, or does it belong on its own?

changed description.

https://codereview.appspot.com/577440044/

Re: Introduce ly_scm2utf8_string, and use it for printing text (issue 577440043 by hanw...@gmail.com)

On 2020/01/30 15:03:02, Dan Eble wrote:
>
https://codereview.appspot.com/577440043/diff/553450043/lily/include/lily-guile.hh
> File lily/include/lily-guile.hh (right):
> 
>
https://codereview.appspot.com/577440043/diff/553450043/lily/include/lily-guile.hh#newcode60
> lily/include/lily-guile.hh:60: std::string ly_scm2utf8_string (SCM);
> The second underscore makes this stand out from the other functions in
this
> group.

it does. Do you have a suggestion to improve?

https://codereview.appspot.com/577440043/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

On 2020/01/31 17:38:55, Dan Eble wrote:
> On 2020/01/30 23:22:46, hanwenn wrote:
> > In the lily/ directory
> > 
> >  git grep 'vector<[^>]\+> &' *c|grep -v const
> > 
> > returns 20 results, which is pretty small, given the number of
methods in the
> > code base. A cursory inspection suggests that Mike introduced most
of these,
> and
> > I would have probably suggested to use pointers there too. I would
also change
> > these signatures if would stumble across them while refactoring
something
> else.
> 
> Wouldn't this policy tend toward crimes against readability like the
following?
> 
> void twiddle_vector(vector *vec)
> {
>   if (!vec || vec->empty ())
> return;
> 
>   for (size_t i = 0; i < vec->size() - 1; ++i)
> {
>   if ((*vec)[i] < 10)
>  (*vec)[i] = (*vec)[i] + 2 * (*vec)[i + 1];
>   else
> (*vec)[i] = (*vec)[i] * 2 - (*vec)[i + 1];
> }
> }
> 
> How would you approach that?

you can do a local alias

  vector<>  = *vec;

to aid readability.



https://codereview.appspot.com/577410045/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

On 2020/01/30 23:22:46, hanwenn wrote:
> In the lily/ directory
> 
>  git grep 'vector<[^>]\+> &' *c|grep -v const
> 
> returns 20 results, which is pretty small, given the number of methods
in the
> code base. A cursory inspection suggests that Mike introduced most of
these, and
> I would have probably suggested to use pointers there too. I would
also change
> these signatures if would stumble across them while refactoring
something else.

Wouldn't this policy tend toward crimes against readability like the
following?

void twiddle_vector(vector *vec)
{
  if (!vec || vec->empty ())
return;

  for (size_t i = 0; i < vec->size() - 1; ++i)
{
  if ((*vec)[i] < 10)
 (*vec)[i] = (*vec)[i] + 2 * (*vec)[i + 1];
  else
(*vec)[i] = (*vec)[i] * 2 - (*vec)[i + 1];
}
}

How would you approach that?

https://codereview.appspot.com/577410045/

Re: Move Guile-style modules from scm to scm-modules (issue 567140045 by m...@ursliska.de)

Urs Liska  writes:

> Am Mittwoch, den 29.01.2020, 07:01 -0800 schrieb d...@gnu.org:
>> 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
>> think it makes sense readjusting use-modules clauses all the time
>> while
>> we are deciding on the final module organisation, so I'd strongly
>> suggest first biting the bullet and deciding on a syntax for a
>> user-level command able to load Scheme modules without further
>> options,
>> and then introduce that command.  In that manner, future directory
>> organisations (which are almost certain to come) will not affect the
>> source of user-level documents any more.
>> 
>> https://codereview.appspot.com/567140045/
>
> Maybe I'm missing something, but AFAICS there will always be the need
> for a module path like (ice-9 regex), or (scm display-lily). We will
> have that with *any* user-facing load syntax.

\loadScmPackage display-lily

or even

\loadPackage display-lily.scm

with the last .scm getting interpreted specially.

#(use-modules ...) is not a user-facing load syntax.  We don't want
people to have to change their input when an optional package migrates
to the system or a local package changes hierarchy.  Specific path
components may make sense for disambiguation, but if I take a look at
what LaTeX does, a flat hierarchy as first user-level approximation does
not appear to have significant scaling problems.

>
> My thought was to separate the two different types of .scm files in
> that directory, and that could of course also be achieved by moving the
> *other* files, those that are loaded with ly:load from lily.scm to a
> different directory.
>
> Or - of course - I can simply drop this and add new modules to that
> same directory for now.
>
>> 
>
>
>

-- 
David Kastrup

Re: Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries (issue 577450043 by thomasmorle...@gmail.com)

Dan Eble  writes:

> On Jan 31, 2020, at 08:31, David Kastrup  wrote:
>>   -fexcess-precision=style
>>   This option allows further control over excess precision on
>>   machines where floating-point operations occur in a format
>>   with more precision or range than the IEEE standard and
>>   interchange floating-point types.  By default,
>>   -fexcess-precision=fast is in effect; this means that
>>   operations may be carried out in a wider precision than the
>>   types specified in the source if that would result in faster
>>   code, and it is unpredictable when rounding to the types
>>   specified in the source code takes place.
>
> This sounds promising.
>
>>   -fexcess-precision=standard is not implemented for languages
>>   other than C.
>
> Never mind.

Hm.  Maybe

-mfpmath=sse

instead?  The problem with switching the whole FPU to reduced precision
is that some library functions might not expect that, so it is making me
queasy.  On the other hand, using SSE might have a negative performance?
I just don't have a good idea what we are dealing with here.

-- 
David Kastrup

Re: Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries (issue 577450043 by thomasmorle...@gmail.com)

Masamichi Hosoda  writes:

>>>   -fexcess-precision=style
>>>   This option allows further control over excess precision on
>>>   machines where floating-point operations occur in a format
>>>   with more precision or range than the IEEE standard and
>>>   interchange floating-point types.  By default,
>>>   -fexcess-precision=fast is in effect; this means that
>>>   operations may be carried out in a wider precision than the
>>>   types specified in the source if that would result in faster
>>>   code, and it is unpredictable when rounding to the types
>>>   specified in the source code takes place.
>> 
>> This sounds promising.
>
> If I understand correctly, such compiler options are not promising.
>
> Even if LilyPond C++ source codes are compiled with such option,
> libguile without the option may calculate in a wider precision.
> In this case, the floating point calculation
> inside GUILE causes these issues.

The problem as far as I can see are non-reproducible results, and the
Guile code paths do not change from one call to the next.  So unless
just-in-time compilation comes into play, I don't think that Guile
behavior should be a problem concerning our non-reproducibility problem.

-- 
David Kastrup

Re: Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries (issue 577450043 by thomasmorle...@gmail.com)

2020-01-31 Thread Masamichi Hosoda

>>   -fexcess-precision=style
>>   This option allows further control over excess precision on
>>   machines where floating-point operations occur in a format
>>   with more precision or range than the IEEE standard and
>>   interchange floating-point types.  By default,
>>   -fexcess-precision=fast is in effect; this means that
>>   operations may be carried out in a wider precision than the
>>   types specified in the source if that would result in faster
>>   code, and it is unpredictable when rounding to the types
>>   specified in the source code takes place.
> 
> This sounds promising.

If I understand correctly, such compiler options are not promising.

Even if LilyPond C++ source codes are compiled with such option,
libguile without the option may calculate in a wider precision.
In this case, the floating point calculation
inside GUILE causes these issues.

Re: Documentation suggestions.

2020-01-31 Thread Peter Toye

I've now consolidated the various replies to my original suggestions - sorry 
it's been so long. Unfortunately I spent far too much time fighting VirtualBox 
and Linux without as much success as I'd like. So here are my - fairly small- 
documentation ideas. Sorry I couldn't make formal patches but I'll be far too 
busy with the real world for the next month.

Peter



LM 1.2.1 Simple notation. Add a paragraph after the 1st music example:

Note-names in all examples use the English or Dutch naming system 
(white piano keys are C-B).

LM 2.1.2 Pitches and key signatures. Subsection Pitch alterations. 3rd paragraph
(1)after 'alterations' add 'and note-names'.
(2) append:

The default language for note-names and alterations is 
nederlands (Dutch).

A question: is "alterations" a good word throughout this subsection? The normal 
English one is "accidentals", which is used in the Music Glossary reference.

NR 3.1.5 File Structure. Subsection Using variables. Add  a "Known Issues"  
subsection at end:


In addition to the normal convention for variable names [add reference 
to LM 2.4.1] variable names can include non-ASCII characters and non-adjacent 
single underscores and dashes. Any combination of characters is allowed if the 
variable name is enclosed in double quotation marks. In this case backslashes 
and double quotation marks need to be escaped with backslashes.
---
NR 1.2.5  Bars. Sub section Bar and bar number checks. Add a "known issues" 
section at end:

If MIDI output is selected and volta repeats are in place, the bar 
number check may fail. It is best to suppress MIDI output whle checking bar 
numbers.
--
NR 3.3.2 Different editions from one source. Subsection Using tags. Add before 
paragraph 3 ("The arguments..."):

\tag, \keepWithTag and \removeWithTag are music functions which take a 
music expression as their second argument. Thus they cannot be used to filter 
items such as  \book or \score blocks.
--
NR 3.2.1 Creating titles headers and footers. Subsection Default layout of 
headers and footers. Rename to:

Default layout of page headers and footers

and index it as "page headers", "page footers", "headers, page", "footers, 
page".
Possibly also promote it to a 3rd-level section? It doesn't have anything in 
common with the previous two subsections.

Re: Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries (issue 577450043 by thomasmorle...@gmail.com)

2020-01-31 Thread Dan Eble

On Jan 31, 2020, at 08:31, David Kastrup  wrote:
>   -fexcess-precision=style
>   This option allows further control over excess precision on
>   machines where floating-point operations occur in a format
>   with more precision or range than the IEEE standard and
>   interchange floating-point types.  By default,
>   -fexcess-precision=fast is in effect; this means that
>   operations may be carried out in a wider precision than the
>   types specified in the source if that would result in faster
>   code, and it is unpredictable when rounding to the types
>   specified in the source code takes place.

This sounds promising.

>   -fexcess-precision=standard is not implemented for languages
>   other than C.

Never mind.
— 
Dan

Re: Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries (issue 577450043 by thomasmorle...@gmail.com)

ArnoldTheresius  writes:

> trueroad wrote
>> Hello Arnold.
>> Thank you for your patch.
>> 
>> If I understand correctly, we only need check the definitions of
>> `__x86__` and `__i386__` check.
>> 
>> In x86_64 environment, neither `__x86__` nor `__i386__` are defined.
>> 
>> ```
>> $ echo | x86_64-w64-mingw32-gcc -dM -E - | grep "__x86__"
>> 
>> $ echo | x86_64-w64-mingw32-gcc -dM -E - | grep "__i386__"
>> 
>> $ echo | i686-w64-mingw32-gcc -dM -E - | grep "__x86__"
>> 
>> $ echo | i686-w64-mingw32-gcc -dM -E - | grep "__i386__"
>> #define __i386__ 1
>> 
>> $
>> ```
>> 
>> Therefore, `defined (__code_model_32__)`  is not necessary.
>> 
>> If `__SSE2_MATH__` is defined, it only indicates that main.cc is
>> compiled with SSE2 math enabled.
>> Shared libraries such as libguile may still use x86 FPU.
>> If the floating point calculation inside GUILE uses x86 FPU, we need to
>> set the precision even if C++ uses SSE2.
>> 
>> Therefore, `defined (__SSE2_MATH__)`  is not necessary.
>> 
>> Furthermore, if the floating-point operations of all modules use SSE2,
>> setting the x86 FPU precision has no effect because it is not used.
>> In other words, there is no problem even if the precision is set on a
>> platform that does not need to set.
>> 
>> Therefore, environment definitions such as `defined (__MINGW32__)`  is
>> not necessary.
>> 
>> 
>> https://codereview.appspot.com/577450043/
>
> Thank you for the clear answer.
>
> Therfore, do it as suggested [move asm() call closer to _FPU_SETCW() call]
>
> ArnoldTheresius.
>
>
>
>
> --
> Sent from: http://lilypond.1069038.n5.nabble.com/Dev-f88644.html

None of the following GCC options would help?

   The following options control compiler behavior regarding
   floating-point arithmetic.  These options trade off between speed
   and correctness.  All must be specifically enabled.

   -ffloat-store
   Do not store floating-point variables in registers, and
   inhibit other options that might change whether a floating-
   point value is taken from a register or memory.

   This option prevents undesirable excess precision on machines
   such as the 68000 where the floating registers (of the 68881)
   keep more precision than a "double" is supposed to have.
   Similarly for the x86 architecture.  For most programs, the
   excess precision does only good, but a few programs rely on
   the precise definition of IEEE floating point.  Use
   -ffloat-store for such programs, after modifying them to store
   all pertinent intermediate computations into variables.

   -fexcess-precision=style
   This option allows further control over excess precision on
   machines where floating-point operations occur in a format
   with more precision or range than the IEEE standard and
   interchange floating-point types.  By default,
   -fexcess-precision=fast is in effect; this means that
   operations may be carried out in a wider precision than the
   types specified in the source if that would result in faster
   code, and it is unpredictable when rounding to the types
   specified in the source code takes place.  When compiling C,
   if -fexcess-precision=standard is specified then excess
   precision follows the rules specified in ISO C99; in
   particular, both casts and assignments cause values to be
   rounded to their semantic types (whereas -ffloat-store only
   affects assignments).  This option is enabled by default for C
   if a strict conformance option such as -std=c99 is used.
   -ffast-math enables -fexcess-precision=fast by default
   regardless of whether a strict conformance option is used.

   -fexcess-precision=standard is not implemented for languages
   other than C.  On the x86, it has no effect if -mfpmath=sse or
   -mfpmath=sse+387 is specified; in the former case, IEEE
   semantics apply without excess precision, and in the latter,
   rounding is unpredictable.

   -ffast-math
   Sets the options -fno-math-errno, -funsafe-math-optimizations,
   -ffinite-math-only, -fno-rounding-math, -fno-signaling-nans,
   -fcx-limited-range and -fexcess-precision=fast.

   This option causes the preprocessor macro "__FAST_MATH__" to
   be defined.

   This option is not turned on by any -O option besides -Ofast
   since it can result in incorrect output for programs that
   depend on an exact implementation of IEEE or ISO
   rules/specifications for math functions. It may, however,
   yield faster code for programs that do not require the
   guarantees of these specifications.

-- 
David Kastrup

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanw...@gmail.com)

On 2020/01/31 09:39:33, hanwenn wrote:
> I've applied your suggestion; PTAL.

If Werner likes it, I'm fine with it.  I haven't tried it myself because
I want to avoid being drawn into discussing the details of the style,
but I like seeing movement toward a better process.


https://codereview.appspot.com/561340043/

Re: Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries (issue 577450043 by thomasmorle...@gmail.com)

2020-01-31 Thread ArnoldTheresius

trueroad wrote
> Hello Arnold.
> Thank you for your patch.
> 
> If I understand correctly, we only need check the definitions of
> `__x86__` and `__i386__` check.
> 
> In x86_64 environment, neither `__x86__` nor `__i386__` are defined.
> 
> ```
> $ echo | x86_64-w64-mingw32-gcc -dM -E - | grep "__x86__"
> 
> $ echo | x86_64-w64-mingw32-gcc -dM -E - | grep "__i386__"
> 
> $ echo | i686-w64-mingw32-gcc -dM -E - | grep "__x86__"
> 
> $ echo | i686-w64-mingw32-gcc -dM -E - | grep "__i386__"
> #define __i386__ 1
> 
> $
> ```
> 
> Therefore, `defined (__code_model_32__)`  is not necessary.
> 
> If `__SSE2_MATH__` is defined, it only indicates that main.cc is
> compiled with SSE2 math enabled.
> Shared libraries such as libguile may still use x86 FPU.
> If the floating point calculation inside GUILE uses x86 FPU, we need to
> set the precision even if C++ uses SSE2.
> 
> Therefore, `defined (__SSE2_MATH__)`  is not necessary.
> 
> Furthermore, if the floating-point operations of all modules use SSE2,
> setting the x86 FPU precision has no effect because it is not used.
> In other words, there is no problem even if the precision is set on a
> platform that does not need to set.
> 
> Therefore, environment definitions such as `defined (__MINGW32__)`  is
> not necessary.
> 
> 
> https://codereview.appspot.com/577450043/

Thank you for the clear answer.

Therfore, do it as suggested [move asm() call closer to _FPU_SETCW() call]

ArnoldTheresius.




--
Sent from: http://lilypond.1069038.n5.nabble.com/Dev-f88644.html

Re: Use a pointer for the output parameter of Lily_lexer::scan_word (issue 577440044 by hanw...@gmail.com)

On 2020/01/29 06:43:19, hanwenn wrote:
> score performer

Does this relate to the lexer change, or does it belong on its own?


https://codereview.appspot.com/577440044/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-31 Thread Benkő Pál

 ezt írta (időpont: 2020. jan. 31., P, 11:55):
>
>
> https://codereview.appspot.com/577410045/diff/581560047/lily/parse-scm.cc
> File lily/parse-scm.cc (right):
>
> https://codereview.appspot.com/577410045/diff/581560047/lily/parse-scm.cc#newcode77
> lily/parse-scm.cc:77: const Input *hi = >start_;
> On 2020/01/31 10:49:13, benko.pal wrote:
> > I understand (and like) adding the const, but can't understand
> changing the
> > reference to pointer even after reading the whole discussion.
>
> the change from & to * here is merely syntactical.

of course it is; but to me it's nearer to obfuscation than to cleanup.

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

https://codereview.appspot.com/577410045/diff/581560047/lily/parse-scm.cc
File lily/parse-scm.cc (right):

https://codereview.appspot.com/577410045/diff/581560047/lily/parse-scm.cc#newcode77
lily/parse-scm.cc:77: const Input *hi = >start_;
On 2020/01/31 10:49:13, benko.pal wrote:
> I understand (and like) adding the const, but can't understand
changing the
> reference to pointer even after reading the whole discussion.

the change from & to * here is merely syntactical. 

For me, the discussion around & vs * is about using non-const & in
function signatures and as class members.

However, what I want most at this point is to submit this change, so I
can get on with the next change in the series.

https://codereview.appspot.com/577410045/

Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by hanw...@gmail.com)

2020-01-31 Thread benko . pal



https://codereview.appspot.com/577410045/diff/581560047/lily/parse-scm.cc
File lily/parse-scm.cc (right):

https://codereview.appspot.com/577410045/diff/581560047/lily/parse-scm.cc#newcode77
lily/parse-scm.cc:77: const Input *hi = >start_;
I understand (and like) adding the const, but can't understand changing
the reference to pointer even after reading the whole discussion.

https://codereview.appspot.com/577410045/

Re: Inline assembler fallback for _FPU_SETCW() missing in MINGW libraries (issue 577450043 by thomasmorle...@gmail.com)

2020-01-31 Thread trueroad

Hello Arnold.
Thank you for your patch.

If I understand correctly, we only need check the definitions of
`__x86__` and `__i386__` check.

In x86_64 environment, neither `__x86__` nor `__i386__` are defined.

```
$ echo | x86_64-w64-mingw32-gcc -dM -E - | grep "__x86__"

$ echo | x86_64-w64-mingw32-gcc -dM -E - | grep "__i386__"

$ echo | i686-w64-mingw32-gcc -dM -E - | grep "__x86__"

$ echo | i686-w64-mingw32-gcc -dM -E - | grep "__i386__"
#define __i386__ 1

$
```

Therefore, `defined (__code_model_32__)`  is not necessary.

If `__SSE2_MATH__` is defined, it only indicates that main.cc is
compiled with SSE2 math enabled.
Shared libraries such as libguile may still use x86 FPU.
If the floating point calculation inside GUILE uses x86 FPU, we need to
set the precision even if C++ uses SSE2.

Therefore, `defined (__SSE2_MATH__)`  is not necessary.

Furthermore, if the floating-point operations of all modules use SSE2,
setting the x86 FPU precision has no effect because it is not used.
In other words, there is no problem even if the precision is set on a
platform that does not need to set.

Therefore, environment definitions such as `defined (__MINGW32__)`  is
not necessary.


https://codereview.appspot.com/577450043/

Re: Add a tentative .clang-format for LilyPond. (issue 561340043 by hanw...@gmail.com)