Prefer fgetc over fgets where possible

From: Seija Kijin <[email protected]>

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

---
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.

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(-)

index 8a1a4fb1f04..4c267395cdf 100644
@@ -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)

base-commit: 830b4a04c45bf0a6db26defe02ed1f490acd18ee

gitgitgadget

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 <[email protected]>" 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.

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

From: Seija Kijin <[email protected]>

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

---
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.

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

Range-diff vs v1:

1: f833a7cc857 ! 1: 26f329befbd Prefer fgetc over fgets where possible
@@ Commit message

+ ## bisect.c ##
+@@ bisect.c: static void show_list(const char *debug, int counted, int nr,
+ subject_len = find_commit_subject(buf, &subject_start);
+ if (subject_len)
+ fprintf(stderr, " %.*s", subject_len, subject_start);
+- fprintf(stderr, "\n");
++ fputc('\n', stderr);
+ }
+ }
+
+
+ ## commit-graph.c ##
+@@ commit-graph.c: static void graph_report(const char *fmt, ...)
+ verify_commit_graph_error = 1;
+ va_start(ap, fmt);
+ vfprintf(stderr, fmt, ap);
+- fprintf(stderr, "\n");
++ fputc('\n', stderr);
+ va_end(ap);
+ }
+
+
## wt-status.c ##
@@ wt-status.c: static void wt_longstatus_print_tracking(struct wt_status *s)
- color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%c",
- comment_line_char);
+ color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%s",
+ comment_line_str);
else
- fputs("\n", s->fp);
+ fputc('\n', s->fp);

 bisect.c       | 2 +-
 commit-graph.c | 2 +-
 wt-status.c    | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

index d71c4e4b44b..3b1e034013f 100644
@@ -179,7 +179,7 @@ static void show_list(const char *debug, int counted, int nr,
 		subject_len = find_commit_subject(buf, &subject_start);
 		if (subject_len)
 			fprintf(stderr, " %.*s", subject_len, subject_start);
-		fprintf(stderr, "\n");
+		fputc('\n', stderr);
 	}
 }
 

index e2e2083951c..b649598916a 100644
@@ -2696,7 +2696,7 @@ static void graph_report(const char *fmt, ...)
 	verify_commit_graph_error = 1;
 	va_start(ap, fmt);
 	vfprintf(stderr, fmt, ap);
-	fprintf(stderr, "\n");
+	fputc('\n', stderr);
 	va_end(ap);
 }
 

index 6a8c05d1cff..75b8e900a4c 100644
@@ -1227,7 +1227,7 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 		color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "%s",
 				 comment_line_str);
 	else
-		fputs("\n", s->fp);
+		fputc('\n', s->fp);
 	strbuf_release(&sb);
 }
 

@@ -1832,7 +1832,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)

base-commit: 063bcebf0c917140ca0e705cbe0fdea127e90086

gitgitgadget