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

Re: [PATCH 2/3] commit.c: add is_scissors_line

From
Jeff King <peff@peff.net>
Date
May 12, 2017, 08:40 UTC
Message-ID
<20170512084028.f6vv4uz6of6ofadu@sigill.intra.peff.net>
In-Reply-To
<20170512050347.30765-3-bmalehorn@gmail.com>
On Thu, May 11, 2017 at 10:03:46PM -0700, Brian Malehorn wrote:
> Move is_scissors_line to commit.c and expose it through commit.h.
> This is needed in commit.c, and mailinfo.c shouldn't really own it.

It was fine for mailinfo to own it until now, since it was the only user. :) I think there are some unsaid bits in your rationale. Perhaps something like:

 Subject: mailinfo: make is_scissors_line() public
 We parse scissors lines only when looking at emails, and thus the
 parsing function has always lived in mailinfo.c. However, we also
 generate scissors lines as part of "git commit -v". We should be able
 to reuse the same function to parse them.
 Once public, it doesn't really belong in mailinfo.c anymore. A better
 place is commit.c because...[I had trouble filling this in; it's really
 _not_ about commits in particular, but rather about the in-editor
 representation of the commit message used by git-commit].

Thinking on it more, though...are the scissors lines generated by "-v" really the same as the ones parsed by mailinfo? In the case of mailinfo, we are parsing an externally generated string according to a heuristic microformat. But with "commit -v", we are the ones who wrote the cut line in the first place. Shouldn't we be a bit more picky and make sure we parse the exact same cut line?

And indeed, we _do_ actually have to parse these cut lines already, when we read the commit message back in. The code is in wt_status_truncate_message_at_cut_line(), and it does do an exact match. We should probably be using that rather than making the mailinfo is_scissors_line() public.

-Peff
Previous: Brian MalehornNext: Jeff King
Message 3 of 24 in “interpret-trailers + commit -v bugfix”
  1. 0/3 interpret-trailers + commit -v bugfixBrian Malehorn, May 12, 2017
  2. 2/3 commit.c: add is_scissors_lineBrian Malehorn, May 12, 2017
  3. Jeff KingMay 12, 2017
  4. Jeff KingMay 12, 2017
  5. Brian MalehornMay 14, 2017
  6. interpret-trailers: obey scissors linesBrian Malehorn, May 14, 2017
  7. Jeff KingMay 14, 2017
  8. Brian MalehornMay 14, 2017
  9. interpret-trailers: obey scissors linesBrian Malehorn, May 14, 2017
  10. Junio C HamanoMay 15, 2017
  11. Junio C HamanoMay 15, 2017
  12. Brian MalehornMay 16, 2017
  13. interpret-trailers: honor the cut lineBrian Malehorn, May 16, 2017
  14. Junio C HamanoMay 16, 2017
  15. Jeff KingMay 15, 2017
  16. Junio C HamanoMay 15, 2017
  17. Jeff KingMay 15, 2017
  18. Junio C HamanoMay 15, 2017
  19. Jeff KingMay 15, 2017
  20. Ævar Arnfjörð BjarmasonMay 14, 2017
  21. 1/3 mailinfo.c: is_scissors_line ends on newlineBrian Malehorn, May 12, 2017
  22. Jeff KingMay 12, 2017
  23. 3/3 commit.c: skip scissors when computing trailersBrian Malehorn, May 12, 2017
  24. Jeff KingMay 12, 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.