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

Re: [PATCH v5 08/10] rebase -i: skip unnecessary picks using the rebase--helper

From
Jeff King <peff@peff.net>
Date
Jun 19, 2017, 16:10 UTC
Message-ID
<20170619161051.silyrlwrnjjspoxe@sigill.intra.peff.net>
In-Reply-To
<alpine.DEB.2.21.1.1706191127450.57822@virtualbox>
On Mon, Jun 19, 2017 at 11:45:50AM +0200, Johannes Schindelin wrote:
> The reason for this suggestion is that one of the revision machinery's
> implementation details is an ugly little semi-secret: the pretty-printing
> machinery uses a global state, and that is why we need the "pretty_given"
> flag in the first place.

I think that's mis-stating Junio's complaint. The point is not the pretty_given flag itself, which we know about and can work around. The point is that we don't know what other similar problems we have or will have due to future changes in the revision code.

In other words, there are two APIs: the one where C code manipulates rev_info directly, and the one where revision.c responds to string arguments. From a maintenance perspective, it is easy for somebody make a change that works for the latter but not the former.

I do agree that the lack of compile-time safety for obvious mistakes like "--pertty" is a downside, though. On the other hand, there are strong run-time checks there, so the tests would catch it.

I do not have a strong opinion myself in either direction.
-Peff
Previous: Johannes SchindelinNext: Junio C Hamano
Message 14 of 20 in “The final building block for a faster rebase -i”
  1. 00/10 The final building block for a faster rebase -iJohannes Schindelin, Jun 14, 2017
  2. 02/10 rebase -i: generate the script via rebase--helperJohannes Schindelin, Jun 14, 2017
  3. 01/10 t3415: verify that an empty instructionFormat is handled as beforeJohannes Schindelin, Jun 14, 2017
  4. 03/10 rebase -i: remove useless indentationJohannes Schindelin, Jun 14, 2017
  5. 04/10 rebase -i: do not invent onelines when expanding/collapsing SHA-1sJohannes Schindelin, Jun 14, 2017
  6. 05/10 rebase -i: also expand/collapse the SHA-1s via the rebase--helperJohannes Schindelin, Jun 14, 2017
  7. 06/10 t3404: relax rebase.missingCommitsCheck testsJohannes Schindelin, Jun 14, 2017
  8. 07/10 rebase -i: check for missing commits in the rebase--helperJohannes Schindelin, Jun 14, 2017
  9. 08/10 rebase -i: skip unnecessary picks using the rebase--helperJohannes Schindelin, Jun 14, 2017
  10. Liam BeguinJun 16, 2017
  11. Johannes SchindelinJun 16, 2017
  12. Liam BeguinJun 17, 2017
  13. Johannes SchindelinJun 19, 2017
  14. Jeff KingJun 19, 2017
  15. Junio C HamanoJun 19, 2017
  16. Liam BeguinJun 20, 2017
  17. 09/10 t3415: test fixup with wrapped onelineJohannes Schindelin, Jun 14, 2017
  18. 10/10 rebase -i: rearrange fixup/squash lines using the rebase--helperJohannes Schindelin, Jun 14, 2017
  19. Junio C HamanoJun 15, 2017
  20. Johannes SchindelinJun 16, 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.