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

Issue 338540043: Issue 5284: improve ASSIGN_EVENT_ONCE (Closed)

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

Description

https://sourceforge.net/p/testlilyissues/issues/5284/ * Really assign the event only once. * End an unnecessary relationship with translator listeners. * Rephrase the warning so that it could be used more broadly than for simultaneous events. * Rephrase the warning so that it could be used with events of different classes (e.g. abc-mark-event and xyz-mark-event). * Inline the common case that the event is assigned without warning.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Address feedback and fix white space #

Total comments: 1

Patch Set 3 : Rephrase warning #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -45 lines) Patch
M input/regression/warn-conflicting-key-signatures.ly View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M lily/include/stream-event.hh View 1 1 chunk +23 lines, -0 lines 0 comments Download
M lily/include/translator.hh View 1 chunk +0 lines, -6 lines 0 comments Download
M lily/stream-event.cc View 1 2 2 chunks +18 lines, -0 lines 0 comments Download
M lily/translator.cc View 2 chunks +0 lines, -36 lines 0 comments Download

Messages

Total messages: 6
Dan Eble
6 years ago (2018-03-04 21:11:16 UTC) #1
dak
https://codereview.appspot.com/338540043/diff/1/lily/include/stream-event.hh File lily/include/stream-event.hh (right): https://codereview.appspot.com/338540043/diff/1/lily/include/stream-event.hh#newcode51 lily/include/stream-event.hh:51: extern void Using "extern" for function prototypes is weird. ...
6 years ago (2018-03-04 22:08:52 UTC) #2
Dan Eble
Test string length & fix white space
6 years ago (2018-03-05 00:34:55 UTC) #3
Carl
I haven't tested the functionality, but I noticed that the warning messages were not consistent ...
6 years ago (2018-03-07 16:51:51 UTC) #4
Dan Eble
Rephrase warning
6 years ago (2018-03-07 23:54:41 UTC) #5
Dan Eble
6 years ago (2018-03-07 23:57:34 UTC) #6
On 2018/03/07 23:54:41, Dan Eble wrote:
> Rephrase warning

Notice that because the flow of the message is quite different now, I chose not
to bother stripping "-event" from the event name anymore.
Sign in to reply to this message.

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