On Thu, Feb 20, 2020 at 12:58:32PM -0800, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@xxxxxxxxxx> writes:
>
> > +static void get_populated_hooks(struct strbuf *hook_info, int nongit)
> > +{
> > + /*
> > + * Doesn't look like there is a list of all possible hooks; so below is
> > + * a transcription of `git help hooks`.
> > + */
>
> It may want to become a NEEDSWORK comment.
Oh, good point. I'll rephrase it.
>
> > + const char *hooks = "applypatch-msg,"
> > + "pre-applypatch,"
> > + "post-applypatch,"
> > + "pre-commit,"
> > + "pre-merge-commit,"
> > + "prepare-commit-msg,"
> > + "commit-msg,"
> > + "post-commit,"
> > + "pre-rebase,"
> > + "post-checkout,"
> > + "post-merge,"
> > + "pre-push,"
> > + "pre-receive,"
> > + "update,"
> > + "post-receive,"
> > + "post-update,"
> > + "push-to-checkout,"
> > + "pre-auto-gc,"
> > + "post-rewrite,"
> > + "sendemail-validate,"
> > + "fsmonitor-watchman,"
> > + "p4-pre-submit,"
> > + "post-index-changex";
>
> Typo here?
Yep, I imagine from trying to press 'x' to remove the ',' from my list.
>
> > + struct string_list hooks_list = STRING_LIST_INIT_DUP;
> > + struct string_list_item *iter = NULL;
> > +
> > +
> > + if (nongit) {
> > + strbuf_addstr(hook_info,
> > + "not run from a git repository - no hooks to show\n");
> > + return;
> > + }
> > +
> > + string_list_split(&hooks_list, hooks, ',', -1);
> > +
> > + for_each_string_list_item(iter, &hooks_list) {
>
> I do not get why you want to use string_list for this, especially if
> you need to use string_list_split.
>
> To me,
>
> int i;
> const char *hook[] = {
> "applypatch-msg",
> "pre-applypatch",
> ...
> "post-index-change",
> };
>
> for (i = 0; i < ARRAY_SIZE(hook); i++) {
> if (hook[i] is enabled)
> strbuf_addf(hook_info, "%s\n", hook[i]);
> }
>
> would be far easier to understand. Do you have an external source
> that can feed you a single long string of comma separated hook names
> in mind, so that the initialization of *hooks will become simpler
> that way, or something?
Huh. I'm having a hard time remembering why I did it this way. I think I
wanted to use the string_list foreach? But you're right that it is
pretty hard to understand, plus expensive; I'll just use an array like
you suggest.
>
> > + if (find_hook(iter->string)) {
> > + strbuf_addstr(hook_info, iter->string);
> > + strbuf_complete_line(hook_info);
> > + }
> > + }
> > +}
> > +
> > static const char * const bugreport_usage[] = {
> > N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"),
> > NULL
> > @@ -166,6 +216,9 @@ int cmd_main(int argc, const char **argv)
> > get_header(&buffer, "Safelisted Config Info");
> > get_safelisted_config(&buffer);
> >
> > + get_header(&buffer, "Enabled Hooks");
> > + get_populated_hooks(&buffer, nongit_ok);
> > +
> > report = fopen_for_writing(report_path.buf);
> >
> > if (report == NULL) {
