git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Bug in "git am" when the body starts with spaces

From
Jeff King <peff@peff.net>
Date
Apr 4, 2017, 06:50 UTC
Message-ID
<20170404065059.3p3lzxj64mrvdtpi@sigill.intra.peff.net>
In-Reply-To
<f3f2b6f0-c6bb-f50a-5030-61afe941eb23@google.com>
On Mon, Apr 03, 2017 at 10:42:46AM -0700, Jonathan Tan wrote:
Show 29 quoted lines
> On 04/01/2017 09:18 PM, Jeff King wrote:
> > On Sat, Apr 01, 2017 at 12:03:44PM -0700, Linus Torvalds wrote:
> > > The logic is fairly simple: if we encounter an empty line, and we have
> > > pending in-body headers, we flush the pending headers, and mark us as
> > > no longer in header mode.
> > 
> > Hmm. I think this may work. At first I thought it was too strict in
> > always checking inbody_header_accum.len, because we want this to kick in
> > always, whether there's whitespace continuation or not. But that
> > accumulator has to collect preemptively, before it knows if there's
> > continuation. So it will always be non-empty if we've seen _any_ header,
> > and it will remain non-empty as long as we keep parsing (because any
> > time we flush, we do so in order to handle another line).
> > 
> > IOW, I think this implements the state-machine thing I wrote in my
> > earlier email, because the state "are we inside in-body header parsing"
> > is always reflected by having a non-empty accumulator. It is a bit
> > non-obvious though.
> 
> About obviousness, I think of a non-empty accumulator merely representing
> that the next line could potentially be a continuation line. And it is
> coincidence that this implies "are we inside in-body header parsing"; if not
> all in-body header lines could be "continued", there would be no such
> implication.
> 
> mi->inbody_header_accum.len is already used in check_inbody_header to mean
> "could the next line potentially be a continuation line" and to trigger a
> check for a negative criterion (in this case, a scissors line). I think it's
> fine to do the same thing, the negative criterion here being a blank line.

FWIW, I looked into making a single "state" variable, but it actually got ugly pretty quickly. I think not because it's not a cleaner method in the long run, but just because the existing code is so dependent on individual state variables that needed changing. So I'm OK with Linus's fix; it certainly follows the existing code patterns.

-Peff
Previous: Jonathan TanNext: Junio C Hamano
Message 7 of 8 in “Bug in "git am" when the body starts with spaces”
  1. Linus TorvaldsApr 1, 2017
  2. Linus TorvaldsApr 1, 2017
  3. Jeff KingApr 1, 2017
  4. Linus TorvaldsApr 1, 2017
  5. Jeff KingApr 2, 2017
  6. Jonathan TanApr 3, 2017
  7. Jeff KingApr 4, 2017
  8. Junio C HamanoApr 2, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.