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

Re: [PATCH v4 01/10] Prepare log/log-tree to reuse the diffopt.close_file attribute

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 24, 2016, 20:56 UTC
Message-ID
<xmqq60syfgqf.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<044f3c6359b63eaa18229cd73e6815e8e15c9778.1466607667.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 12 quoted lines
> We are about to teach the log-tree machinery to reuse the diffopt.file
> field to output to a file stream other than stdout, in line with the
> diff machinery already writing to diffopt.file.
>
> However, we might want to write something after the diff in
> log_tree_commit() (e.g. with the --show-linear-break option), therefore
> we must not let the diff machinery close the file (as per
> diffopt.close_file.
>
> This means that log_tree_commit() itself must override the
> diffopt.close_file flag and close the file, and if log_tree_commit() is
> called in a loop, the caller is responsible to do the same.
Makes sense.
Show 23 quoted lines
> Note: format-patch has an `--output-directory` option. Due to the fact
> that format-patch's options are parsed first, and that the parse-options
> machinery accepts uniquely abbreviated options, the diff options
> `--output` (and `-o`) are shadowed. Therefore close_file is not set to 1
> so that cmd_format_patch() does *not* need to handle the close_file flag
> differently, even if it calls log_tree_commit() in a loop.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/log.c | 15 ++++++++++++---
>  log-tree.c    |  5 ++++-
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 099f4f7..27bc88d 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -243,9 +243,10 @@ static struct itimerval early_output_timer;
>  
>  static void log_show_early(struct rev_info *revs, struct commit_list *list)
>  {
> -	int i = revs->early_output;
> +	int i = revs->early_output, close_file = revs->diffopt.close_file;

Probably not worth a reroll, but I find this kind of thing easier to read as two separate definitions.

>  	int show_header = 1;

And this was separate from "int i = ...;" for the same reason. It is OK to write "int i, j, k;" but "show_header" is something that keeps track of the more important state during the control flow and it is easier to read if we make it stand out. close_file falls into the same category, I would think.

>  		case commit_error:
> +			if (close_file)
> +				fclose(revs->diffopt.file);

I wondered if we want to also clear, i.e. revs->diffopt.file = NULL, but I do not think .close_file does that either, so this is good.

Thanks.
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 64 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.