Jeff King <peff@xxxxxxxx> writes: >> +void trace_run_command(const struct child_process *cp) >> +{ >> + struct strbuf buf = STRBUF_INIT; >> + >> + if (!prepare_trace_line(__FILE__, __LINE__, >> + &trace_default_key, &buf)) >> + return; > > I think technically this should be TRACE_CONTEXT instead of __FILE__, > though I wonder if anybody has ever set that (there is not even a > Makefile knob for it, so you'd have to -DTRACE_CONTEXT manually). Oooh. Today I learned a new thing ;-) > I actually wonder if it would be nicer to keep this whole thing in > run-command.c, and just prepare it all in a buffer. That's basically > what we're doing here anyway, and then we could get rid of the funny > __FILE__ stuff. I.e., something like: > > struct strbuf buf = STRBUF_INIT; > > if (!trace_want(&trace_default_key)) > return; > > strbuf_addf(&buf, "trace: run_command: "); > sq_quote_argv_pretty(&buf, cp->argv); > > trace_printf("%s", buf.buf); > strbuf_release(&buf); > > AFAICT we aren't really using any internals of trace.c in our new > function. That would get rid of this __FILE__ bit, it would eliminate > the need for trace.h to know about "struct child_process", and it would > mean the output still says "run-command.c" in it. Sounds nice.