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

Re: [PATCH 3/3] format-patch: support --output option

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Nov 4, 2020, 20:16 UTC
Message-ID
<CAPig+cSOB+raZHEpSeqnZZ-VHNqLqVyrfVUA8fCr97mv8oSBFw@mail.gmail.com>
In-Reply-To
<20201104191559.GA3053889@coredump.intra.peff.net>
On Wed, Nov 4, 2020 at 2:16 PM Jeff King <peff@peff.net> wrote:
Show 13 quoted lines
> On Wed, Nov 04, 2020 at 12:27:30PM -0500, Eric Sunshine wrote:
> > The commit message's justification for supporting --output seems
> > reasonable. However, my knee-jerk reaction to the implementation was
> > that it feels overly magical and a bit too hacky. I can see the logic
> > in it but it also leaves a bad taste when the implementation has to
> > "undo" a side-effect of some other piece of code, which makes it feel
> > unplanned and fragile. [...]
>
> FWIW, that unsetting of rev.diffopt.close_file is exactly how git-log
> does it. So while I agree it's a bit ugly, this is the intended way for
> --output to be used across multiple diffs, and with log_tree_commit().
> I'd prefer to go this way for now, and if somebody wants to make it less
> ugly, they can clean up all of the callers in one go.

If you intend to re-roll, it might make sense to include this explanation in the commit message since existing precedent helps sell the ugliness (bad taste), making for a less negative knee-jerk reaction.

Previous: Jeff KingNext: Junio C Hamano
Message 11 of 17 in “[Bug report] Crash when creating patch”
  1. Johannes PostlerNov 4, 2020
  2. Jeff KingNov 4, 2020
  3. 1/3 format-patch: refactor output selectionJeff King, Nov 4, 2020
  4. Eric SunshineNov 4, 2020
  5. Jeff KingNov 4, 2020
  6. 2/3 format-patch: tie file-opening logic to output_directoryJeff King, Nov 4, 2020
  7. Eric SunshineNov 4, 2020
  8. 3/3 format-patch: support --output optionJeff King, Nov 4, 2020
  9. Eric SunshineNov 4, 2020
  10. Jeff KingNov 4, 2020
  11. Eric SunshineNov 4, 2020
  12. Junio C HamanoNov 4, 2020
  13. format-patch --outputJeff King, Nov 4, 2020
  14. 1/3 format-patch: refactor output selectionJeff King, Nov 4, 2020
  15. 2/3 format-patch: tie file-opening logic to output_directoryJeff King, Nov 4, 2020
  16. 3/3 format-patch: support --output optionJeff King, Nov 4, 2020
  17. Johannes PostlerNov 5, 2020

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.