Re: Removing rollback from VC mode - request for comment

[Jonas Bernoulli]

Eli Zaretskii <address@hidden> writes:

> I didn't say that magit uses some undocumented feature, or uses it
> contrary to documentation.  That's not the issue.
>
> The issue is that magit uses these features too much, at times almost
> completely obscuring the buffer text with display and overlay
> strings.  The Emacs display engine was never meant to cope with such
> massive usage of these features.  I think magit also uses a lot of
> overlay/display strings with newlines, or maybe it did in the past.
> That is one of the nastiest thing to do with Emacs display.  (I can
> explain if you are interested.)

Magit uses a lot of overlays indeed, but that is slowly being phased out
where possible.

One thing that will certainly stay is that section bodies are made
invisible using the invisible text-property.  I cannot think of another
place were Magit hides text.  You mention newlines being molested, but
I cannot remember having seen anything like that in Magit.

Back to overlays.  Magit highlights the current section(s) using an
overlay, which by default changes the background color like the region
does.  Unlike the region, it is always visible and it would therefor be
undesirable if it did overwrite text-property backgrounds.  To some
users that would be annoying for example in logs where "ref labels" like
branch names and tags, would lose their background.

Since overlays always win against text-properties the only way to keep
the backgrounds of various pieces of text visible, is to paint them
using overlays.  I agree that is icky, but as long as overlays cannot
have a negative priority, I can see why people use this sort of hack.

Unfortunately there was no abstraction to do this in Magit and some of
the ad hock implementations had severe bugs.  A lot of cleaning up later
there is an abstraction: `magit-insert'.  Like `insert' it inserts the
strings, but unlike that it also moves the face property to an overlay.
It's a hack.  The only thing missing is an option to control whether it
should do this at all, then it would behave more or less like `insert'
together with `propertize'.

Once that option exists and is set to *off* Magit won't use overlays in
any excessive kind of way anymore.  I will do that once I have changed
the "default themes" to not set the background anymore.

But this is not something that can be changed without there being a way
to "go back as it was in the good old days".  Users will protest about
this, even when there is an option to enable the hack again.

> And frankly, I don't understand why this kind of design was necessary:
> it's not like magit shows some file whose contents it can't control.
> It presents a buffer that is entirely created out of thin air by Git.

Mostly historic accidents, I think, and because negative priorities
haven't been implemented.

> If what Git generates doesn't fit what magit wants to show the user,
> what prevents magit from massaging the text it gets from Git to its
> heart's content, when creating the buffer?

Not much, and in fact that's what it is doing most of the time, except
when `magit-insert' is used.  There are other places where overlays are
used, but once `magit-insert' has been taught to not do so, they won't
be used excessively anymore.

> Instead of overlaying buffer text with display and overlay strings, it
> could simply format the buffer text any way it likes, and then avoid
> the need to do all that.

If a user wants to highlight the current section and also refnames with
a background color, which don't get lost, then Magit *does* have to use
overlays.  I wouldn't have added the overlay hack in the first place but
it has existed for years now, and cannot be removed completely because
doing that would be perceived as a regression.

> I hope this explains why I feel badly about that.

Yes.  I think you are saying that using these techniques aren't bad
per se, but that using them excessively has a negative impact on
performance.  The point I am trying to make is that I generally agree,
but there is/was a reason why it is/was done this way.  Also I think,
that while Magit has some performance issues, the use of overlays isn't
really a major cause of this.  And I am prioritizing fixing actual
performance issues over dealing with the overlay problem.