Reviewers: ,
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.
Please review this at https://codereview.appspot.com/338540043/
Affected files (+53, -45 lines):
M input/regression/warn-conflicting-key-signatures.ly
M lily/include/stream-event.hh
M lily/include/translator.hh
M lily/stream-event.cc
M lily/translator.cc
Index: input/regression/warn-conflicting-key-signatures.ly
diff --git a/input/regression/warn-conflicting-key-signatures.ly
b/input/regression/warn-conflicting-key-signatures.ly
index
7d805072a78c089af89bc54d7e7c29cf11c631bc..eb228ebe484ab2f2d42fd5d98f0287488a686e01
100644
--- a/input/regression/warn-conflicting-key-signatures.ly
+++ b/input/regression/warn-conflicting-key-signatures.ly
@@ -1,7 +1,7 @@
\version "2.16.0"
#(ly:set-option 'warning-as-error #f)
-#(ly:expect-warning (ly:translate-cpp-warning-scheme "Two simultaneous %s
events, junking this one") "key-change")
-#(ly:expect-warning (ly:translate-cpp-warning-scheme "Previous %s event
here") "key-change")
+#(ly:expect-warning (ly:translate-cpp-warning-scheme "Junking this %s
event ...") "key-change")
+#(ly:expect-warning (ly:translate-cpp-warning-scheme "... because it
conflicts with this %s event") "key-change")
\header {
texidoc = "If you specify two different key signatures at one point, a
@@ -9,7 +9,7 @@ warning is printed."
}
-\score {
+\score {
\context Voice <<
{ \key cis \major cis4 \key bes \major bes4 }
{ \key cis \major fis4 \key es \major g4 }
Index: lily/include/stream-event.hh
diff --git a/lily/include/stream-event.hh b/lily/include/stream-event.hh
index
d554f584dcec8eaaeb34556edc1a465a1e0784cf..927e390b2331206cd8cefe97768944a08df5a0df
100644
--- a/lily/include/stream-event.hh
+++ b/lily/include/stream-event.hh
@@ -48,4 +48,28 @@ public:
SCM ly_event_deep_copy (SCM);
+extern void
+warn_reassign_event_ptr (Stream_event &old_ev, Stream_event *new_ev);
+
+// Assign to old_ev at most once, returning true when it happens, and
warning
+// (with some exceptions) about attempted reassignments.
+inline bool
+assign_event_ptr_once (Stream_event *&old_ev, Stream_event *new_ev)
+{
+ if (!old_ev)
+ {
+ old_ev = new_ev;
+ return old_ev;
+ }
+ else
+ {
+ warn_reassign_event_ptr (*old_ev, new_ev);
+ return false;
+ }
+}
+
+// This macro is no longer necessary.
+// If you have time to eliminate it, go ahead.
+#define ASSIGN_EVENT_ONCE(o,n) assign_event_ptr_once (o, n)
+
#endif /* STREAM_EVENT_HH */
Index: lily/include/translator.hh
diff --git a/lily/include/translator.hh b/lily/include/translator.hh
index
91bd59fe4aef5c72efe334a9687a04dd2d1626ad..33acd337f137e1777b5c2f8e79b2b2cebc4db6bb
100644
--- a/lily/include/translator.hh
+++ b/lily/include/translator.hh
@@ -224,10 +224,4 @@ SCM get_translator_creator (SCM s);
Moment get_event_length (Stream_event *s, Moment now);
Moment get_event_length (Stream_event *s);
-/*
- This helper is only meaningful inside listen_* methods.
-*/
-extern bool internal_event_assignment (Stream_event **old_ev, Stream_event
*new_ev, const char *function);
-#define ASSIGN_EVENT_ONCE(o,n) internal_event_assignment (&o, n,
__FUNCTION__)
-
#endif // TRANSLATOR_HH
Index: lily/stream-event.cc
diff --git a/lily/stream-event.cc b/lily/stream-event.cc
index
64c9a979d9950907dde19ffbb0a49978a5a21f12..16a230f44d249633205d15c004f7338b889ab0e7
100644
--- a/lily/stream-event.cc
+++ b/lily/stream-event.cc
@@ -21,8 +21,10 @@
#include "context.hh"
#include "input.hh"
+#include "international.hh"
#include "music.hh"
#include "pitch.hh"
+#include <string>
/* TODO: Rename Stream_event -> Event */
@@ -113,3 +115,27 @@ Stream_event::undump (SCM data)
obj->mutable_property_alist_ = scm_reverse (scm_cdr (data));
return obj->unprotect ();
}
+
+void
+warn_reassign_event_ptr (Stream_event &old_ev, Stream_event *new_ev)
+{
+ if (!new_ev) // not expected
+ return;
+
+ if (to_boolean (scm_equal_p (old_ev.self_scm (), new_ev->self_scm ())))
+ return; // nothing of value was lost
+
+ std::string nc = ly_symbol2string (scm_car (new_ev->get_property
("class")));
+ std::string oc = ly_symbol2string (scm_car (old_ev.get_property
("class")));
+
+ // remove the suffix "-event" if it is present
+ const std::string suffix("-event");
+ if (0 == nc.compare(nc.size() - suffix.size(), suffix.size(), suffix))
+ nc.erase(nc.size() - suffix.size());
+ if (0 == oc.compare(oc.size() - suffix.size(), suffix.size(), suffix))
+ oc.erase(oc.size() - suffix.size());
+
+ new_ev->origin ()->warning (_f ("Junking this %s event ...", nc.c_str
()));
+ old_ev.origin ()->warning (_f ("... because it conflicts with this %s
event",
+ oc.c_str ()));
+}
Index: lily/translator.cc
diff --git a/lily/translator.cc b/lily/translator.cc
index
e57e1c391af53eec7dc93253da33fe28d5cff8e7..9f489102d5a0251a115fe0ea3606b3836cb9fd6b
100644
--- a/lily/translator.cc
+++ b/lily/translator.cc
@@ -22,7 +22,6 @@
#include "context-def.hh"
#include "dispatcher.hh"
#include "global-context.hh"
-#include "international.hh"
#include "translator-group.hh"
#include "warn.hh"
@@ -266,39 +265,4 @@ get_event_length (Stream_event *e, Moment now)
return len;
}
-/*
- Helper, used through ASSIGN_EVENT_ONCE to throw warnings for
- simultaneous events. The helper is only useful in listen_* methods
- of translators.
-*/
-bool
-internal_event_assignment (Stream_event **old_ev, Stream_event *new_ev,
const char *function)
-{
- if (*old_ev
- && !to_boolean (scm_equal_p ((*old_ev)->self_scm (),
- new_ev->self_scm ())))
- {
- /* extract event class from function name */
- string ev_class = function;
-
- /* This assertion fails if EVENT_ASSIGNMENT was called outside a
- translator listener. Don't do that. */
- const char *prefix = "listen_";
- assert (0 == ev_class.find (prefix));
-
- /* "listen_foo_bar" -> "foo-bar" */
- ev_class.erase (0, strlen (prefix));
- replace_all (&ev_class, '_', '-');
-
- new_ev->origin ()->warning (_f ("Two simultaneous %s events, junking
this one", ev_class.c_str ()));
- (*old_ev)->origin ()->warning (_f ("Previous %s event here",
ev_class.c_str ()));
- return false;
- }
- else
- {
- *old_ev = new_ev;
- return true;
- }
-}
-
// Base class. Not instantiated. No ADD_TRANSLATOR call.
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
