On Tue, Aug 16, 2016 at 11:22 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jacob Keller <jacob.e.keller@xxxxxxxxx> writes: > >> To make this work, we have to fix a few bugs in the graph API that force >> graph_show_commit_msg to be used only when you have a valid graph. >> Additionally, we extend the default_diff_output_prefix handler to work >> even when no graph is enabled. >> >> This is somewhat of a hack on top of the graph API, but I think it >> should be acceptable here. > > Unlike the opt-prefix-length I removed in 1/3, the length of the > line-prefix will never change during the lifetime of a single > diff_options struct, so it might turn out that repeated strlen() > on it for each and every line output is wasteful. > I could change this to store the length at option initialization, probably a good idea. Would it be better to just store it as a strbuf, since these know their length already? Or just add a line_prefix_length field? I agree that calling strlen a lot is probably wasteful. > Other than that, I didn't spot anything immediately questionable. > Thanks. I definitely feel like this is somewhat abusing the graph API because we're adding something not exactly graph-related. But in some ways it really is graph related as well, so I think it makes some sense. Regards, Jake > Thanks. -- 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