From: Rubén Justo Date: Thu, 28 Mar 2024 00:39:12 GMT Subject: Re: [PATCH v2 2/2] add-patch: do not print hunks repeatedly Message-ID: <5abeffbe-892a-468f-8d2c-40016662b0d9@gmail.com> In-Reply-To: On Wed, Mar 27, 2024 at 11:06:49AM +0000, Phillip Wood wrote: > > (1/4) Stage this hunk [y,n,q,a,d,j,J,g,/,e,p,?]? U > > Unknown option "U". Use '?' for help. > > Yes, I like that (though I'd use the same quotes for both parts of the > message) Yes, you're right. Using the same quotes are the correct thing to do. I don't know how I thought we should print the result of git_read_line_interactively(). After thinking about it again, I see that would be misleading, to say the least. > > If find having two strbuf_reset()'s in a row confusing. Maybe it is > > just me not seeing that that second strbuf_reset is "close" to noop. > > If we don't print the hunk then the second call to strbuf_reset is indeed a > noop. In our code base it is common to see a call to strbuf_reset() > immediately before adding new content to the buffer, rather than cleaning up > ready for reuse after the buffer has been used. If you grep 'strbuf_reset' > in this file you'll see all the calls come immediately before adding new > content to the buffer. By moving the call inside the conditional we're > moving from a pattern of cleaning up before adding new content to a pattern > of cleaning up afterwards which I think is harder to follow given the way > the rest of the code uses strbuf_reset() I have no strong objection. I'll reroll leaving that strbuf_reset untouched.