Re: Issue 5362: Generalize context searches (issue 344050043 by address@hidden)

[nine . fierce . ballads]

https://codereview.appspot.com/344050043/diff/20001/lily/change-iterator.cc
File lily/change-iterator.cc (right):

https://codereview.appspot.com/344050043/diff/20001/lily/change-iterator.cc#newcode67
lily/change-iterator.cc:67: Context::warning_cannot_find (origin,
to_type, to_id);
On 2018/07/01 13:21:43, dak wrote:
> Having a separate function for each different warning text seems
clumsy.  Can
> this be factored into a more generic warning function and possibly
something
> formatting to_type and to_id?

I'm willing to change this, but it's not perfectly clear to me what you
are asking for.  (The only change I would object to is pushing the
logging back into the find/create functions, and you're obviously not
asking for that.)

Would you prefer a generic message saying "cannot find or create ..."
even when only one has been tried?  Would you prefer that the caller
provide the mode as a function parameter?

I am also having trouble imagining what additional formatting of to_type
and to_id you are thinking of.  One is a symbol and one is a string, and
the warning functions print them as strings.  If you mean you want the
caller to provide some formatting, I would prefer to leave Context in
control of the formatting to help maintain consistency.

https://codereview.appspot.com/344050043/