jit-lock-antiblink-grace

classic Classic list List threaded Threaded
58 messages Options
123
Reply | Threaded
Open this post in threaded view
|

jit-lock-antiblink-grace

João Távora
Hello all,

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. 

jit-lock-antiblink-grace is set to 2 seconds by default, which works
nicely in my testing. Just launch your emacs, ensure you don't have
a quote-matching facility such as electric-pair-mode setup (in which
case the feature is mostly useless to you), and add/or remove
strings to some source code.

Thanks,
João Távora

BTW: I tried to force-push the cleaned-up version to the
scratch/jit-lock-antiblink branch. It didn't work.  Is force-pushing
disabled in those branches? It didn't use to be, IIRC.
Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Stefan Monnier
> 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.

No objection from me.

> BTW: I tried to force-push the cleaned-up version to the
> scratch/jit-lock-antiblink branch. It didn't work.  Is force-pushing
> disabled in those branches? It didn't use to be, IIRC.

Force-pushing is disabled repository-wide.  You can workaround it by
removing+pushing the branch, instead (this workaround can technically be
used on any branch, but of course we don't want it used elsewhere than
in scratch/* branches).


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Eli Zaretskii
In reply to this post by João Távora
> From: João Távora <[hidden email]>
> 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.

Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

João Távora
On Sat, Oct 12, 2019 at 10:34 AM Eli Zaretskii <[hidden email]> wrote:
> > 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.  

Everything slows down everything, the question is where and by how much.
A noop will probably not slow paging very much.  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.

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

> 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.
Or use some hack where the current timers detect the variable has been
enabled/disabled.

> Some more comments about the code:
>
> > +** New customizable variable 'jit-lock-antiblink-grace'
>
> This line should end in a period

OK.

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

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

OK.

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

What's bothering you? (assuming all other objections you stated already
are somehow dealt with satisfactorily.)

> 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.
Moreover this is a hint as to how the system is implemented, which some
users may appreciate.

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

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.

Do you really prever jit-lock--antiblink-in-string-or-comment and
jit-lock--antiblink-line-beginning-position? I think it's much harder to
follow.  But I will abide, especially if you suggest alternatives.

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

See above.

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

João
Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Juanma Barranquero
On Sat, Oct 12, 2019 at 12:59 PM João Távora <[hidden email]> wrote:

> Do you really prever jit-lock--antiblink-in-string-or-comment and
> jit-lock--antiblink-line-beginning-position?

In a clean emacs -Q we already have things like

minibuffer-history-case-insensitive-variables
jit-lock-after-change-extend-region-functions
lisp-el-font-lock-keywords-for-backtraces-1
lisp-el-font-lock-keywords-for-backtraces-2
display-buffer--action-function-custom-type
jka-compr-added-to-file-coding-system-alist
revert-buffer-insert-file-contents-function
w32-direct-print-region-use-command-dot-com
backup-by-copying-when-privileged-mismatch
window-adjust-process-window-size-function
electric-indent-functions-without-reindent
list-matching-lines-default-context-lines
lisp-el-font-lock-keywords-for-backtraces
syntax-propertize-extend-region-functions
jka-compr-compression-info-list--internal

which are longer. 

Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

João Távora
On Sat, Oct 12, 2019 at 2:02 PM Juanma Barranquero <[hidden email]> wrote:
On Sat, Oct 12, 2019 at 12:59 PM João Távora <[hidden email]> wrote:

> Do you really prever jit-lock--antiblink-in-string-or-comment and
> jit-lock--antiblink-line-beginning-position?

In a clean emacs -Q we already have things like

minibuffer-history-case-insensitive-variables
jit-lock-after-change-extend-region-functions
lisp-el-font-lock-keywords-for-backtraces-1
lisp-el-font-lock-keywords-for-backtraces-2
display-buffer--action-function-custom-type
jka-compr-added-to-file-coding-system-alist
revert-buffer-insert-file-contents-function
w32-direct-print-region-use-command-dot-com
backup-by-copying-when-privileged-mismatch
window-adjust-process-window-size-function
electric-indent-functions-without-reindent
list-matching-lines-default-context-lines
lisp-el-font-lock-keywords-for-backtraces
syntax-propertize-extend-region-functions
jka-compr-compression-info-list--internal

which are longer. 

I don't understand how this can be an argument.
Is not exceeding the length of the longest symbol
ever, i.e. playing to the 99% percentile, acceptable
criteria for readability? I don't think so, personally.

That said, I'm neither in love with the solution I came
up with nor will I kill myself over long symbol names :-)

Perhaps a plist `jit-lock--antiblink-info` can be a
compromise.
--
João Távora
Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Eli Zaretskii
In reply to this post by João Távora
> From: João Távora <[hidden email]>
> Date: Sat, 12 Oct 2019 11:57:39 +0100
> Cc: emacs-devel <[hidden email]>, Stefan Monnier <[hidden email]>
>
> > 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?

Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Stefan Monnier
>> 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

[ FWIW: I can't remember why I recommended/suggested
  post-command-hook, sorry.  ]

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

Really?  AFAIK we redisplay at the end of every command executed.
We additionally redisplay after processing filters and after receiving
an event that turned out to be a prefix key, etc...

So, AFAICT we generally redisplay at least as often as we run
post-command-hook.  The only case where we don't is when we can't keep
up with the input events in which case we skip redisplay, but that's the
case where we're *already* too slow.

> It's a backward-incompatible behavior, and is not being developed due
> to bug reports,

It was developed because people like Alan are so bothered by the
flashing that they're going through lengths to find other ways to
avoid it.

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

It shouldn't slow down cursor motion, normally (at least not in any
measurable way).

Also I expect the implementation will change over time, as experience is
gained with it.

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

Code like:

    var x = "foo y = "bar";

where the user is in the middle of writing `x = "foobar";` but hasn't yet
closed the string.


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

João Távora
In reply to this post by Eli Zaretskii
On Sat, Oct 12, 2019 at 2:33 PM Eli Zaretskii <[hidden email]> wrote:

> The post-command-hook is notorious for slowing down cursor motion.

Probably a fame unjustly inherited from functions that people put there.

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

OK, will do.

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

OK, will try that.

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

If we do it like that, it will take effect only when jit-lock-mode is
toggled.  Therefore, if you set the variable to nil in a buffer you will
only see the desired effects in post-command-hook after additionally
toggling jit-lock-mode on and off again.

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

OK.

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

Naturally Eli, I hope you believe me that I try hard, indeed very hard,
to write good documentation.  I spend I good deal of time in it,
sometimes much more than writing the code itself.  If I sometimes fail
to meet your standards, it's to be expected, (1) because they are high
(which is very good) and (2) because we all have unwritten guidelines of
what we believe is good style.  I tried many versions (many more than
you can probably get evidence of in the emacs-diffs ML) and settled on
the one I presented.

With requesting a suggestion from you, I am not taking an adversarial
position, simply a way forward on what I (and many) consider to be a
difficult and underestimated problem in programming.  Also, English is
not my first language, and Emacs-english has certain rules that you will
much more familiar with.

> > > (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).

Regarding slowdown, we have to check by how much.  Regarding the
pertinence of the modificaiton, there are mode-specific modifications
with (IMO much worse) backward-incompatible behaviour being made to
modes like to c-mode to circumvent precisely this problem.  Perhaps you
could weigh in on the pertinence of those on-by-default (and moreover
impossible-to-turn-off) alternatives, too.  Although those other
modifications target a reduced subset of modes, indeed precisely because
of that fact, I think it's better that Emacs provides an effective and
more generic solution to this problem.

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

Buffer 1:

l1: foofoo "bar            < unterminated string at EOL, terminated multiline string
l2: barbar" foofoo

Buffer 2:

l1: foofoo "bar            < unterminated string at EOL, unterminated string
l2: barbar

It's an informal way of referring to both situations at line 1.  If I
said "unterminated string" the user might be led to believe it doesn't
apply to the situation of buffer 1.

But the fact that it confused you so much is probably a hint it's not a
great idea.  Again, let me humbly request a suggestion for rephrasing :-)

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

Maybe.  Sometimes such hints are good because they help consolidate the
user's prediction of timer-based behaviour.

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

It doesn't, but then I lose the ability to i-search for
jit-lock--antiblink and see its related variables. It's all trade-offs.

But I'll put in the longer names or use a plist, don't worry.

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

Huh? If I lose the message the form becomes a noop.

>  Why is it important to display it?

Run-time consistency assertions are useful, right?  I can change
'message' to whatever you think has the optimimum noise-level for such
an assertion ('warn' with a :debug level, maybe?)  I can also remove the
form completely and we just pray that noone every breaks the mechanism
in subtle ways :-)

João
Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Eli Zaretskii
In reply to this post by Stefan Monnier
> From: Stefan Monnier <[hidden email]>
> Cc: João Távora <[hidden email]>,
>   [hidden email]
> Date: Sat, 12 Oct 2019 10:13:42 -0400
>
> > 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.
>
> Really?  AFAIK we redisplay at the end of every command executed.

No, we do that only if there's no other event in the event queue.

> We additionally redisplay after processing filters and after receiving
> an event that turned out to be a prefix key, etc...

Filters run only when Emacs is idle, so they don't run when there are
keyboard events in the queue.

> So, AFAICT we generally redisplay at least as often as we run
> post-command-hook.

I don't think this is correct, not AFAIR.

> The only case where we don't is when we can't keep up with the input
> events in which case we skip redisplay, but that's the case where
> we're *already* too slow.

My point is that post-command-hook might be called more often than
once per redisplay cycle, and I think we agree on that, right?

> > It's a backward-incompatible behavior, and is not being developed due
> > to bug reports,
>
> It was developed because people like Alan are so bothered by the
> flashing that they're going through lengths to find other ways to
> avoid it.

I'm not saying this is not a useful feature, and I'm not objecting to
its inclusion.  I'm asking why do we need to turn it on by default
right when we introduce it.

> > 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).
>
> It shouldn't slow down cursor motion, normally (at least not in any
> measurable way).

Font lock does slow down Emacs, so calling it in more cases/places
will do so as well.

> > 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?
>
> Code like:
>
>     var x = "foo y = "bar";
>
> where the user is in the middle of writing `x = "foobar";` but hasn't yet
> closed the string.

Yes, and what is the other example I asked for, where we don't have an
unterminated string at EOL due to such editing?

Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Stefan Monnier
>> The only case where we don't is when we can't keep up with the input
>> events in which case we skip redisplay, but that's the case where
>> we're *already* too slow.
>
> My point is that post-command-hook might be called more often than
> once per redisplay cycle, and I think we agree on that, right?

Yes, but only if we can't keep up (i.e. if the event queue is already
non-empty when we're done processing an event).  So whether something is
done in redisplay or in post-command-hook doesn't affect the repeat-rate
threshold where we start skipping redisplay: it only affect how much
speed we gain by skipping redisplay (i.e. how quickly we can recover).

>> > It's a backward-incompatible behavior, and is not being developed due
>> > to bug reports,
>> It was developed because people like Alan are so bothered by the
>> flashing that they're going through lengths to find other ways to
>> avoid it.
> I'm not saying this is not a useful feature, and I'm not objecting to
> its inclusion.  I'm asking why do we need to turn it on by default
> right when we introduce it.

Because I think it offers the behavior 99% of the users will want
(modulo bugs).  Everyone I ever talked to about this agreed that the
current behavior is somewhat annoying and several of them mentioned
using "strategies" to avoid it (e.g. use electric-pair-mode, or be sure
to keep typing quickly so the string/comment gets closed before the
jit-lock-context-timer fires, ...).

>> > 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).
>> It shouldn't slow down cursor motion, normally (at least not in any
>> measurable way).
> Font lock does slow down Emacs, so calling it in more cases/places
> will do so as well.

AFAICT the code doesn't call font-lock.

>> > 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?
>>
>> Code like:
>>
>>     var x = "foo y = "bar";
>>
>> where the user is in the middle of writing `x = "foobar";` but hasn't yet
>> closed the string.
>
> Yes, and what is the other example I asked for, where we don't have an
> unterminated string at EOL due to such editing?

That would be "any normal line", as in:

    var x = "foobar"; y = "bar";

"unterminated string at EOL" means (nth 3 (syntax-ppss
(line-end-position))) is non-nil.


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Juanma Barranquero
In reply to this post by João Távora


On Sat, Oct 12, 2019 at 3:14 PM João Távora <[hidden email]> wrote:

> I don't understand how this can be an argument.

It wasn't an argument, it was a light-hearted comment.

> Is not exceeding the length of the longest symbol
> ever, i.e. playing to the 99% percentile, acceptable
> criteria for readability? I don't think so, personally.

Well, me, I prefer longer names to cryptic ones.

Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

João Távora
On Sat, Oct 12, 2019, 15:50 Juanma Barranquero <[hidden email]> wrote:


On Sat, Oct 12, 2019 at 3:14 PM João Távora <[hidden email]> wrote:

> I don't understand how this can be an argument.

It wasn't an argument, it was a light-hearted comment.

Sorry for the knee-jerk, then.


Well, me, I prefer longer names to cryptic ones

Then it _was_ an argument :-) "cryptic" is a value judgement. I personally decide on form on a case by case basis, but I can't really write the cost function I use. Unfortunately, after a while, you become biased. It's often when you actually witness others try to understand what you wrote that you realize the consequences of one decision over the other. That's hard to do in this ML... But did you try to read it?

João
Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Juanma Barranquero

On Sat, Oct 12, 2019 at 5:11 PM João Távora <[hidden email]> wrote:

> Then it _was_ an argument :-)

Nope. I can easily decouple what I prefer, and what could be best, or what most prefer ;-)  I dislike customize, but I would never argue against it, for example.

> But did you try to read it?

Yes. But I have nothing of value to add, other than misplaced light-hearted comments ;-)  I'l shut up now.


Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Alan Mackenzie
In reply to this post by Stefan Monnier
Hello, Stefan.

On Sat, Oct 12, 2019 at 10:13:42 -0400, Stefan Monnier wrote:

[ .... ]

> > It's a backward-incompatible behavior, and is not being developed due
> > to bug reports,

> It was developed because people like Alan are so bothered by the
> flashing that they're going through lengths to find other ways to
> avoid it.

Who, me?  The flashing bothers me not at all, though there will surely
be people it does bother.  I think you're referring to the feature in CC
Mode where unterminated strings are font-locked to match their error
syntax, i.e. the string face ends at the next unescaped EOL.  This is a
static feature, not a dynamic one like João has just developed.  The two
features are not closely related.

[ .... ]

>         Stefan

--
Alan Mackenzie (Nuremberg, Germany).

Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Eli Zaretskii
In reply to this post by Stefan Monnier
> From: Stefan Monnier <[hidden email]>
> Cc: [hidden email],  [hidden email]
> Date: Sat, 12 Oct 2019 10:34:44 -0400
>
> >> The only case where we don't is when we can't keep up with the input
> >> events in which case we skip redisplay, but that's the case where
> >> we're *already* too slow.
> >
> > My point is that post-command-hook might be called more often than
> > once per redisplay cycle, and I think we agree on that, right?
>
> Yes, but only if we can't keep up (i.e. if the event queue is already
> non-empty when we're done processing an event).  So whether something is
> done in redisplay or in post-command-hook doesn't affect the repeat-rate
> threshold where we start skipping redisplay: it only affect how much
> speed we gain by skipping redisplay (i.e. how quickly we can recover).

What is the purpose of this argument?  Do you agree that adding too
much to post-command-hook should be avoided?  If you do, then let's
not split hair about the rest, okay?

> > I'm not saying this is not a useful feature, and I'm not objecting to
> > its inclusion.  I'm asking why do we need to turn it on by default
> > right when we introduce it.
>
> Because I think it offers the behavior 99% of the users will want
> (modulo bugs).

You are entitled to your opinion, and I'm entitled to mine.

> Everyone I ever talked to about this agreed that the current
> behavior is somewhat annoying

Here, you are talking to someone who is much more annoyed by the
sluggishness of our movement commands than by an occasional "flashes"
of incorrect fontification when I'm in the process of modifying code.
So I guess not "everyone" finds this annoying enough.

> > Font lock does slow down Emacs, so calling it in more cases/places
> > will do so as well.
>
> AFAICT the code doesn't call font-lock.

It adds a timer that does.

Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Eli Zaretskii
In reply to this post by João Távora
> From: João Távora <[hidden email]>
> Date: Sat, 12 Oct 2019 15:23:08 +0100
> Cc: emacs-devel <[hidden email]>, Stefan Monnier <[hidden email]>
>
> > The post-command-hook is notorious for slowing down cursor motion.
>
> Probably a fame unjustly inherited from functions that people put there.

Not people: our own minor modes.

> > 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.
>
> If we do it like that, it will take effect only when jit-lock-mode is
> toggled.  Therefore, if you set the variable to nil in a buffer you will
> only see the desired effects in post-command-hook after additionally
> toggling jit-lock-mode on and off again.

It doesn't have to be like that.  It depends on how you arrange for
the hook not to be added.

> > 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.
>
> Naturally Eli, I hope you believe me that I try hard, indeed very hard,
> to write good documentation.  I spend I good deal of time in it,
> sometimes much more than writing the code itself.  If I sometimes fail
> to meet your standards, it's to be expected, (1) because they are high
> (which is very good) and (2) because we all have unwritten guidelines of
> what we believe is good style.  I tried many versions (many more than
> you can probably get evidence of in the emacs-diffs ML) and settled on
> the one I presented.
>
> With requesting a suggestion from you, I am not taking an adversarial
> position, simply a way forward on what I (and many) consider to be a
> difficult and underestimated problem in programming.  Also, English is
> not my first language, and Emacs-english has certain rules that you will
> much more familiar with.

All I wanted to say was that the goal is almost in reach, and that you
don't need to give up in this case.

But if you think it's too hard, then okay, I will write the text when
the time comes.

> > 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).
>
> Regarding slowdown, we have to check by how much.  Regarding the
> pertinence of the modificaiton, there are mode-specific modifications
> with (IMO much worse) backward-incompatible behaviour being made to
> modes like to c-mode to circumvent precisely this problem.  Perhaps you
> could weigh in on the pertinence of those on-by-default (and moreover
> impossible-to-turn-off) alternatives, too.  Although those other
> modifications target a reduced subset of modes, indeed precisely because
> of that fact, I think it's better that Emacs provides an effective and
> more generic solution to this problem.

When I find a backward-incompatible change, I usually do try to see if
it's justified.  So I think I already do what you ask me to do in
those other cases.

> > 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?
>
> Buffer 1:
>
> l1: foofoo "bar            < unterminated string at EOL, terminated multiline string
> l2: barbar" foofoo
>
> Buffer 2:
>
> l1: foofoo "bar            < unterminated string at EOL, unterminated string
> l2: barbar
>
> It's an informal way of referring to both situations at line 1.

That's what I thought, but then I think we should rather talk about
"unbalanced quotes", which should catch both cases without involving
EOL.

> > Why not just lose the message?
>
> Huh? If I lose the message the form becomes a noop.

I meant lose the entire form.  Why should a user care that there was
already a timer?  Will that adversely affect the code in some way?

> Run-time consistency assertions are useful, right?

Only as a debug option, or if the code absolutely cannot continue
under that condition.  Which I think is not the case here.

Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Alan Mackenzie
In reply to this post by João Távora
Hello, João.

On Sat, Oct 12, 2019 at 15:23:08 +0100, João Távora wrote:
> On Sat, Oct 12, 2019 at 2:33 PM Eli Zaretskii <[hidden email]> wrote:

[ .... ]

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

> Regarding slowdown, we have to check by how much.  Regarding the
> pertinence of the modificaiton, there are mode-specific modifications
> with (IMO much worse) backward-incompatible behaviour being made to
> modes like to c-mode to circumvent precisely this problem.

No.  The feature in CC Mode I think you're referring to, namely the
correct fontification of unterminated strings is a static feature,
clearly indicating that the string hasn't (yet) been terminated, and
where its current end (as would be interpreted by a compiler) is.  The
feature you've implemented is, if I understand correctly (I haven't tried
it, yet) a dynamic one, where fontification gets delayed a bit to give
the user a certain chance to terminate an unterminated string before
fontification kicks in.

I would guess that your new feature is less needed in a CC Mode mode than
in modes where the fontification of a string extends to the next string
quote, no matter how far away.

> Perhaps you could weigh in on the pertinence of those on-by-default
> (and moreover impossible-to-turn-off) alternatives, too.  Although
> those other modifications target a reduced subset of modes, indeed
> precisely because of that fact, I think it's better that Emacs provides
> an effective and more generic solution to this problem.

I agree with that.  My view is that such a feature should be provided in
syntax.c and the font locking system, such that a major mode can specify
that unterminated strings are to be regarded as terminated by an
unescaped new line.

I think I said this before, off hand, but it didn't go anywhere.

[ .... ]

> João

--
Alan Mackenzie (Nuremberg, Germany).

Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Stefan Monnier
In reply to this post by Eli Zaretskii
> What is the purpose of this argument?  Do you agree that adding too
> much to post-command-hook should be avoided?

I agree that post-command-hook is never the "right" place.  But I don't
think moving to a place that runs at every redisplay will help very much
in terms of the risk of impacting responsiveness.

Also often there's no "right" place, so we end up choosing
post-command-hook because it's the least bad choice.

I'd like the patch to be installed, because I think it'll need tweaking
but that we need more experience with it in order to figure out what
to do.

>> > Font lock does slow down Emacs, so calling it in more cases/places
>> > will do so as well.
>> AFAICT the code doesn't call font-lock.
> It adds a timer that does.

AFAICT this only runs code which has been delayed (i.e. it would have
been run earlier if it weren't for this feature), so it's "no worse".

And it's an idle-timer run after 2s of idle time, so impact on
responsiveness should be minimal even in the worst case.


        Stefan


Reply | Threaded
Open this post in threaded view
|

Re: jit-lock-antiblink-grace

Eli Zaretskii
> From: Stefan Monnier <[hidden email]>
> Cc: [hidden email],  [hidden email]
> Date: Sat, 12 Oct 2019 13:16:45 -0400
>
> >> AFAICT the code doesn't call font-lock.
> > It adds a timer that does.
>
> AFAICT this only runs code which has been delayed (i.e. it would have
> been run earlier if it weren't for this feature), so it's "no worse".
>
> And it's an idle-timer run after 2s of idle time, so impact on
> responsiveness should be minimal even in the worst case.

Idle timers that run expensive code make Emacs feel very unresponsive.
That's one reason why we introduced JIT lock, many years ago.

123