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

Re: [PATCH] am: terminate state files with a newline

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 24, 2015, 06:48 UTC
Message-ID
<xmqqegit5ghf.fsf@gitster.dls.corp.google.com>
In-Reply-To
<20150824051344.GA12490@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 22 quoted lines
> FWIW, I had a similar thought when reading the original thread. I also
> noted that all of the callers here pass "1" for the "fatal" parameter,
> and that they are either bools or single strings. I wonder if:
>
>   void write_state_bool(struct am_state *state, const char *name, int v)
>   {
> 	write_file(am_path(state, name), 1, "%s\n", v ? "t" : "f");
>   }
>
> would make the call-sites even easier to read (and of course the "\n"
> would be dropped here if it does migrate up to write_file()).
>
>> @@ -634,6 +641,9 @@ int write_file(const char *path, int fatal, const char *fmt, ...)
>>  	va_start(params, fmt);
>>  	strbuf_vaddf(&sb, fmt, params);
>>  	va_end(params);
>> +	if (sb.len)
>> +		strbuf_complete_line(&sb);
>> +
>
> I think the "if" here is redundant; strbuf_complete_line already handles
> it.

True. And I like your write_state_bool() wrapper (which should be "static void" to the builtin/am.c) very much.

On top of that, I think the right thing to do to write_file() would be to first clean-up the second parameter "fatal" to an "unsigned flags" whose (1<<0) bit is "fatal", (1<<1) bit is "binary", and make this new call to "strbuf_complete_line()" only when "binary" bit is not set.

The new comment I added before write_file() function needs to be adjusted if we were to do this, obviously.

Previous: Jeff KingNext: Jeff King
Message 7 of 36 in “Minor builtin 'git am' side-effect”
  1. SZEDER GáborAug 20, 2015
  2. Junio C HamanoAug 20, 2015
  3. am: terminate state files with a newlinePaul Tan, Aug 23, 2015
  4. SZEDER GáborAug 23, 2015
  5. Junio C HamanoAug 23, 2015
  6. Jeff KingAug 24, 2015
  7. Junio C HamanoAug 24, 2015
  8. Jeff KingAug 24, 2015
  9. 0/5 "am" state file fix with write_file() clean-upJunio C Hamano, Aug 24, 2015
  10. 1/5 builtin/am: introduce write_state_*() helper functionsJunio C Hamano, Aug 24, 2015
  11. 2/5 builtin/am: make sure state files are textJunio C Hamano, Aug 24, 2015
  12. 3/5 write_file(): introduce an explicit WRITE_FILE_GENTLY requestJunio C Hamano, Aug 24, 2015
  13. Junio C HamanoAug 24, 2015
  14. Duy NguyenAug 25, 2015
  15. setup: update the right file in multiple checkoutsNguyễn Thái Ngọc Duy, Aug 25, 2015
  16. Junio C HamanoAug 25, 2015
  17. Duy NguyenAug 31, 2015
  18. 4/5 write_file(): do not leave incomplete line at the endJunio C Hamano, Aug 24, 2015
  19. 5/5 write_file(): clean up transitional mess of flag words and terminating LFJunio C Hamano, Aug 24, 2015
  20. Jeff KingAug 24, 2015
  21. Junio C HamanoAug 24, 2015
  22. Jeff KingAug 24, 2015
  23. Junio C HamanoAug 24, 2015
  24. 0/6 "am" state file fix with write_file() clean-upJunio C Hamano, Aug 24, 2015
  25. 1/6 builtin/am: introduce write_state_*() helper functionsJunio C Hamano, Aug 24, 2015
  26. 2/6 builtin/am: make sure state files are textJunio C Hamano, Aug 24, 2015
  27. Jeff KingAug 24, 2015
  28. Junio C HamanoAug 25, 2015
  29. Jeff KingAug 25, 2015
  30. Junio C HamanoAug 25, 2015
  31. 3/6 write_file(): drop "fatal" parameterJunio C Hamano, Aug 24, 2015
  32. 4/6 write_file_v(): do not leave incomplete line at the endJunio C Hamano, Aug 24, 2015
  33. 5/6 write_file(): drop caller-supplied LF from calls to create a one-liner fileJunio C Hamano, Aug 24, 2015
  34. 6/6 write_file(): drop caller-supplied LF from multi-line fileJunio C Hamano, Aug 24, 2015
  35. Jeff KingAug 25, 2015
  36. brian m. carlsonAug 24, 2015

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.