#3687 only full-measure-rests should be able to compress

[Anonymous]

Originally created by: *anonymous

Originally created by: janek.li...@gmail.com
Originally owned by: tdanielsmusic

\version "2.16.0"

\markup {
  Ordinary rests shouldn't compress, because the results are
  confusing this looks like one full-measure rest, while it's not:
}
{
  \compressFullBarRests
  \time 2/4
  b2 [r1] b2
}

\layout {
  \override Score.BarNumber #'break-visibility = #'#(#f #t #t)
}

[Google Importer]

Originally posted by: PhilEHol...@googlemail.com

Interesting.  It's not just rests.

{
  \compressFullBarRests
  \time 2/4
  b2 b\breve b2
}

\layout {
  \override Score.BarNumber #'break-visibility = #'#(#f #t #t)
}

Take out the compressFullBarRests and we get a number of empty bars.

[Google Importer]

Originally posted by: tdanielsmusic

It's relatively easy to write a music function which uses skipBars like \compressFullBarRests but which filters out the MM rests first, like this:

compressMMRests =
#(define-music-function (parser location music) (ly:music?)
   (music-map
    (lambda (m)
      (if (or
           (member 'rhythmic-event (ly:music-property m 'types))
           (member 'multi-measure-rest (ly:music-property m 'types)))
        (if (eq? 'MultiMeasureRestMusic (ly:music-property m 'name))
          #{ \set Score.skipBars = ##t #m #}
          #{ \set Score.skipBars = ##f #m #} )
        m))
    music))

I've used a different name, because \compressFullBarRests is likely to be embedded in many scores and it would be difficult to impossible to write a convert-ly entry to replace it with a music function.  One way forward would be to install the proposed compressMMRests music function, leaving \compressFullBarRests and skipBars unchanged, then modify the manuals to use \compressMMRests {...} instead of \compressFullBarRests.

What do you think?  Should I submit a patch to do that?

Trevor

[Google Importer]

Originally posted by: tdanielsmusic

Well, I've not been overwhelmed with responses, but at least no one has said this is a stupid idea.  Let me flesh it out a little more.  Here's what I propose:

1. Add a new \compressMMRests music function as sketched out above, whose action is limited to full-measure rests only.

2. Amend the docs to use the new function instead of \compressFullBarRests and \expandFullBarRests.

3. Rename \compressFullBarsRests and \expandFullBarsRests to \compressEmptyMeasures and \expandEmptyMeasures, which more accurately express what setting skipBars actually does, together with a convert-ly entry to cover this change.

4. Perhaps modify the use of skipBars in Ancient music to use \compressEmptyMeasures and \expandEmptyMeasures instead.

Trevor

[Google Importer]

Originally posted by: tdanielsmusic

Issue 3687 (part 4): Update regression tests to use \compressMMRests

Issue 3687 (part 3) Update snippets to use \compressMMRests

Issue 3687 (part 2): Amend docs to use \compressMMRests

  Amend the examples in the LM and NR to use the \compressMMRests
  music function rather than the predefs \compressFullBarRests and
  \expandFullBarRests to avoid advocating the use of techniques
  which may have undesirable side effects.

Issue 3687 (part 1): Provide a compress MM function for MM rests only

  Multi-measure rests were previously compressed (i.e. all bars
  following the first were removed) by using the \compressFullBarRests
  predef.  This simply turned on 'skipBars, which causes bar lines at
  the ends of bars which contain either no rhythmic event or contain
  a rhythmic event with a duration extending beyond the bar to be removed.
  Such bars are generated by notes with durations longer than a bar,
  like a1*3, or by shorter notes which extend beyond the bar.

  This patch provides a function which also uses 'skipBars, but with
  a filter so that 'skipBars is activated only when an actual MM rest
  is encountered and deactivated by the first following rhythmic event.

Issue 3687 (part 0): run convert-ly

  Run convert-ly on the doc files to be changed to bring them
  up to date.

http://codereview.appspot.com/232130043

Labels: Patch-new
Owner: tdanielsmusic
Status: Started

[Google Importer]

Originally posted by: tdanielsmusic

The patch just uploaded (#4) covers the first two points of #3 only.  I decided that points 3 and 4 were better left to a separate patch to be applied later, perhaps even under a new tracker.

Trevor

[Google Importer]

Originally posted by: pkx1...@gmail.com

Patchy the autobot says: passes tests.  Includes a full make doc

Labels: -Patch-new Patch-review

[Google Importer]

Originally posted by: tdanielsmusic

Also fixes Issue 4350, or rather removes the underlying problem that 4350 was trying to mitigate.

[Google Importer]

Originally posted by: pkx1...@gmail.com

Patch on countdown for May 6th

Labels: -Patch-review Patch-countdown

[Google Importer]

Originally posted by: pkx1...@gmail.com

Patch counted down - please push

Labels: -Patch-countdown Patch-push

[Google Importer]

Originally posted by: tdanielsmusic

Pushed to staging as five commits:

Part 0: run convert-ly
[rca016880c383a964f3d3542cfba64ada0e1d41cd]
Part 1: Provide a compress MM function for MM rests only
[r689f63e0473f8325b63d6bba66f9a56335548303]
Part 2: Amend docs to use \compressMMRests
[rba9a8dc730bb3a681455150bc0a3eef0976523af]
Part 3: Update snippets to use \compressMMRests
[rf509f0d4df0b7d4336d904ca61902ad67dc2be5e]
Part 4: Update regression tests to use \compressMMRests
[rbe72b961ab5c4f6ade95e962c872babf7b0338a0]

Trevor

Labels: -Patch-push Fixed_2_19_20
Status: Fixed

[Google Importer]

Originally posted by: tdanielsmusic

Issue 4375 has been created to cover points 3 and 4 in #3 above.

[Google Importer]

Originally posted by: PhilEHol...@googlemail.com

Verified on 2.20.20

Status: Verified