Prefer fgetc over fgets where possible

Commit Message

Seija Kijin July 17, 2023, 3:38 p.m. UTC

From: Seija Kijin <[e-mail]>

fputc is meant for single characters,
fputs is for strings. We are better off
inserting sole \n characters as
characters, not whole strings.

Signed-off-by: Seija Kijin [e-mail]
---
    Prefer fgetc over fgets where possible
    
    fputc is meant for single characters, fputs is for strings. We are
    better off inserting sole \n characters as characters, not whole
    strings.
    
    Signed-off-by: Seija Kijin [e-mail]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1550%2FAtariDreams%2Ffgetc-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1550/AtariDreams/fgetc-v1
Pull-Request: https://github.com/git/git/pull/1550

 wt-status.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: 830b4a04c45bf0a6db26defe02ed1f490acd18ee

Comments

Junio C Hamano July 17, 2023, 4:51 p.m. UTC | #1

"AtariDreams via GitGitGadget" <[e-mail]> writes:

> From: Seija Kijin <[e-mail]>
>
> fputc is meant for single characters,
> fputs is for strings. We are better off
> inserting sole \n characters as
> characters, not whole strings.

I do not see if these short lines are deliberate; are they meant to
follow some sort of poetry styles?

In any case, while the above is correct, I do not see the patch
noise is worth it in this particular case.  Yes, if we are writing
code snippets shown with the context in these hunks afresh, please
carefully choose between fputs() and fputc().  But once the code is
written and it is in, it is not worth to go back and fix it, unless
we are fixing surrounding area and doing the clean-up as a "while at
it" change.

By the way, because my mail program warned against an address that
apparently refuses to receive any replies, I had to manually remove
"AtariDreams <[e-mail]>" while
composing this message.  I'd appreciate it if you arrange to ensure
that your next patch will not have such addresses on your CC: line.

Thanks.

Taylor Blau July 17, 2023, 8:14 p.m. UTC | #2

On Mon, Jul 17, 2023 at 03:38:11PM +0000, AtariDreams via GitGitGadget wrote:
> ---
>     Prefer fgetc over fgets where possible
>
>     fputc is meant for single characters, fputs is for strings. We are
>     better off inserting sole \n characters as characters, not whole
>     strings.
>
>     Signed-off-by: Seija Kijin [e-mail]

I tend to agree with Junio's assessment that adding churn here isn't
strictly necessary / worth it, but if we do end up taking this patch,
note that the subject should reference `fputc()` and `fputs()` instead
of `fgetc()` and `fgets()`.

Taylor

Patch

diff --git a/wt-status.c b/wt-status.c
index 8a1a4fb1f04..4c267395cdf 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1215,7 +1215,7 @@  static void wt_longstatus_print_tracking(struct wt_status *s)
 		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c",
 				 comment_line_char);
 	else
-		fputs("\n", s->fp);
+		fputc('\n', s->fp);
 	strbuf_release(&sb);
 }
 
@@ -1814,7 +1814,7 @@  static void wt_longstatus_print_state(struct wt_status *s)
 	if (state->merge_in_progress) {
 		if (state->rebase_interactive_in_progress) {
 			show_rebase_information(s, state_color);
-			fputs("\n", s->fp);
+			fputc('\n', s->fp);
 		}
 		show_merge_in_progress(s, state_color);
 	} else if (state->am_in_progress)