Thread (1 message) 1 message, 1 author, 2020-11-06

Re: [PATCH] Lengthening FORMAT_PATCH_NAME_MAX to 80

From: Junio C Hamano <hidden>
Date: 2020-11-06 17:45:16

hukeping [off-list ref] writes:
quoted
I do not mind getting rid of the "FORMAT_PATCH_NAME_MAX" constant and
replacing it with a variable that defaults to 64 and can be tweaked by a command
line option and/or a configuration variable.
It does not feel it is worth the effort to replace one hardcoded constant with
another hardcoded constant.
quoted
Looking at the code which uses the constant, I suspect it could also
be made simpler:

  - the PATH_MAX check in open_next_file() seems pointless. Once upon a
    time it mattered for fitting into a PATH_MAX buffer, but these days
    we use a dynamic buffer anyway. We are probably better off to just
    feed the result to the filesystem and see if it complains (since
    either way we are aborting; I'd feel differently if we adjusted our
    truncation size)

  - the logic in fmt_output_subject() could probably be simpler if the
    constant was "here's how long the subject should be", not "here's
    how long the whole thing must be".

But those are both orthogonal to your patch and can be done separately.
Yes, these clean-ups seem worth doing.
Agreed, and I'd like to do it with two separated commits:
- commit-1,  cleanup the open_next_file() by drop the if (filename.len>=..) statements.

- commit-2,  replace FORMAT_PATCH_NAME_MAX in fmt_output_subject() with a constant
  in there and make it to 80(or other value?), and drop FORMAT_PATCH_NAME_MAX
  from log-tree.h.

Is this works for you?
I am not sure what you meant by "Agreed".  I said two things:

 - It is dubious that it is worth the effort to replace a hardcoded
   constant with another.  Making it configurable with command line
   option and/or configuration variable may be worth doing.

 - Two observations Peff made for further clean-up are probably
   worth doing.

If you are agreeing to both of the above and following through, then
yes, it seems like a good plan.  If agreement is only to the former,
it probably still is worth doing.  Anything else, I don't know.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help