Re: [PATCH] Extra information in autoload error

[Eli Zaretskii <eliz@gnu.org>]

> From: Alex <agrambot@gmail.com>
> Date: Sat, 22 Oct 2016 10:23:06 -0600
> Cc: emacs-devel@gnu.org
> 
> Alex <agrambot@gmail.com> writes:
> 
> > Davis Herring <herring@lanl.gov> writes:
> >
> >>> +	error ("Autoloading failed to define function %s in file %s",
> >>
> >> I'd phrase it as "Autoloading file %s failed to define function %s". (It's a bit
> >> odd to say the function is in the file when we've just learned it isn't.)
> >>
> >> Davis
> >
> > Thanks, I agree that it's better. I suppose I wanted to append rather
> > than rewrite the message, but there's really no reason to hold back.
> >
> > The updated patches follow:
> >
> > diff --git a/src/eval.c b/src/eval.c
> > index e08a25a..4daf052 100644
> > --- a/src/eval.c
> > +++ b/src/eval.c
> > @@ -1987,7 +1987,8 @@ it defines a macro.  */)
> >        Lisp_Object fun = Findirect_function (funname, Qnil);
> >  
> >        if (!NILP (Fequal (fun, fundef)))
> > -	error ("Autoloading failed to define function %s",
> > +	error ("Autoloading file %s failed to define function %s",
> > +	       SDATA (Fcar (Fcar (Vload_history))),
> >  	       SDATA (SYMBOL_NAME (funname)));
> >        else
> >  	return fun;
> >
> > diff --git a/src/eval.c b/src/eval.c
> > index e08a25a..8c5c5e5 100644
> > --- a/src/eval.c
> > +++ b/src/eval.c
> > @@ -1987,7 +1987,8 @@ it defines a macro.  */)
> >        Lisp_Object fun = Findirect_function (funname, Qnil);
> >  
> >        if (!NILP (Fequal (fun, fundef)))
> > -	error ("Autoloading failed to define function %s",
> > +	error ("Autoloading filename %s failed to define function %s",
> > +	       SDATA (Fcar (Fcdr (fundef))),
> >  	       SDATA (SYMBOL_NAME (funname)));
> >        else
> >  	return fun;
> 
> Ping. Does anyone have an opinion on which of the two diffs is better?

The second one, IMO.