On 4/10/2020 7:27 AM, Johannes Schindelin via GitGitGadget wrote: > From: Johannes Schindelin <johannes.schindelin@xxxxxx> > > There are quite a few code locations (e.g. `git clean --interactive`) > where Git asks the user for an answer. In preparation for fixing a bug > shared by all of them, and also to DRY up the code, let's refactor it. > > Please note that most of these callers trimmed white-space both at the > beginning and at the end of the answer, instead of trimming only the > end (as the caller in `add-patch.c` does). add-patch also only trims the newline! This is still a good change. > THerefore, technically speaking, we change behavior in this patch. At Strange capitalization here. > the same time, it can be argued that this is actually a bug fix. > > @@ -1158,9 +1159,8 @@ static int read_single_character(struct add_p_state *s) > return res; > } > > - if (strbuf_getline(&s->answer, stdin) == EOF) > + if (git_read_line_interactively(&s->answer) == EOF) > return EOF; > - strbuf_trim_trailing_newline(&s->answer); (Pointing out the trailing newline trim here.) > + > +int git_read_line_interactively(struct strbuf *line) > +{ > + int ret = strbuf_getline_lf(line, stdin); > + > + if (ret != EOF) > + strbuf_trim_trailing_newline(line); > + > + return ret; > +} This looks good. Do we need a trailing newline or something? The way the diff ends abruptly after the "}" line made me think so. Thanks, -Stolee