Magit ate my homework!

53 views
Skip to first unread message

Eli Barzilay

unread,
Jan 19, 2012, 2:51:29 PM1/19/12
to ma...@googlegroups.com
I've lost a bunch of little bits of work here and there in the past,
and now that I lost a more substantial bit I finally sat down to find
the guilty party -- which turned out to be magit.

It's probably best to explain what I do:

* Open some file, work on it.

* Do a pull to get my repo updated, find that the git doesn't like
the file with the edits there.

* Run a git checkout on the file, re-run the pull.

* rm the file, save the buffer that still holds my edited version.

All of that works perfectly fine, if the git work is done on a shell.
[You might argue about the habit of depending on an Emacs buffer to
hold onto some work (and I'll counter with the fact that my machine
crashes less frequently than your HD) but that's besides the point.]

What I just found is that just hittin `g' in the magit status buffer
will revert open buffers in that repo! This is an **extremely** bad
idea, IMO. It's kind of nice in some cases to have magit keep a
buffer updated (eg, when I undo a certain part), but just reverting
the whole thing on an operation like "refresh" on a status buffer
should *not* touch any of my buffers. Also, looking at the code, it
seems that `magit-revert-buffers' is responsible for it, and there is
no option to turn it off.

1. This should be turned off when refreshing the status view, and
possibly other such file-passive magit actions. This is in
contrast to something like `magit-reverse-item' where I *want* a
file to change, and therefore it's often fine to assume that I want
my buffer to go with it.

2. Still, there should be an option to turn it off completely, so that
even such things as revert won't touch my buffers. I personally
would set such an option on, but I'm guessing that many people are
fine with that. (I have a hack that makes reverting a buffer much
more easy and useful than the default Emacs behavior.)

3. Finally, when a buffer is reverted, it is dangerous to just use
`revert-buffer' as is, since it also discards the undo
information. It is possible to save this before reverting the
buffer, and then restore it afterwards so that the revert operation
is itself undo-able. (That's the hack that I referred to in the
last point.) But this might be an overkill, given that people
usually expect that. Having that option (from the previous item)
means that I explicitly want Emacs to deal with file changes as
usual when files change under it.

--
((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:
http://barzilay.org/ Maze is Life!

Andrea Crotti

unread,
Jan 19, 2012, 3:38:43 PM1/19/12
to Eli Barzilay, ma...@googlegroups.com
On 01/19/2012 07:51 PM, Eli Barzilay wrote:
> I've lost a bunch of little bits of work here and there in the past,
> and now that I lost a more substantial bit I finally sat down to find
> the guilty party -- which turned out to be magit.
>
> It's probably best to explain what I do:
>
> * Open some file, work on it.
>
> * Do a pull to get my repo updated, find that the git doesn't like
> the file with the edits there.
>
> * Run a git checkout on the file, re-run the pull.
>
> * rm the file, save the buffer that still holds my edited version.
>

Yes I agree with you, this should not be the default behaviour.
But how is it possible that it just reverted the buffers (which were
modified)
without even asking you for confirmation?
Are you sure you have default settings about that?
In particular revert-without-query is by default nil on my machine..

And in general I never followed this workflow, for me it's much easier
to modify
directly the conflicted file, fixing the conflicts by hand, and I never
had a problem
with that...

Eli Barzilay

unread,
Feb 3, 2012, 10:13:05 AM2/3/12
to Andrea Crotti, ma...@googlegroups.com
About two weeks ago, Andrea Crotti wrote:
> On 01/19/2012 07:51 PM, Eli Barzilay wrote:
> > I've lost a bunch of little bits of work here and there in the
> > past, and now that I lost a more substantial bit I finally sat
> > down to find the guilty party -- which turned out to be magit.
> >
> > It's probably best to explain what I do:
> >
> > * Open some file, work on it.
> >
> > * Do a pull to get my repo updated, find that the git doesn't
> > like the file with the edits there.
> >
> > * Run a git checkout on the file, re-run the pull.
> >
> > * rm the file, save the buffer that still holds my edited
> > version.
>
> Yes I agree with you, this should not be the default behaviour. But
> how is it possible that it just reverted the buffers (which were
> modified) without even asking you for confirmation? Are you sure
> you have default settings about that? In particular
> revert-without-query is by default nil on my machine..

I should have clarifiedL when I say "work on it" I mean that it's also
saved. After I see the problem in updating, I just re-checkout the
file on disk, and Emacs thinks that the file is unmodified. If at
this point I'll touch the file again, it will obviously notice that it
was changed on disk and offer to revert it. It asks about it, since
there could be important stuff that will get lost -- which is exactly
the right thing for me.

What magit does, is that revert operation, but it doesn't ask about
it, and thefore I do lose that work, and until that incident, without
even knowing about it. (I later realized that this was the reason for
a few times I pushed code out and later wondered how come I forgot
some change.)

I think that magit should leave the buffers as is, because any layer
of convenience in keeping files up-to-date should be left for some
lower-level code. In this case, the code is already there, and it's
already doing the right thing (ask when the buffer is touched).
Another example where not doing the clever thing that it tries to do
now pays of: I override that ask-about-revert thing, and make it auto
revert after a ding to alert me, and making sure that I can undo the
revert operation so nothing gets lost. Because magit updates the
files behind my back, all that code is made irrelevant.

Moritz Bunkus

unread,
Feb 3, 2012, 10:18:37 AM2/3/12
to Magit
Hey,

On Fri, Feb 3, 2012 at 16:13, Eli Barzilay <e...@barzilay.org> wrote:

> I think that magit should leave the buffers as is

I disagree. I like the current behavior and rely on the buffers being
up to date with the working tree after having checkout out a revision.
So if this is changed then it should be configurable (e.g. "revert
without asking", "revert with asking", "don't revert at all").

m.

PJ Weisberg

unread,
Feb 3, 2012, 10:37:11 AM2/3/12
to Moritz Bunkus, Magit
This I can agree with.  Make "asking" the default, and have a "never ask again" option, to keep all the convenience without the potential for surprising data loss.

--

-PJ

Gehm's Corrollary to Clark's Law: Any technology distinguishable from
magic is insufficiently advanced.

Eli Barzilay

unread,
Feb 3, 2012, 10:50:20 AM2/3/12
to PJ Weisberg, Moritz Bunkus, Magit
20 minutes ago, Moritz Bunkus wrote:
> Hey,
>
> On Fri, Feb 3, 2012 at 16:13, Eli Barzilay <e...@barzilay.org> wrote:
>
> > I think that magit should leave the buffers as is
>
> I disagree. I like the current behavior and rely on the buffers
> being up to date with the working tree after having checkout out a
> revision. So if this is changed then it should be configurable
> (e.g. "revert without asking", "revert with asking", "don't revert
> at all").

Obviously, the current behavior should be available as an option. I
think that it's also obvious that there *should* be an option.

The subjective point is what should the default be. IMO, having the
revert-without-asking thing is dangerous enough that it shouldn't be
the default (but for myself I'd be happy regardless to have an
option).


Just now, PJ Weisberg wrote:
>
> This I can agree with.  Make "asking" the default, and have a "never
> ask again" option, to keep all the convenience without the potential
> for surprising data loss.

That's a good way to have neither as a default, but there are two
problems with it:

1. How do you implement the "never ask again"? Editing .emacs files
to add options seems to be very out of fashion. (Maybe just invoke
the custom functionality?)

2. Also, this means that the "asking" that you're talking about is
going to be magit code -- not the asking that gets done by Emacs
when a buffer's file is modified. This is probably not too much of
a technical problem, but it might be confusing.

Either way, I'd be happy to write a patch that adds the option, and I
can try to do the asking part too if you have an idea about #1 above.

Eli Barzilay

unread,
Feb 3, 2012, 11:05:50 AM2/3/12
to PJ Weisberg, Moritz Bunkus, Magit
(I forgot another footnote comment: another reason why I don't like it
as a default is that it's incomplete in that it's doing one easy case.
A complete thing would also automatically refresh the status buffer
when files change, and probably a bunch of similar things too.)

Moritz Bunkus

unread,
Feb 3, 2012, 11:08:51 AM2/3/12
to Eli Barzilay, PJ Weisberg, Magit
Hey,

On Fri, Feb 3, 2012 at 17:05, Eli Barzilay <e...@barzilay.org> wrote:

> A complete thing would also automatically refresh the status buffer
> when files change,

Uhm... please no. Refreshing already takes several seconds with larger
repositories (e.g. a few thousand files over NFS).

Kind regards,
m.

Eli Barzilay

unread,
Feb 3, 2012, 11:16:11 AM2/3/12
to Moritz Bunkus, PJ Weisberg, Magit

(I wasn't suggesting that it would do that -- just explaining why it's
partial. The fact that doing this kind of refreshing is too long is a
good explanation why it will probably always be partial in this
sense...)

Pieter Praet

unread,
Feb 3, 2012, 1:35:23 PM2/3/12
to Eli Barzilay, PJ Weisberg, Moritz Bunkus, Magit
On Fri, 3 Feb 2012 10:50:20 -0500, Eli Barzilay <e...@barzilay.org> wrote:
> 20 minutes ago, Moritz Bunkus wrote:
> > Hey,
> >
> > On Fri, Feb 3, 2012 at 16:13, Eli Barzilay <e...@barzilay.org> wrote:
> >
> > > I think that magit should leave the buffers as is
> >
> > I disagree. I like the current behavior and rely on the buffers
> > being up to date with the working tree after having checkout out a
> > revision. So if this is changed then it should be configurable
> > (e.g. "revert without asking", "revert with asking", "don't revert
> > at all").
>

Same here.

> Obviously, the current behavior should be available as an option. I
> think that it's also obvious that there *should* be an option.
>
> The subjective point is what should the default be. IMO, having the
> revert-without-asking thing is dangerous enough that it shouldn't be
> the default (but for myself I'd be happy regardless to have an
> option).
>

How about simply committing/stashing your changes in advance?

Or using `git pull --rebase', or `git fetch', or `git remote update', ... ?


> [...] Editing .emacs files


> to add options seems to be very out of fashion. (Maybe just invoke
> the custom functionality?)
>

Say what ?!? Also, what is this "fashion"-thing you're referring to?


> [...]


A more flexible solution might be to add some pre/post-* hooks,
so we can do stuff like:

#+begin_src emacs-lisp
(add-hook 'notmuch-pre-pull-hook
(lambda() (ignore-errors (magit-stash-snapshot))))
#+end_src

Peace

--
Pieter

PJ Weisberg

unread,
Feb 3, 2012, 4:22:33 PM2/3/12
to Eli Barzilay, Moritz Bunkus, Magit
On Fri, Feb 3, 2012 at 7:50 AM, Eli Barzilay <e...@barzilay.org> wrote:

> 1. How do you implement the "never ask again"?  Editing .emacs files
>   to add options seems to be very out of fashion.  (Maybe just invoke
>   the custom functionality?)

I was, indeed, thinking of calling `customize-save-variable', which
would edit .emacs, or wherever `custom-file' points. :-)

Reply all
Reply to author
Forward
0 new messages