Re: jit-lock-antiblink-grace

[Eli Zaretskii]

> From: João Távora <address@hidden>
> Date: Thu, 10 Oct 2019 23:34:25 +0100
> 
> A while back I coded up a feature in a scratch branch that 
> I dubbed the "antiblink".  It helps avoid the fontification "blinking"
> behaviour observed when creating temporarily unterminated 
> strings to source code.
> 
> I recall that Lars and Clément tested it with generally positive results.
> 
> At Stefan's request, I have reworked the implementation and 
> cleaned up the feature, which is embodied in the jit-lock-antiblink-grace
> variable and want to land it on master if no-one objects.
> 
> If you want to try it before that happens see the 
> scratch/jit-lock-antiblink-cleaned-up branch.  

Bother: this unconditionally adds a post-command-hook, which will
necessarily slow down paging through a file.  If there's no better
solution than using that over-used hook, then at the very least we
should give users a way of NOT adding a post-command-hook when this
feature is disabled.

Some more comments about the code:

> +** New customizable variable 'jit-lock-antiblink-grace'

This line should end in a period

> +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.

> +(defcustom jit-lock-antiblink-grace 2
> +  "Like `jit-lock-context-time' but for unterminated multiline strings.
> +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, allow for an idle
> +\"grace\" period to elapse before deciding it is a multi-line
> +string and fontifying the remainder of the buffer accordingly."
> +  :type '(number :tag "seconds")
> +  :group 'jit-lock)

This new defcustom should have a :version tag.

The doc string should say how to disable the feature.  Also, the doc
string makes it sound like the default is not a positive number of
seconds by default, but it is.  (I question the wisdom of making this
the default behavior, btw.)

I don't understand the "at EOL" part: isn't any unterminated string
appear as if it is "at EOL"?

> +(defvar jit-lock--antiblink-l-b-p (make-marker)
> +  "Last line beginning position (l-b-p) after last command (a marker).")
> +(defvar jit-lock--antiblink-i-s-o-c nil
> +  "In string or comment (i-s-o-c) after last command (a boolean).")

Please don't use such cryptic variable names, at least not on the file
level (preferably also not locally inside functions).

> +      (add-hook 'post-command-hook 'jit-lock--antiblink-post-command nil t)

As mentioned above, this hook should not be added if the feature is
disabled.

> +           (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?

Thanks.