Vikrant Varma wrote: > I agree with Matthieu, the people who don't want to see this advice never > will, because they won't make that mistake. Maybe advice is the wrong word, > corrections might be more appropriate. Makes sense. Perhaps it would make sense to hook into help.autocorrect. I would definitely like that. >> I see that there are other structs in our codebase suffixing _cb, to >> indicate "callback data". I normally reserve _cb for callback >> functions. > > > I'm following the convention (builtin/merge.c: struct append_ref_cb). If > there's a better way to name it, I'll use that. Fine leaving it as it is. > ref_cb.similar_refs has already been defined. The compiler won't let me > assign to it unless I cast first. However, I think compound literals are a > C99/gcc feature. Is this better? > > struct similar_ref_cb ref_cb = {ref, STRING_LIST_INIT_NODUP}; As Johannes pointed out, ref is a variable and that is problematic. Leave the cast on: I didn't notice the compiler warning in my head. > Q_ is a pluralization helper that picks one of the two strings based on > ref_cb.similar_refs.nr. It's used in help.c:help_unknown_cmd for the same > reason. Thanks. > Again, because help_unknown_cmd exited with 1. I've tried to follow the > convention as laid down there. Ah, I didn't notice that. > What's the significance of the error code for > die()? When something _really_ bad happens, exit() with 128, without bothering to go up the callstack. Return error() is a common idiom to print an error message immediately, and propagate the return status -1 up the callstack. > When is it correct to use die(), and when to use error() followed by > exit(128)? exit(128) is a bit rare [grep for it yourself]. It's for when you want to die(), but exit() only after doing an important task in the caller. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html