On 2023/02/10 00:23:48 +1100, Mark Jamsek <mark@jamsek.com> 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!