On Sat, Nov 04, 2017 at 10:07:00PM -0400, Jeff King wrote:
> Yes, I think what you've written here (and below) is quite close to the
> error_context patches I linked elsewhere in the thread. In other
> words, I think it's a sane approach.
In contrast to error_context I'd like to keep all exiting
behavior (die, ignore, etc.) in the hand of the caller and not
use any callbacks as that makes the control flow much harder to
follow.
> I agree it might be nice for the error context to have a positive "there
> was an error" flag. It's probably worth making it redundant with the
> return code, though, so callers can use whichever style is most
> convenient for them.
Agreed.
Regarding the API, should it be allowed to pass NULL as error
pointer to request no additional error handling or should the
error functions panic on NULL? Allowing NULL makes partial
conversions possible (e.g. for write_in_full) where old callers
just pass NULL and check the return values and converted callers
can use the error struct.
How should translations get handled? Appending ": %s" for
strerror(errno) might be problematic. Same goes for "outer
message: inner message" where the helper function just inserts ":
" between the messages. Is _("%s: %s") (with appropriate
translator comments) enough to handle these cases?
Suggestions how to name the struct and the corresponding
functions? My initial idea was struct error and to use error_ as
prefix, but I'm not sure if struct error is too broad and may
introduce conflicts with system headers. Also error_ is a little
long and could be shorted to just err_ but I don't know if that's
clear enough. The error_ prefix doesn't conflict with many git
functions, but there are some in usage.c (error_errno, error,
error_routine).
And as general question, is this approach to error handling
something we should pursue or are there objections? If there's
consensus that this might be a good idea I'll look into
converting some parts of the git code (maybe refs.c) to see how
it pans out.
Regards
Simon
--
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
