On Fri, Aug 11, 2023 at 02:38:05PM -0700, Junio C Hamano wrote:
+ if (strbuf_read_file(buf, desc_file, 2000) < 0)
You would probably want to do "2000" -> "0" here.
hmm, yeah, i wonder where i got it from, given that there is no
precedent. i suppose i simply thought that 2k is a reasonably expectable
max size for a description. if you think the default 8k hint is a better
idea, then let's go with it.
static void prepare_cover_text(struct pretty_print_context *pp,
+ const char *description_file,
const char *branch_name,
This is kind of suboptimal, but let's let it pass.
A better design is to pass the description string itself to this
function and the make_cover_letter() function, and have the higher
level callers of the callchain prepare the either read_desc_file()
or read_branch_desc() to prepare that string before calling into the
callchain.
there is only one caller, and doing this change would essentially result
in inlining prepare_cover_text(). probably not the best outcome.
Such a division of labor between the callers and this
function will allow us to more easily add another option to the
command, to feed the description string itself (instead of having to
create a temporary file and storing the description in it).
that's a good point. in fact, passing in the description directly would
probably fit my use case better ... i just happened to already have the
code for creating that temp file anyway (for editing), so i didn't give
it a second thought. i can add both options in the same go, given that
it's almost no code.
regards