On Mon, Feb 29, 2016 at 3:36 AM, Moritz Neeb <lists@xxxxxxxxxxxxx> wrote:
> The format of a line that is expected when copying notes via stdin
> is "sha1 sha1". As this is text-only, strbuf_getline() should be used
> instead of strbuf_getline_lf(), as documentation of this fact.
>
> When reading with strbuf_getline() the trimming of split[1] can be
> removed. It was necessary before to remove potential CRs inserted
> through a dos editor.
s/dos/DOS/
This may not be worth a re-roll, but the suggestion of my v3 review
was to keep both rtrim's in this patch and then remove them in the
next patch when converting to string_list_split(). A benefit of doing
so is that you can then drop the above paragraph altogether, and both
patches become simpler (description and content), thus easier to
review.
If you do elect to keep things the way they are, then (as mentioned in
my v2 review) it would be helpful for the above paragraph to explain
that strbuf_split() leave the "terminator" on the split elements, thus
clarifying why the rtrim() of split[0] is still needed.
> Signed-off-by: Moritz Neeb <lists@xxxxxxxxxxxxx>
> ---
> builtin/notes.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index ed6f222..706ec11 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -290,7 +290,7 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
> t = &default_notes_tree;
> }
>
> - while (strbuf_getline_lf(&buf, stdin) != EOF) {
> + while (strbuf_getline(&buf, stdin) != EOF) {
> unsigned char from_obj[20], to_obj[20];
> struct strbuf **split;
> int err;
> @@ -299,7 +299,6 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
> if (!split[0] || !split[1])
> die(_("Malformed input line: '%s'."), buf.buf);
> strbuf_rtrim(split[0]);
> - strbuf_rtrim(split[1]);
> 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))
> --
> 2.4.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html