On 22-06-16 06:18pm, Stefan Sperling wrote:
> On Thu, Jun 16, 2022 at 05:43:24PM +0200, Omar Polo wrote:
> > Mark Jamsek <mark@jamsek.com> wrote:
> > > On 22-06-17 12:59am, Mark Jamsek wrote:
> > > > I habitually use {fossil,git} diff -v to see the diff I'm committing in
> > > > vim when editing the commit message. It saves me all the time from
> > > > missing things (see my previous commits for example).
> > > > 
> > > > Any objections to adding this?
> > > > 
> > > 
> > > s/diff/commit
> > > 
> > > got commit -v
> > 
> > I didn't know about `fossil/git diff -v', but in general I'd like the
> > idea.
> 
> I am a bit afraid that such a feature could lead to people accidentally
> committing pf.c as their log message. The Git documentation claims that
> the diff won't be part of the committed log message, but how can we filter
> the diff out of the file again with 100% accuracy?
> I don't think there is a reliable way. Once the user has had access to the
> file and gives it back to us, we have no idea what the file contents mean
> and we cannot assume that any meta-data we have about the log message is
> still valid.

You're right, we can't control what happens in the editor. If the user
decides to delete whatever string we use to demarcate log message from
diff, the diff will end up in the message. In that sense, I guess it's
the same as deleting the leading '#' for the commit changeset that is
currently displayed; but with greater consequences. That said, we added
this to Fossil about 18 months ago, and we haven't had a reported
incident yet. (Now I've said that, I bet we get one :) The only
protection we have is a hash line, and everything after that is ignored.

##############################################################################
# The following diff is excluded from the commit message:

...

> > what i'd really like, even more than putting the diff in the log edit
> > buffer with all the consequences it has (i.e. that special line to
> > differentiate between the log message and the diff), would be for `got
> > diff' to work while `got commit' is waiting for the editor.
> > 
> > (blatantly a feature request since I haven't looked into how hard would
> > be and which consequences it would have.)
> 
> The locks we use for the work tree are very simple exclusive locks.
> As soon as a lock exists, everyone else is locked out. flock(2) does
> not support a concurrent multi-reader single-writer model.
> 
> And there is no separate staging area like in Git. Staged changes just
> add fileindex entries which point at blobs already written to the repo.
> 
> If we don't lock the work tree during commit we are effectively running
> a transaction without protecting the integrity of transaction state.
> Which could lead to file index corruption or unexpected changes creeping
> into commits (e.g. a 'got add' command could modify the work tree while
> the log message is being written).
> 
> Unlocking earlier than we do now is not an option because the work tree
> needs to remain locked until we have updated its base commit ID to that
> of the newly created commit. Which we obviously won't know until the
> very end of the commit process.

This is precisely why I implemented this in Fossil: the checkout db is
locked and you can't run 'fossil diff' in this state.  I don't think
trying to circumvent the locks is the answer either.

> > to be honest thought I got used to open an xterm with `got di | less' in
> > it before doing a commit from the command line.
> 
> This is what I do as well. Or save the diff to a file, and load it
> into a separate editor buffer to view log message and diff side-by-side.

This is what I'll get in the habit of doing too.

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68