From 23be819607ba813e6c8d0775371adbf6d24b6ac8 Mon Sep 17 00:00:00 2001
From: Jean Abou Samra <address@hidden>
Date: Wed, 4 Mar 2020 12:15:08 +0100
Subject: [PATCH] Clean up warnings in lily/change-iterator.cc

Improve the wordings (hopefully), with special cases for
frequent mistakes. Remove a redundant and confusing warning.
---
 lily/change-iterator.cc         | 63 ++++++++++++++++++---------------
 lily/include/change-iterator.hh |  3 +-
 2 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/lily/change-iterator.cc b/lily/change-iterator.cc
index 21dfbd112f..e35eb35eae 100644
--- a/lily/change-iterator.cc
+++ b/lily/change-iterator.cc
@@ -33,26 +33,16 @@ Change_iterator::error (const string &reason)
   string to_type = ly_symbol2string (get_music ()->get_property ("change-to-type"));
   string to_id = ly_scm2string (get_music ()->get_property ("change-to-id"));
 
-  string warn1 = _f ("cannot change `%s' to `%s'", to_type, to_id)
+  string message = _f ("cannot change `%s' to `%s'", to_type, to_id)
                  + ": " + reason;
-
-  /*
-    GUHG!
-  */
-  string warn2 = "Change_iterator::process (): "
-                 + get_outlet ()->context_name () + " = `"
-                 + get_outlet ()->id_string () + "': ";
-  warning (warn2);
-  get_music ()->origin ()->warning (warn1);
+  get_music ()->origin ()->warning (message);
 }
 
-string
+void
 Change_iterator::change_to (Music_iterator &it,
                             SCM to_type,
                             const string &to_id)
 {
-  string result; // error message
-
   // Find the context that should have its parent changed.
   Context *last = find_context_above_by_parent_type (it.get_outlet (), to_type);
   if (last)
@@ -66,23 +56,44 @@ Change_iterator::change_to (Music_iterator &it,
         }
       else
         {
+          // We could not find a parent context to move to.
           Input *ori = it.get_music ()->origin ();
           ori->warning (_f ("cannot find context to change to: %s",
                             Context::diagnostic_id (to_type, to_id).c_str ()));
         }
     }
-  else if (it.get_outlet ()->is_alias (to_type))
+  else
     {
-      // No enclosing context of the right kind was found
-      // and the iterator's immediate context is the kind that was sought.
-
-      result = _f ("not changing to same context type: %s", ly_symbol2string (to_type).c_str ());
+    // We could not find a child context to move.
+    Input *ori = it.get_music ()->origin ();
+    string name = ly_symbol2string (to_type).c_str (); // The context we should have switched to
+    // Special-case some common errors.
+    if (it.get_outlet ()->is_alias (to_type))
+      {
+      /* No enclosing context that is suitable for being moved was found
+         and the iterator's immediate context is the kind that was sought
+         for moving to. */
+      if (name == "Staff")
+        {
+        ori->warning ("cannot change this Staff from Staff to Staff; "
+                      "an explicit Voice should be instantiated");
+        }
+      else if (name == "Voice")
+        {
+          ori->warning( "cannot switch between Voice contexts; "
+                        "there should be a child context to move" );
+        }
+      else
+        {
+          ori->warning (_f ("cannot locate the context to be moved; "
+                            "we are already in a %s", name));
+        }
+      }
+    else
+      {
+        ori->warning ("cannot locate the context to be moved");
+      }
     }
-  else
-    /* FIXME: incomprehensible message */
-    result = _ ("none of these in my family");
-
-  return result;
 }
 
 /*
@@ -93,11 +104,7 @@ Change_iterator::process (Moment m)
 {
   SCM to_type = get_music ()->get_property ("change-to-type");
   string to_id = ly_scm2string (get_music ()->get_property ("change-to-id"));
-
-  string msg = change_to (*this, to_type, to_id);
-  if (!msg.empty ())
-    error (msg);
-
+  change_to (*this, to_type, to_id);
   Simple_music_iterator::process (m);
 }
 
diff --git a/lily/include/change-iterator.hh b/lily/include/change-iterator.hh
index e28c10733d..844afdf0ac 100644
--- a/lily/include/change-iterator.hh
+++ b/lily/include/change-iterator.hh
@@ -30,8 +30,7 @@ public:
   DECLARE_SCHEME_CALLBACK (constructor, ());
   OVERRIDE_CLASS_NAME (Change_iterator);
 
-  // returns an error message (empty on success)
-  static std::string change_to (Music_iterator &it,
+  static void change_to (Music_iterator &it,
                            SCM to_type, const std::string &to_id);
 
 private:
-- 
2.17.1