Re: jit-lock-antiblink-grace

[Eli Zaretskii]

> From: João Távora <address@hidden>
> Date: Sat, 12 Oct 2019 11:57:39 +0100
> Cc: emacs-devel <address@hidden>, Stefan Monnier <address@hidden>
> 
> > Bother: this unconditionally adds a post-command-hook, which will
> > necessarily slow down paging through a file.  
> 
> Everything slows down everything, the question is where and by how much.

The post-command-hook is notorious for slowing down cursor motion.
Many complaints I hear about Emacs being sluggish eventually boil down
to some minor mode inserting itself into that hook and doing
non-trivial stuff there.  We should therefore do so very cautiously.

This case is more important than others because JIT font-lock is
enabled by default, so this change will affect every user out there,
in any possible major mode.

> Can you tell me more about the kinds of files you're anxious about
> and exact meaning of "paging"?  Is it C-n'ing from the first line to
> the last?  I could benchmark.

C-n and C-v.  Try C mode for starters, and then Emacs Lisp.

> > If there's no better solution than using that over-used hook,
> 
> My very first version relied on an extension of the existing
> jit-lock-context-time, but I seem to remember it broke down here and
> there sometimes.  I agreed with Stefan (possibly off-list) to use a
> post-command-hook, which is safer.  But I can have a look at the
> original version and re-study those problems more closely.

We also have display-related hooks.  If you could use one of them,
that might be better, because one could generally type quite a few
commands before redisplay kicks in, and post-command-hook runs once
for every command.

> > then at the very least we should give users a way of NOT adding a
> > post-command-hook when this feature is disabled.
> 
> Given that I intend for this to be controlled via a customization
> variable, I only see it done via a `:set` hook or something like that.

I'm not sure I understand.  What I wanted to suggest is that when the
new defcustom is nil (which seems to be the way to disable this
feature), the post-command-hook should not be added.

In addition, I think major modes which present read-only buffers
should also disable this feature, and not add to post-command-hook.

> > > +Setting this to a positive number of seconds helps avoid the
> > > +fontification "blinking" behaviour observed when adding temporarily
> > > +unterminated strings to source code.  If the user has recently created
> > > +an unterminated string at EOL, jit fontification allows this idle
> > > +"grace" period to elapse before deciding it is a multi-line string and
> > > +fontifying the remainder of the buffer accordingly.
> >
> > This should be simplified and shortened.  (In general, copy/paste of
> > doc strings into NEWS is not a good idea.)  In particular, if the
> > default is to have this behavior (see below), the NEWS entry should
> > tell how to disable that.
> 
> OK.  Supposing you've already already gotten the idea, I invite you to
> submit a suggestion.

I'd rather had you try.  I think it's important that more people can
write good documentation, and I think in this case it isn't hard.  You
can use other entries as examples.

> > (I question the wisdom of making this the default behavior, btw.)
> 
> What's bothering you?

It's a backward-incompatible behavior, and is not being developed due
to bug reports, so why make it the default right from the start?  It
also slows down cursor motion (which should probably be in the doc
string as well).

> > I don't understand the "at EOL" part: isn't any unterminated string
> > appear as if it is "at EOL"?
> 
> An unterminated string at EOL might be terminated somewhere _after_ EOL,
> i.e. a perfectly legitimate (as "in your intentions") multiline string.

I still don't think I understand what would constitute an
"unterminated string at EOL", then.  Could you show two examples, one
where there is such a string, the other where there isn't?

> Moreover this is a hint as to how the system is implemented, which some
> users may appreciate.

This kind of stuff should be in comments, not ion doc strings, IMO.

> > Please don't use such cryptic variable names, at least not on the file
> > level (preferably also not locally inside functions).
> 
> The docstring explains the abbreviation.  I'm afraid that given current
> naming practice (prefix, double dash, sub-feature) I couldn't do much
> better.  I think jit-lock--antiblink-l-b-p is a better name than
> jit-lock--antiblink-pos, or jit-lock--pos, because it makes the reader
> "chase" the doc and learn of the exact meaning of the abbreviation.

The variables don't need to use jit-lock--antiblink- prefix, they
could use jit-lock-- prefix instead.  jit-lock--last-line-begpos
doesn't sound too long to me, for example.

> > > +           (when jit-lock--antiblink-grace-timer
> > > +             (message "internal warning: 
> > > `jit-lock--antiblink-grace-timer' not null"))
> >
> > We should in general avoid calling 'message' here, because such a
> > message will appear after every command, which is a nuisance.  Is this
> > really needed?
> 
> This is an internal consistency check, i.e. a run-time assertion.  It
> should never happen, except when the program is buggy.  I had this set
> to 'warn', but Stefan suggested I change it.  What do you suggest?
> Perhaps I could warn and turn off the feature.

Why not just lose the message?  Why is it important to display it?