Re: [PATCH v3 0/7] replacing strbuf_getline_lf() by strbuf_getline()
On 02/28/2016 07:30 AM, Eric Sunshine wrote:
Show 24 quoted lines
> On Sun, Feb 28, 2016 at 12:07 AM, Moritz Neeb <lists@moritzneeb.de> wrote:
>> This series deals with strbuf_getline_lf() in certain codepaths:
>> Those, where the input that is read, is/was trimmed before doing anything that
>> could possibly expect a CR character. Those places can be assumed to be "text"
>> input, where a CR never would be a meaningful control character.
>> [...]
>>
>> Changes since v2:
>>
>> * Line splitting in notes_copy_from_stdin() is changed to string_list_split as
>> suggested by Eric Sunshine.
>> * The behavior change in interactive cleaning from patch v2 is undone.
>> * Some of the previous patches were broken because of some unexpected
>> whitespace. This should be fixed now.
>
> In the future, as an aid to reviewers, please include an interdiff
> since the previous version, as well a link to the previous round[1].
> It's also very helpful to say which patches have changed (and which
> have not).
>
> Thanks.
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/285118/focus=286865
>
Maybe not too late for other reviewers, here comes the interdiff (this assumes the non-broken version 2):
diff --git a/builtin/clean.c b/builtin/clean.c
index 18b6056..5b17a31 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -570,7 +570,9 @@ static int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff)
clean_get_color(CLEAN_COLOR_RESET));
}
- if (strbuf_getline(&choice, stdin) == EOF) {
+ if (strbuf_getline(&choice, stdin) != EOF) {
+ strbuf_trim(&choice);
+ } else {
eof = 1;
break;
}
@@ -650,7 +652,9 @@ static int filter_by_patterns_cmd(void)
clean_print_color(CLEAN_COLOR_PROMPT);
printf(_("Input ignore patterns>> "));
clean_print_color(CLEAN_COLOR_RESET);
- if (strbuf_getline(&confirm, stdin) == EOF)
+ if (strbuf_getline(&confirm, stdin) != EOF)
+ strbuf_trim(&confirm);
+ else
putchar('\n');
/* quit filter_by_pattern mode if press ENTER or Ctrl-D */
@@ -746,7 +750,9 @@ static int ask_each_cmd(void)
qname = quote_path_relative(item->string, NULL, &buf);
/* TRANSLATORS: Make sure to keep [y/N] as is */
printf(_("Remove %s [y/N]? "), qname);
- if (strbuf_getline(&confirm, stdin) == EOF) {
+ if (strbuf_getline(&confirm, stdin) != EOF) {
+ strbuf_trim(&confirm);
+ } else {
putchar('\n');
eof = 1;
}
diff --git a/builtin/notes.c b/builtin/notes.c
index 706ec11..660c0b7 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -292,17 +292,17 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
while (strbuf_getline(&buf, stdin) != EOF) {
unsigned char from_obj[20], to_obj[20];
- struct strbuf **split;
+ struct string_list split = STRING_LIST_INIT_DUP;
int err;
- split = strbuf_split(&buf, ' ');
- if (!split[0] || !split[1])
+ string_list_split(&split, buf.buf, ' ', -1);
+
+ if (split.nr != 2)
die(_("Malformed input line: '%s'."), buf.buf);
- strbuf_rtrim(split[0]);
- if (get_sha1(split[0]->buf, from_obj))
- die(_("Failed to resolve '%s' as a valid ref."), split[0]->buf);
- if (get_sha1(split[1]->buf, to_obj))
- die(_("Failed to resolve '%s' as a valid ref."), split[1]->buf);
+ if (get_sha1(split.items[0].string, from_obj))
+ die(_("Failed to resolve '%s' as a valid ref."), split.items[0].string);
+ if (get_sha1(split.items[1].string, to_obj))
+ die(_("Failed to resolve '%s' as a valid ref."), split.items[1].string);
if (rewrite_cmd)
err = copy_note_for_rewrite(c, from_obj, to_obj);
@@ -312,11 +312,11 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
if (err) {
error(_("Failed to copy notes from '%s' to '%s'"),
- split[0]->buf, split[1]->buf);
+ split.items[0].string, split.items[1].string);
ret = 1;
}
- strbuf_list_free(split);
+ string_list_clear(&split, 0);
}
if (!rewrite_cmd) {