Re: New alist to replace special characters. (issue4553056)

[nicolas . sceaux]

A replacement function for text is a very good idea, and would be very
useful.

General comments:
A regtest for lyrics might be missing.

The current implementation of replacement function, in the C++ part,
need some rework.

An alternate design might be:
Define in the guile part the replacement function. The C++ code would
just call that function on the string. By default, the replacement
function could do exactly the job that is done in current LilyPond
version (replacing line feeds, etc, by spaces).
Another replacement function would be proposed, doing the work you
propose here, e.g. compiling and applying a regexp, instead of manually
implementing the replacement (in a possibly less efficient way than the
built-in would do).
And a \useTheFancyTextReplacementThing switch could select the later
replacement function.
(just brainstorming)

If the second replacement function has not much impact on performances,
it may even be the default.


http://codereview.appspot.com/4553056/diff/9003/input/regression/markup-special-characters-shorthands.ly
File input/regression/markup-special-characters-shorthands.ly (right):

http://codereview.appspot.com/4553056/diff/9003/input/regression/markup-special-characters-shorthands.ly#newcode15
input/regression/markup-special-characters-shorthands.ly:15:
(additional-replacements text-font-defaults
The interface for adding replacements/shortcuts seems a bit complicated.
Maybe a function call modifying text-font-defaults behind the scene
would be better?

#(add-text-replacements! '(...))

http://codereview.appspot.com/4553056/diff/9003/lily/text-interface.cc
File lily/text-interface.cc (right):

http://codereview.appspot.com/4553056/diff/9003/lily/text-interface.cc#newcode46
lily/text-interface.cc:46: for (int j = 0; j < scm_to_int (scm_length
(replacement_alist)); j++)
This is not the way to loop over a list: the idiom can be found many
times in the C++ code. scm_length and scm_list_ref are O(n).

What's more, at this point the `str' argument is not yet used. So maybe
this could be cached to avoid being computed again each time.

http://codereview.appspot.com/4553056/diff/9003/lily/text-interface.cc#newcode62
lily/text-interface.cc:62: str->replace (i, j, ligature);
We see three embedded loops: while, for, and a call to ly_assoc_get...
The previous add only one loop.
What is the impact on performance, wrt to current version?

Have you considered compiling (and caching) a regexp and use the
appropriate builtin function which will do all the replacement work?

Or maybe... get this remplacement part out of C++ code, and define in
the scheme part a replacement function, that will be called here. A
default replacement function might do the \n, \t \v replacement alone
(as on current version). An other replacement function would be proposed
which does more complicated things.

http://codereview.appspot.com/4553056/