I'm sorry, but I find this incredibly unhelpful. "You've done it
wrong"
is insulting to someone who's spent about 40 hours trying to get it
right.
The original code was wrong, with any number of incorrect assumtions. If you could point me to what is incorrect, I'll try to fix it. I see nothing wrong with the images you've supplied.
Oh! Sorry about that. I thought that the differences between the old and new shape are obvious. If you do a blinking comparison it is easy to see that the lower left and right vertical extensions have become shorter and longer, respectively. Since you don't say anywhere that you are going the change the glyph shape itself, I've assumed (and still assume) that this is an unintentional change. On the other hand, the upper left and right vertical extensions now unnecessarily protrude the clef body, which can be dangerous. From mf/README: If outlines intersect, avoid grazing intersections. In case two outlines intersect in an explicitly defined point, include this point in both intersecting paths to avoid problems due to rounding errors.
The mix of tabs and spaces made indentation difficult to follow. Please explain why mixing tabs and spaces is good.
It's not about mixing tabs and spaces but about consistency, which I consider extremely important. If you do grep -1 -h 'def .* ([^)]*$' parmesan-custodes.mf you can see that the indentation pattern follows this scheme def foo (expr XXX, ..., YYY, ...) in case the `def' doesn't fit into a single line. The same holds for the other MF files (with three exceptions which are formatting mistakes). Another issue is that your indentation changes actually distract from your real code changes. Your patch has become unnecessarily much longer due to the indentation stuff. If you *really* want to change the indentation, it should be done in a separate patch which should be then tagged as `formatting only' or something like that. BTW, this is true in general: Always try to separate formatting and code changes.
I found the line break hindered understanding of the syntax, to no benefit. Most editors extend part 80 characters these days.
This is exactly the same issue as above. BTW, I don't share your opinion, and except some well-founded exceptions in parmesan-noteheads.mf, *all* source code lines of the MF files are within the 80-char limit. http://codereview.appspot.com/6503091/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel