From: Omar Polo Subject: Re: abort commit if merged log messages are left as-is To: Mark Jamsek Cc: gameoftrees@openbsd.org Date: Thu, 09 Feb 2023 15:20:22 +0100 On 2023/02/10 00:23:48 +1100, Mark Jamsek wrote: > On 23-02-09 01:59PM, Stefan Sperling wrote: > > On Tue, Feb 07, 2023 at 04:56:42PM +1100, Mark Jamsek wrote: > > > @@ -486,7 +486,7 @@ edit_logmsg(char **logmsg, const char *editor, const c > > > "commit message cannot be empty, aborting"); > > > goto done; > > > } > > > - if (require_modification && > > > + if (require_modification && logmsg_len >= initial_content_len && > > > strcmp(*logmsg, initial_content_stripped) == 0) > > > err = got_error_msg(GOT_ERR_COMMIT_MSG_EMPTY, > > > "no changes made to commit message, aborting"); > > > > This approach is a little too suttle for my taste. For someone > > looking at this code years from now this will not be very obvious. > > Tbh, I wasn't too sure if I'd understood the desired behaviour so > I really wanted to see if I was on the right track; but you're right, it > should at the very least be commented else I'd have no idea why that > condition was there even a month from now--let alone a year :) > > That said, I'm glad to see I did indeed get the behaviour right with > a one liner! > > > I would suggest the approach implemente by the patch below. > > Granted, this patch adds extra work. But log message should not be huge > > in the first place, so this is still cheap. > > Yes, it provides the same behaviour, matching what I think op was > after, and also reads good to me. Thanks! ok for me works like a charm, ok for me too, thanks!