Re: Fixes position of mensural c clef (issue 6503091)

[lemzwerg]

> 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/