Menu

#2300 Patch: do not tinker with the position of a pitched rest

Verified
nobody
Enhancement
2012-02-19
2012-02-06
Anonymous
No

Originally created by: *anonymous

Originally created by: benko....@gmail.com
Originally owned by: benko....@gmail.com

do not tinker with the position of a pitched rest

http://codereview.appspot.com/5434061

Discussion

  • Google Importer

    Google Importer - 2012-02-06

    Originally posted by: dak@gnu.org

    Patchy the autobot says: LGTM.  But why don't you use the glyph with ledger line when the rest is 'skewed' inside of a system?

    Labels: Patch-review

     
  • Google Importer

    Google Importer - 2012-02-06

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

    > why don't you use the glyph with ledger line when the rest is 'skewed' inside of a system?

    well, if you (plural!) accept a wrongly placed rest in an ordinary staff to have a ledger line, I'm more than happy to modify the patch accordingly.

     
  • Google Importer

    Google Importer - 2012-02-06

    Originally posted by: dak@gnu.org

    Plural would be "youse" in Southern U.S.A., "ye" in Early Modern English.  My personal take is that nothing else makes sense for that input.  The visual feature defining the rest length of half and full rests is the relation to its base line, and when the base line is not there, you use a ledger line.

    If the base line is not there because you missed the grid inside of the staff, that's a perfectly valid reason for using a ledger line in my book.

     
  • Google Importer

    Google Importer - 2012-02-07

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

    (No comment was entered for this change.)

    Labels: -Patch-review Patch-countdown

     
  • Google Importer

    Google Importer - 2012-02-08

    Originally posted by: janek.li...@gmail.com

    Issue 2109 has been merged into this issue.

     

    Related

    Issues: #2109

  • Google Importer

    Google Importer - 2012-02-08

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

    (No comment was entered for this change.)

    Labels: -Patch-countdown Patch-needs_work

     
  • Google Importer

    Google Importer - 2012-02-08

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

    do not tinker with the position of a pitched rest

    http://codereview.appspot.com/5434061

    Labels: Patch-new

     
  • Google Importer

    Google Importer - 2012-02-09

    Originally posted by: dak@gnu.org

    Patchy the autobot says: LGTM.  Patch adds one blank line at EOF.  I was surprised at the short church rests with ledger lines: never saw one of those before.  But if we have them, they should likely be used just like you did.

    Labels: Patch-review

     
  • Google Importer

    Google Importer - 2012-02-09

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

    I was surprised that breve rests have two glyphs (with and without ledger lines) while longa has only one.  this is an inconsistency I not intended to resolve with this patch.

     
  • Google Importer

    Google Importer - 2012-02-09

    Originally posted by: dak@gnu.org

    Totally reasonable.  It is conceivable that the ledger line breve rest is there for the purpose of being able to properly reproduce a minority of engravings.  But without more definite information, I would keep things just as you did.  It might also be that a square blob outside of the system is less recognizable without ledger lines than a long block, and this was the motivation.  And it may be that the duration of a longa rest is such that you would never employ it in multi-voice situations, so there is no point in a glyph.

    Whatever the reason: if nobody digs up more definite references, my vote is on merging this patch.  Its behavior seems reasonable to me.

    Owner: benko....@gmail.com
    Status: Accepted

     
  • Google Importer

    Google Importer - 2012-02-09

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

    (No comment was entered for this change.)

    Labels: -Patch-review Patch-countdown

     
  • Google Importer

    Google Importer - 2012-02-12

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

    Counted down to 20120212, please push.

    Labels: -Patch-countdown Patch-push

     
  • Google Importer

    Google Importer - 2012-02-13

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

    Could someone (Janek?) with push right please push it?

     
  • Google Importer

    Google Importer - 2012-02-13

    Originally posted by: dak@gnu.org

    After having commented, I'll feel honored to push.

     
  • Google Importer

    Google Importer - 2012-02-13

    Originally posted by: dak@gnu.org

    Pushed as [r3d8f4559228bd8a4a30bb024163b64d425b76f18] to staging.

    Labels: -Patch-push Fixed_2_15_30
    Status: Fixed

     
  • Google Importer

    Google Importer - 2012-02-13

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

    thanks, David!

     
  • Google Importer

    Google Importer - 2012-02-19

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

    Verified committish for this patch is present in the repository.

     
  • Google Importer

    Google Importer - 2012-02-19

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

    Forgot to change status to Verified.

    Status: Verified

     
Auth0 Logo