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

Re: [PATCH v4 06/10] format-patch: explicitly switch off color when writing to files

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jun 26, 2016, 06:49 UTC
Message-ID
<alpine.DEB.2.20.1606260847590.12947@virtualbox>
In-Reply-To
<xmqq1t3mfdpy.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Fri, 24 Jun 2016, Junio C Hamano wrote:
Show 27 quoted lines
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > We rely on the auto-detection ("is stdout a terminal?") to determine
> > whether to use color in the output of format-patch or not. That
> > happens to work because we freopen() stdout when redirecting the
> > output to files.
> >
> > However, we are about to fix that work-around, in which case the
> > auto-detection has no chance to guess whether to use color or not.
> >
> > But then, we do not need to guess to begin with. As argued in the
> > commit message of 7787570c (format-patch: ignore ui.color,
> > 2011-09-13), we do not allow the ui.color setting to affect
> > format-patch's output. The only time, therefore, that we allow color
> > sequences to be written to the output files is when the user specified
> > the --color command-line option explicitly.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> 
> The right fix in the longer term (long after this series lands, that
> is) is probably to update the world view that the codepath from
> want_color_auto() to check_auto_color() has always held.  In their
> world view, when they are asked to make --color=auto decision, the
> output always goes the standard output, and that is why they
> hardcode isatty(1) to decide.  The existing freopen() was a part of
> that world view.

I agree, and I briefly looked into this. It looks a bit more involved than I am prepared for, what with my real goal being the rebase--helper, not clean-ups ;-)

> We'd need a workaround like this patch if we want to leave the
> want_color_auto() as-is, and as a workaround I think this is the
> least invasive one, so let's queue it as-is.

Thanks! Dscho

Previous: Junio C HamanoNext: Johannes Schindelin
Message 62 of 67 in “Let log-tree and friends respect diffopt's `file` field”
  1. 0/5 Let log-tree and friends respect diffopt's `file` fieldJohannes Schindelin, Jun 18, 2016
  2. 1/5 log-tree: respect diffopt's configured output file streamJohannes Schindelin, Jun 18, 2016
  3. 3/5 graph: respect the diffopt.file settingJohannes Schindelin, Jun 18, 2016
  4. 4/5 shortlog: support outputting to streams other than stdoutJohannes Schindelin, Jun 18, 2016
  5. 5/5 format-patch: avoid freopen()Johannes Schindelin, Jun 18, 2016
  6. Eric SunshineJun 19, 2016
  7. Johannes SchindelinJun 20, 2016
  8. Eric SunshineJun 20, 2016
  9. Johannes SchindelinJun 20, 2016
  10. Junio C HamanoJun 20, 2016
  11. Johannes SchindelinJun 21, 2016
  12. Junio C HamanoJun 21, 2016
  13. Johannes SchindelinJun 22, 2016
  14. Junio C HamanoJun 22, 2016
  15. Johannes SchindelinJun 22, 2016
  16. Junio C HamanoJun 22, 2016
  17. Junio C HamanoJun 22, 2016
  18. 2/5 line-log: respect diffopt's configured output file streamJohannes Schindelin, Jun 18, 2016
  19. 0/7 Let log-tree and friends respect diffopt's `file` fieldJohannes Schindelin, Jun 20, 2016
  20. 2/7 line-log: respect diffopt's configured output file streamJohannes Schindelin, Jun 20, 2016
  21. 1/7 log-tree: respect diffopt's configured output file streamJohannes Schindelin, Jun 20, 2016
  22. Junio C HamanoJun 20, 2016
  23. Johannes SchindelinJun 21, 2016
  24. Johannes SchindelinJun 21, 2016
  25. Johannes SchindelinJun 21, 2016
  26. 3/7 graph: respect the diffopt.file settingJohannes Schindelin, Jun 20, 2016
  27. 6/7 format-patch: avoid freopen()Johannes Schindelin, Jun 20, 2016
  28. 5/7 format-patch: explicitly switch off color when writing to filesJohannes Schindelin, Jun 20, 2016
  29. 4/7 shortlog: support outputting to streams other than stdoutJohannes Schindelin, Jun 20, 2016
  30. 7/7 format-patch: use stdout directlyJohannes Schindelin, Jun 20, 2016
  31. Junio C HamanoJun 20, 2016
  32. Johannes SchindelinJun 20, 2016
  33. 0/9 Let log-tree and friends respect diffopt's `file` fieldJohannes Schindelin, Jun 21, 2016
  34. 8/9 format-patch: avoid freopen()Johannes Schindelin, Jun 21, 2016
  35. 7/9 format-patch: explicitly switch off color when writing to filesJohannes Schindelin, Jun 21, 2016
  36. 9/9 format-patch: use stdout directlyJohannes Schindelin, Jun 21, 2016
  37. 2/9 Disallow diffopt.close_file when using the log_tree machineryJohannes Schindelin, Jun 21, 2016
  38. Junio C HamanoJun 21, 2016
  39. Junio C HamanoJun 21, 2016
  40. Junio C HamanoJun 21, 2016
  41. Johannes SchindelinJun 22, 2016
  42. 4/9 line-log: respect diffopt's configured output file streamJohannes Schindelin, Jun 21, 2016
  43. 3/9 log-tree: respect diffopt's configured output file streamJohannes Schindelin, Jun 21, 2016
  44. 1/9 am: stop ignoring errors reported by log_tree_diff()Johannes Schindelin, Jun 21, 2016
  45. Junio C HamanoJun 21, 2016
  46. Johannes SchindelinJun 22, 2016
  47. 6/9 shortlog: support outputting to streams other than stdoutJohannes Schindelin, Jun 21, 2016
  48. 5/9 graph: respect the diffopt.file settingJohannes Schindelin, Jun 21, 2016
  49. Paul TanJun 21, 2016
  50. Johannes SchindelinJun 21, 2016
  51. Paul TanJun 22, 2016
  52. 00/10 Let log-tree and friends respect diffopt's `file` fieldJohannes Schindelin, Jun 22, 2016
  53. 09/10 shortlog: respect the --output=<file> settingJohannes Schindelin, Jun 22, 2016
  54. 10/10 Ensure that log respects --output=<file>Johannes Schindelin, Jun 22, 2016
  55. 07/10 format-patch: avoid freopen()Johannes Schindelin, Jun 22, 2016
  56. 08/10 format-patch: use stdout directlyJohannes Schindelin, Jun 22, 2016
  57. Junio C HamanoJun 24, 2016
  58. 05/10 shortlog: support outputting to streams other than stdoutJohannes Schindelin, Jun 22, 2016
  59. 02/10 log-tree: respect diffopt's configured output file streamJohannes Schindelin, Jun 22, 2016
  60. 06/10 format-patch: explicitly switch off color when writing to filesJohannes Schindelin, Jun 22, 2016
  61. Junio C HamanoJun 24, 2016
  62. Johannes SchindelinJun 26, 2016
  63. 01/10 Prepare log/log-tree to reuse the diffopt.close_file attributeJohannes Schindelin, Jun 22, 2016
  64. Junio C HamanoJun 24, 2016
  65. Johannes SchindelinJun 26, 2016
  66. 04/10 graph: respect the diffopt.file settingJohannes Schindelin, Jun 22, 2016
  67. 03/10 line-log: respect diffopt's configured output file streamJohannes Schindelin, Jun 22, 2016

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.