Re: PATCH: Refactor script-column.cc for improved reading and fewer lines

[Carl Sorensen]

On 12/5/09 3:15 AM, "David Kastrup" <address@hidden> wrote:

> "Trevor Daniels" <address@hidden> writes:
> 
>> Carl Sorensen wrote Friday, December 04, 2009 6:53 PM
>> 
>>> On 12/4/09 9:24 AM, "David Kastrup" <address@hidden> wrote:
>>> 
>>> 
>>>> Could you describe in simple words what the behavior is supposed to
>>>> achieve?  If you do that, I promise to submit some simple code that
>>>> does
>>>> that.
>>>> 
>>> 
>>> Take three text_scripts, all with outside_staff_priority of 450, and
>>> with
>>> script_priorities of 201, 202, 203.
>>> 
>>> Convert them to three text_scripts with outside_staff_priorities of
>>> 450,
>>> 451, 452, so that the script_priority is moved to outside_staff
>>> priority.
>> 
>> What would happen if there was a TextScript at the same
>> moment with an 'outside-staff-priority of 451, set by
>> the user?
> 
> Before picking the particular algorithm: I have had my trouble following
> the previous discussion.  To summarize what I glean from the code:
> 
> script_priority: input property for the function.  The output lists
> shall have non-descending(?) values of script_priority.  This is
> achieved by a stable sort at the start.
> 
> outside_staff_priority: optional input property, required output
> property.  Has no influence on the order of output.  Resulting
> outside_staff_priority shall be non-descending.  Previously specified
> values of outside_staff_priority are taken into account.
> 
> Note that if outside_staff_priority has no influence on the sort order,
> and we want to get a non-descending sequence of outside_staff_priority,
> this implies that most preexisting values of outside_staff_priority have
> to be trashed.
> 
> Unless I don't understand something.  Well, I certainly don't, but that
> does not mean that it is relevant.

script-priority is used to order TextScripts (and Scripts) that occur at the
same moment relative to one another.

Other grobs besides TextScripts can exist outside the staff.
outside-staff-priority is used to establish the relative locations of the
different outside-staff grobs.  (See section 4.4.3 of the Learning Manual)

When it is finally time to set a note-column, all of the outside-staff
objects are sorted by outside-staff-priority.

So this particular function needs to create a set of
outside-staff-priorities that are consistent with their script-priorities.

The preexisting values of outside-staff-priority only need to be adjusted in
the case that multiple objects have the same outside-staff-priority.

I have appreciated your input and your comments.  I believe that the code is
more readable now than it was before, and there are fewer lines.  The
algorithm is clearer, and better commented.

But as far as I can see, we have reached the point of diminishing returns.
The code works, and it's not worth spending more of my time on it.  I'll
wait a couple of days for any specific comments on the code or for any
alternative proposals.  Then I'll push a patch.

I'm not going to spend more time explaining the algorithm.  It's not my
algorithm; I had to figure it out myself in order to try to solve the bug.
The bug was the fact that there was (previously) no check to see if the
various scripts had the same outside-staff-priority.  So, previously, the
first TextScript ended up with an outside-staff-priority of 450, and all the
rest had an outside-staff-priority of 450.1, and so the sort-order was not
unique.

With the new patch, the first TextScript ends up with an
outside-staff-priority of 450, the second, 450.1; the third, 450.2; and so
on.

It seems to me that further work on this involves primarily some contest
about who writes the best code.  I will happily allow anybody else who wants
to write better code to take over this issue.  But I'm not going to spend
more time explaining how things work.   It's not a good use of my time.

Thanks,

Carl