Re: [PATCH v2 4/6] trailer: teach find_patch_start about --no-divider

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 11, 2023, 17:25 UTC
Message-ID: <xmqqv8cgvd02.fsf@gitster.g>
In-Reply-To: <f5f507c4c6c4514af7dca35e307ca68e72435afb.1694240177.git.gitgitgadget@gmail.com>

"Linus Arver via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 14 quoted lines

> From: Linus Arver <linusa@google.com>
>
> Currently, find_patch_start only finds the start of the patch part of
> the input (by looking at the "---" divider) for cases where the
> "--no-divider" flag has not been provided. If the user provides this
> flag, we do not rely on find_patch_start at all and just call strlen()
> directly on the input.
>
> Instead, make find_patch_start aware of "--no-divider" and make it
> handle that case as well. This means we no longer need to call strlen at
> all and can just rely on the existing code in find_patch_start. By
> forcing callers to consider this important option, we avoid the kind of
> mistake described in be3d654343 (commit: pass --no-divider to
> interpret-trailers, 2023-06-17).

OK. The code pays attention to "---" so making it stop doing so when the "--no-*" option is given will make the function responsible for finding the beginning of the patch.

I wonder if we should rename this function while we are at it, though. When "--no-divider" is given, the expected use case is *not* to have a patch at all, and it is dubious that a function whose name is find_patch_start() can possibly do anything useful.

The real purpose of this function is to find the end of the log message, isn't it? And the caller uses the end of the log message it found and gives it to find_trailer_start() and find_trailer_end() as the upper boundary of the search for the trailer block.