Re: Use a stable sort when ordering MIDI items (issue 353790043 by address@hidden)

[MrBobo1239]

On 2018/11/01 01:50:38, Dan Eble wrote:
> > (I'm surprised this hasn't been fixed until now.)

> Welcome to LilyPond, where you supply the fixes.  Your change looks
good to me,
> but judging from your questions, it sounds like it could be tested
better.

> Have you read the chapter on Regression Tests in the Contributor's
Guide?  You
> should follow the procedure and check whether your change has any
impact on the
> MIDI regression tests.  If there are changes, you should sift through
them to
> make sure the changes are intended.  If there are no changes, it
probably means
> that the test coverage is poor (in this case) and you should add your
tiny
> example as a new regression test.

I didn't know that the regression tests also cover the MIDI output.
Thanks for
the pointer. I had to grab LilyDev as the regression tests fail locally
with
"Undefined subroutine &main::get_index called at
$LILY_DIR/Documentation/
lilypond-texi2html.init line 2408." and indeed, a change is caught for
input/regression/midi/staff-map-voice.midi:

@@ -5,8 +5,8 @@
   ev (0, (255, 81, '\x0fB@'))
   ev (3072, (255, 47, ''))
 track 1  ev (0, (255, 33, '\x01'))
+  ev (0, (255, 3, 'treble:one'))
   ev (0, (255, 89, '\xfd\x01'))
-  ev (0, (255, 3, 'treble:one'))
   ev (0, (144, 77, 90))
   ev (0, (145, 72, 90))
   ev (192, (144, 77, 0))
@@ -29,10 +29,10 @@
   ev (960, (145, 77, 90))
   ev (1152, (144, 80, 0))
   ev (1152, (145, 77, 0))
+  ev (1152, (144, 79, 90))
   ev (1152, (145, 75, 90))
-  ev (1152, (144, 79, 90))
+  ev (1344, (144, 79, 0))
   ev (1344, (145, 75, 0))
-  ev (1344, (144, 79, 0))
   ev (1344, (144, 77, 90))
   ev (1344, (145, 74, 90))
   ev (1536, (144, 77, 0))

Don't know about the first change but the other two do make the output
more
consistent.

(I've also hit a regression in input/regression/rest-dot-position.ly but
AFAIKT
that is https://sourceforge.net/p/testlilyissues/issues/5217/.)

https://codereview.appspot.com/353790043/