On Mon, Apr 27, 2020 at 11:09:34AM -0700, Junio C Hamano wrote: > > modified store files which might have empty lines or even comments > > were reported[1] failing to parse as valid credentials. > > These files are not supposed to be viewed or edited without the help > of the credential helpers. Do these blank lines and comments even > survive when a new credential is approved, or do we just overwrite > and lose them? That's a good question. In the older code we do save them, because credential-store passes through lines which don't match the credential we're operating on. But in Carlo's patch, the immediate "continue" means we wouldn't ever call "other_cb", which is what does that pass-through. > I'd rather not to do either, if we did not have to, but if it were > necessary for us to do something, I am OK to ignore empty lines. > But I'd prefer not to mix the new "# comment" feature in, if we did > not have to. > > Also, triming the lines that are not empty is unwarranted. IIUC, > what the "store" action writes encodes whitespaces, so as soon as > you see whitespace on either end, (or anywhere on the line for that > matter), it is a hand-edited cruft in the file. If you ignore > comments, you probably should ignore those lines, too. Yeah, all of that seems quite sensible. -Peff