Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(56)

Issue 342210043: Remove Grob_info::origin_contexts ()

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 9 months ago by dak
Modified:
5 years, 9 months ago
Reviewers:
Dan Eble
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Remove Grob_info::origin_contexts () I cannot believe this: this function is obscure both in definition and usage. Most uses were completely nonsensical, and one use just an overly complex method of achieving a semi-random goal. I cannot figure out what the original aim of the respective code passages was, but whatever it was, seemingly other workarounds took its place successfully. Also contains some preparatory commits: Remove Grob_info::origin_contexts ().size () uses The size of Grob_info::origin_contexts () was used several times as a flag, but the function could never return an empty array anyway. Sanitize Break_align_engraver::create_alignment source Break_align_engraver went to some contortions to get an engraver from the same context as the grob it acknowledged. There is no real point in it not just using the origin engraver for that purpose, however.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -39 lines) Patch
M lily/break-align-engraver.cc View 1 chunk +6 lines, -6 lines 0 comments Download
M lily/grid-line-span-engraver.cc View 1 chunk +6 lines, -10 lines 0 comments Download
M lily/grob-info.cc View 1 chunk +0 lines, -15 lines 0 comments Download
M lily/include/grob-info.hh View 1 chunk +0 lines, -1 line 0 comments Download
M lily/span-arpeggio-engraver.cc View 1 chunk +1 line, -2 lines 0 comments Download
M lily/span-bar-engraver.cc View 1 chunk +1 line, -2 lines 0 comments Download
M lily/vertical-align-engraver.cc View 1 chunk +1 line, -3 lines 0 comments Download

Messages

Total messages: 1
Dan Eble
5 years, 9 months ago (2018-07-07 21:16:58 UTC) #1
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b