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/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
