Jacob Keller [off-list ref] 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.
Other than that, I didn't spot anything immediately questionable.
Thanks.
On Tue, Aug 16, 2016 at 11:22 AM, Junio C Hamano [off-list ref] wrote:
Jacob Keller [off-list ref] writes:
quoted
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.