{"thread":{"id":"41290","subject":"Replacing strbuf_getline_lf() by strbuf_getline() in wt-status.c","startedAt":"2016-01-30T14:30:46Z","lastAt":"2016-01-31T16:59:55Z","messageCount":3,"participants":["Moritz Neeb","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"277086","messageId":"56ACC916.8020008@moritzneeb.de","threadId":"41290","inReplyTo":null,"subject":"Replacing strbuf_getline_lf() by strbuf_getline() in wt-status.c","fromName":"Moritz Neeb","fromEmail":"lists@moritzneeb.de","sentAt":"2016-01-30T14:30:46Z","receivedAt":"2016-01-30T14:30:46Z","isPatch":false,"sender":{"key":"lists@moritzneeb.de","avatar":null},"body":"Currently I am working on replacing strbuf_getline_lf() by\nstrbuf_getline() in places where the input is trimmed immediately after\nreading, cf. $gmane/284104, \"Notes on the remaining strbuf_getline_lf()\ncallers\", 2nd point.\n\nOne instance I found was in wt-status.c. In read_rebase_todolist() the\nlines are read, checked for a comment_line_char and then trimmed. I\nwonder why the input is not trimmed before checking for this character?\nIs it safe to replace strbuf_getline_lf() by strbuf_getline() anyway?\n\nThe only case I can imagine that could lead to unexpected behaviour then\nwould be when someone sets the comment_line_char to CR. How likely is that?\n\nWhy is the trim after checking for the comment char anyway? Should\nsomething like \"   # foobar\" not be considered as comment?\n"},{"id":"277100","messageId":"56AD3DE9.2000004@moritzneeb.de","threadId":"41290","inReplyTo":"56ACC916.8020008@moritzneeb.de","subject":"Re: Replacing strbuf_getline_lf() by strbuf_getline() in wt-status.c","fromName":"Moritz Neeb","fromEmail":"lists@moritzneeb.de","sentAt":"2016-01-30T22:49:13Z","receivedAt":"2016-01-30T22:49:13Z","isPatch":false,"sender":{"key":"lists@moritzneeb.de","avatar":null},"body":"On 01/30/2016 03:30 PM, Moritz Neeb wrote:\n> Currently I am working on replacing strbuf_getline_lf() by\n> strbuf_getline() in places where the input is trimmed immediately after\n> reading, cf. $gmane/284104, \"Notes on the remaining strbuf_getline_lf()\n> callers\", 2nd point.\n\nThis thread turned obsolete, as I decided to post a patch to focus the\ndiscussion a bit more:\n\n    http://thread.gmane.org/gmane.comp.version-control.git/285118\n\n-Moritz\n"},{"id":"277127","messageId":"xmqqd1shy99w.fsf@gitster.mtv.corp.google.com","threadId":"41290","inReplyTo":"56ACC916.8020008@moritzneeb.de","subject":"Re: Replacing strbuf_getline_lf() by strbuf_getline() in wt-status.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-31T16:59:55Z","receivedAt":"2016-01-31T16:59:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Moritz Neeb <lists@moritzneeb.de> writes:\n\n> Currently I am working on replacing strbuf_getline_lf() by\n> strbuf_getline() in places where the input is trimmed immediately after\n> reading, cf. $gmane/284104, \"Notes on the remaining strbuf_getline_lf()\n> callers\", 2nd point.\n>\n> One instance I found was in wt-status.c. In read_rebase_todolist() the\n> lines are read, checked for a comment_line_char and then trimmed. I\n> wonder why the input is not trimmed before checking for this character?\n> Is it safe to replace strbuf_getline_lf() by strbuf_getline() anyway?\n>\n> The only case I can imagine that could lead to unexpected behaviour then\n> would be when someone sets the comment_line_char to CR. How likely is that?\n>\n> Why is the trim after checking for the comment char anyway? Should\n> something like \"   # foobar\" not be considered as comment?\n\nThat is debatable, I would think, as \"git commit\" and others do not\ngenerally accept a line that does not begin with a comment char as a\ncomment.\n\nI however think we made an exception for the rebase-i's insn file\ndue to a mistake we made long time ago that allowed such line,\ncaused people to build a workflow around the assumption that it is\nOK to prefix a comment line with whitespaces.\n\n  Cf. the last paragraph of 1db168ee (rebase-i: loosen over-eager\n      check_bad_cmd check, 2015-10-01).\n\n  Cf. http://thread.gmane.org/gmane.comp.version-control.git/278841\n\nSo we probably want to be consistent with that in this codepath.\n"}]}