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

Issue 349740043: Issue 5366: Move warnings out of find/create context functions (Closed)

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

Description

https://sourceforge.net/p/testlilyissues/issues/5366/ The motivation for this is that Context::find_create_context () and find_context_near () should probably be merged for maintainability, but one of the differences between them that must be dealt with is that find_create_context () logs when it fails and find_context_near () does not. Adding warnings to find_context_near () risks being too noisy, leaving the option taken here. The new method Context::diagnostic_id (name, id) returns a formatted string (e.g. "Voice" or "Voice = mel") for use in a log message. It is used for the warnings that are being moved as well as a few other existing warnings to increase consistency. This work was suggested during discussion of https://codereview.appspot.com/344050043/.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -74 lines) Patch
M input/regression/context-defaultchild-cycle.ly View 1 chunk +4 lines, -4 lines 0 comments Download
M input/regression/lyric-combine-empty-warning.ly View 1 chunk +2 lines, -2 lines 0 comments Download
M lily/change-iterator.cc View 1 chunk +5 lines, -2 lines 0 comments Download
M lily/context.cc View 5 chunks +27 lines, -22 lines 0 comments Download
M lily/context-def.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M lily/context-specced-music-iterator.cc View 2 chunks +20 lines, -3 lines 0 comments Download
M lily/include/context.hh View 2 chunks +6 lines, -3 lines 0 comments Download
M lily/lyric-combine-music-iterator.cc View 1 chunk +16 lines, -12 lines 0 comments Download
M lily/quote-iterator.cc View 1 chunk +15 lines, -9 lines 0 comments Download
M lily/simultaneous-music-iterator.cc View 2 chunks +19 lines, -13 lines 0 comments Download

Messages

Total messages: 1
Carl
5 years, 9 months ago (2018-07-03 10:48:35 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