bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello, Simen.

In article <[hidden email]> you wrote:
> In js-mode, the following JavaScript comment is filled as expected when
> fill-paragraph (M-q) is used with point within it:

> /*
>  * This is a long comment that should break over multiple lines when fill-paragraph is used.
>  */

> Is filled as:

> /*
>  * This is a long comment that should break over multiple lines when
>  * fill-paragraph is used.
>  */

> In mhtml-mode however, the same JavaScript comment:

> <html>
>   <script>
>     /*
>      * This is a long comment that should break over multiple lines when fill-paragraph is used.
>      */
>   </script>
> </html>

> Is filled as:

> <html>
>   <script>
>     /* * This is a long comment that should break over multiple lines
>      when fill-paragraph is used.  */
>   </script>
> </html>

Yes.  This is happening because the mechanism mhtml-mode is using to
"switch to" js-mode is imperfect.  Having more than one major mode in a
buffer is a difficult problem for Emacs, and there are currently no
really satisfactory solutions.

"Switching to" another mode essentially means setting buffer local
variable copies.  In the existing code, a variable essential to filling,
adaptive-fill-regexp, isn't getting set to a value suitable for js-mode.

Also, there was an error in the js-mode setting of the variable
comment-start-skip.

The patch below fixes both these errors, and seems to allow your test
case to work.  However, it is an incomplete solution - in particular,
filling caused by typing at the end of a long line still isn't working
properly.  Still, it may be better than nothing, at least for now.  I
intend to carry on working on this bug.

You haven't said what version of Emacs you're running.  The patch
applies cleanly both to the upcoming emacs-27 branch and the master
branch at GNU savannah.  To apply it, cd into ..../emacs/lisp, and
execute:

    $ patch -p2 < this-email

.  Then byte-compile the two changed files and load them (possibly by
restarting Emacs).

Here's the patch:



diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 04b449ecd2..f5b6a4c260 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -4570,7 +4570,7 @@ js-mode
 
   ;; Comments
   (setq-local comment-start "// ")
-  (setq-local comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
+  (setq-local comment-start-skip "\\(?://+\\|/\\*+\\)\\s *")
   (setq-local comment-end "")
   (setq-local fill-paragraph-function #'js-fill-paragraph)
   (setq-local normal-auto-fill-function #'js-do-auto-fill)
diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
index 1ae07c0a30..069329f4e4 100644
--- a/lisp/textmodes/mhtml-mode.el
+++ b/lisp/textmodes/mhtml-mode.el
@@ -73,7 +73,8 @@ mhtml-tag-relative-indent
 
 (defconst mhtml--crucial-variable-prefix
   (regexp-opt '("comment-" "uncomment-" "electric-indent-"
-                "smie-" "forward-sexp-function" "completion-" "major-mode"))
+                "smie-" "forward-sexp-function" "completion-" "major-mode"
+                "adaptive-fill-" "fill-"))
   "Regexp matching the prefix of \"crucial\" buffer-locals we want to capture.")
 
 (defconst mhtml--variable-prefix


--
Alan Mackenzie (Nuremberg, Germany).




Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Simen Heggestøyl-3
Hi Alan, thanks for working on this.

Alan Mackenzie <[hidden email]> writes:

> The patch below fixes both these errors, and seems to allow your test
> case to work.

Yes, this seems to fix it in both emacs-27 and master, thanks.

However in emacs-27 with the patch applied I now get a strange
side-effect: After filling the comment, mhtml-mode seems to lose track
of where the JavaScript portion of the buffer is. Please see the
attached image. The position where point is should still be recognized
as HTML+JS, but it's recognized as plain HTML+ instead after
filling. This doesn't happen on master.

> You haven't said what version of Emacs you're running.

I put the version number in the bug subject but forgot to mention it in
the description, sorry about that.

-- Simen

bug41897.png (63K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello again, Simen.

On Sat, Jun 20, 2020 at 20:27:22 +0200, Simen Heggestøyl wrote:
> Alan Mackenzie <[hidden email]> writes:

> > The patch below fixes both these errors, and seems to allow your test
> > case to work.

> Yes, this seems to fix it in both emacs-27 and master, thanks.

> However in emacs-27 with the patch applied I now get a strange
> side-effect: After filling the comment, mhtml-mode seems to lose track
> of where the JavaScript portion of the buffer is. Please see the
> attached image. The position where point is should still be recognized
> as HTML+JS, but it's recognized as plain HTML+ instead after
> filling. This doesn't happen on master.

Yes.  mhtml-mode keeps track of the position of the JavaScript bit by
applying text-properties.  In certain circumstances, when point is at
(point-min), it fails to re-apply those properties after removing them.
This was happening in the filling, which uses narrowing.  I think I've
got this fixed, now.  (See patch below.)

> > You haven't said what version of Emacs you're running.

> I put the version number in the bug subject but forgot to mention it in
> the description, sorry about that.

No, my apologies: I completely missed it in the subject line.  I'm happy
that you're working in master, and not Emacs 26.  :-)

Anyhow, I've made significant progress on the problem.  As well as the
problem above, I've repaired a few defects with auto-fill-mode.  It's
more or less working.  But M-q isn't completely working.  If a buffer
contains the long line of your test case, followed by another line, M-q
doesn't fill them together, leaving the short line untouched.

I've had problems with auto-fill-function, and the current state in the
patch involves mhtml-mode assuming auto-fill-function is enabled.  This
obviously needs fixing, but that's the way it is for now.  Sorry.

There's still a bug with M-q where it sometimes throws an error about a
search being "on the wrong side of point".  I haven't had time to
investigate this, yet.

So here's the current patch for the problem.  Please install it on
master after removing yesterday's patch.  From the .../emacs/lisp
directory, it should work with $ patch -p2 < this-email.  (Or, maybe
there's a git command to do it more directly.)



diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 04b449ecd2..f5b6a4c260 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -4570,7 +4570,7 @@ js-mode
 
   ;; Comments
   (setq-local comment-start "// ")
-  (setq-local comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
+  (setq-local comment-start-skip "\\(?://+\\|/\\*+\\)\\s *")
   (setq-local comment-end "")
   (setq-local fill-paragraph-function #'js-fill-paragraph)
   (setq-local normal-auto-fill-function #'js-do-auto-fill)
diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
index 1ae07c0a30..c5970cbcd1 100644
--- a/lisp/textmodes/mhtml-mode.el
+++ b/lisp/textmodes/mhtml-mode.el
@@ -73,15 +73,18 @@ mhtml-tag-relative-indent
 
 (defconst mhtml--crucial-variable-prefix
   (regexp-opt '("comment-" "uncomment-" "electric-indent-"
-                "smie-" "forward-sexp-function" "completion-" "major-mode"))
+                "smie-" "forward-sexp-function" "completion-" "major-mode"
+                "adaptive-fill-" "fill-" "auto-fill-function"))
   "Regexp matching the prefix of \"crucial\" buffer-locals we want to capture.")
 
 (defconst mhtml--variable-prefix
   (regexp-opt '("font-lock-" "indent-line-function"))
   "Regexp matching the prefix of buffer-locals we want to capture.")
 
-(defun mhtml--construct-submode (mode &rest args)
-  "A wrapper for make-mhtml--submode that computes the buffer-local variables."
+(defun mhtml--construct-submode (mode crucial-localized localized &rest args)
+  "A wrapper for make-mhtml--submode that computes the buffer-local variables.
+CRUCIAL-LOCALIZED and LOCALIZED are lists of symbols which must
+be considered crucial-local and local variables respectively."
   (let ((captured-locals nil)
         (crucial-captured-locals nil)
         (submode (apply #'make-mhtml--submode args)))
@@ -94,6 +97,16 @@ mhtml--construct-submode
       (unless (variable-binding-locus 'font-lock-fontify-region-function)
         (setq-local font-lock-fontify-region-function
                     #'font-lock-default-fontify-region))
+      ;; Get the values from global variables which might become buffer local.
+      (dolist (iter crucial-localized)
+        (let ((sym (if (symbolp iter) iter (car iter)))
+              (val (symbol-value (if (symbolp iter) iter (cdr iter)))))
+          (push (cons sym val) crucial-captured-locals)))
+      (dolist (iter localized)
+        (let ((sym (if (symbolp iter) iter (car iter)))
+              (val (symbol-value (if (symbolp iter) iter (cdr iter)))))
+          (push (cons sym val) captured-locals)))
+
       (dolist (iter (buffer-local-variables))
         (when (string-match mhtml--crucial-variable-prefix
                             (symbol-name (car iter)))
@@ -119,6 +132,8 @@ mhtml--mark-crucial-buffer-locals
 
 (defconst mhtml--css-submode
   (mhtml--construct-submode 'css-mode
+                            '((auto-fill-function . normal-auto-fill-function))
+                            nil
                             :name "CSS"
                             :end-tag "</style>"
                             :syntax-table css-mode-syntax-table
@@ -127,6 +142,8 @@ mhtml--css-submode
 
 (defconst mhtml--js-submode
   (mhtml--construct-submode 'js-mode
+                            '((auto-fill-function . normal-auto-fill-function))
+                            nil
                             :name "JS"
                             :end-tag "</script>"
                             :syntax-table js-mode-syntax-table
@@ -202,12 +219,16 @@ mhtml--stashed-crucial-variables
 (defun mhtml--stash-crucial-variables ()
   (setq mhtml--stashed-crucial-variables
         (mapcar (lambda (sym)
-                  (cons sym (buffer-local-value sym (current-buffer))))
+                  (if (boundp sym)
+                      (cons sym (buffer-local-value sym (current-buffer)))
+                    sym))
                 mhtml--crucial-variables)))
 
 (defun mhtml--map-in-crucial-variables (alist)
   (dolist (item alist)
-    (set (car item) (cdr item))))
+    (if (symbolp item)
+        (makunbound item)
+      (set (car item) (cdr item)))))
 
 (defun mhtml--pre-command ()
   (let ((submode (get-text-property (point) 'mhtml-submode)))
@@ -255,17 +276,14 @@ mhtml--syntax-propertize
    sgml-syntax-propertize-rules))
 
 (defun mhtml-syntax-propertize (start end)
-  ;; First remove our special settings from the affected text.  They
-  ;; will be re-applied as needed.
-  (remove-list-of-text-properties start end
-                                  '(syntax-table local-map mhtml-submode))
-  (goto-char start)
-  ;; Be sure to look back one character, because START won't yet have
-  ;; been propertized.
-  (unless (bobp)
-    (let ((submode (get-text-property (1- (point)) 'mhtml-submode)))
-      (if submode
-          (mhtml--syntax-propertize-submode submode end))))
+  (let ((submode (get-text-property start 'mhtml-submode)))
+    ;; First remove our special settings from the affected text.  They
+    ;; will be re-applied as needed.
+    (remove-list-of-text-properties start end
+                                    '(syntax-table local-map mhtml-submode))
+    (goto-char start)
+    (if submode
+        (mhtml--syntax-propertize-submode submode end)))
   (sgml-syntax-propertize (point) end mhtml--syntax-propertize))
 
 (defun mhtml-indent-line ()


> -- Simen

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
In reply to this post by Simen Heggestøyl-3
Hello, Simen.

We're gradually converging on working code.  Since yesterday, the
improvements are:

(i) (slightly) better handling of auto-fill-mode, which is no longer
automatically enabled;

(ii) An enhancement to CC Mode, used by mhtml-mode and js-mode, which
invalidates a CC Mode cache used by the filling code.  This gets rid of
a few strange looking bugs;

(iii) The inclusion of "paragraph-" in the "crucial variables" thing, so
that paragraph-start and paragraph-separate from js-mode get into
mhtml-mode.

On Sat, Jun 20, 2020 at 20:27:22 +0200, Simen Heggestøyl wrote:
> Hi Alan, thanks for working on this.

> Alan Mackenzie <[hidden email]> writes:

> > The patch below fixes both these errors, and seems to allow your test
> > case to work.

> Yes, this seems to fix it in both emacs-27 and master, thanks.

> However in emacs-27 with the patch applied I now get a strange
> side-effect: After filling the comment, mhtml-mode seems to lose track
> of where the JavaScript portion of the buffer is. Please see the
> attached image. The position where point is should still be recognized
> as HTML+JS, but it's recognized as plain HTML+ instead after
> filling. This doesn't happen on master.

I think this is now properly fixed.

Would you please try the latest patch, which might, just might, be fully
working code.  As always, this is a diff on the code _without_ previous
patches.  I haven't tested bits of enclosed css-mode source - you're
certainly better equipped to do this than I am.



diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index 8c8296fd6d..d8279ba4f0 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -3209,6 +3209,24 @@ c-truncate-lit-pos-cache
  c-semi-near-cache-limit (min c-semi-near-cache-limit pos)
  c-full-near-cache-limit (min c-full-near-cache-limit pos)))
 
+(defun c-foreign-truncate-lit-pos-cache (beg _end)
+  "Truncate CC Mode's literal cache.
+
+This function should be added to the `before-change-functions'
+hook by major modes that use CC Mode's filling functionality
+without initializing CC Mode.  Currently (2020-06) these are
+js-mode and mhtml-mode."
+  (c-truncate-lit-pos-cache beg))
+
+(defun c-foreign-init-lit-pos-cache ()
+  "Initialize CC Mode's literal cache.
+
+This function should be called from the mode functions of major
+modes which use CC Mode's filling functionality without
+initializing CC Mode.  Currently (2020-06) these are js-mode and
+mhtml-mode."
+  (c-truncate-lit-pos-cache 1))
+
 
 ;; A system for finding noteworthy parens before the point.
 
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 04b449ecd2..5c50e2accd 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -4570,7 +4570,7 @@ js-mode
 
   ;; Comments
   (setq-local comment-start "// ")
-  (setq-local comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
+  (setq-local comment-start-skip "\\(?://+\\|/\\*+\\)\\s *")
   (setq-local comment-end "")
   (setq-local fill-paragraph-function #'js-fill-paragraph)
   (setq-local normal-auto-fill-function #'js-do-auto-fill)
@@ -4591,6 +4591,8 @@ js-mode
   (setq imenu-create-index-function #'js--imenu-create-index)
 
   ;; for filling, pretend we're cc-mode
+  (c-foreign-init-lit-pos-cache)
+  (add-hook 'before-change-functions #'c-foreign-truncate-lit-pos-cache nil t)
   (setq-local comment-line-break-function #'c-indent-new-comment-line)
   (setq-local comment-multi-line t)
   (setq-local electric-indent-chars
diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
index 1ae07c0a30..bed0ced618 100644
--- a/lisp/textmodes/mhtml-mode.el
+++ b/lisp/textmodes/mhtml-mode.el
@@ -73,7 +73,9 @@ mhtml-tag-relative-indent
 
 (defconst mhtml--crucial-variable-prefix
   (regexp-opt '("comment-" "uncomment-" "electric-indent-"
-                "smie-" "forward-sexp-function" "completion-" "major-mode"))
+                "smie-" "forward-sexp-function" "completion-" "major-mode"
+                "adaptive-fill-" "fill-" "normal-auto-fill-function"
+                "paragraph-"))
   "Regexp matching the prefix of \"crucial\" buffer-locals we want to capture.")
 
 (defconst mhtml--variable-prefix
@@ -94,6 +96,7 @@ mhtml--construct-submode
       (unless (variable-binding-locus 'font-lock-fontify-region-function)
         (setq-local font-lock-fontify-region-function
                     #'font-lock-default-fontify-region))
+
       (dolist (iter (buffer-local-variables))
         (when (string-match mhtml--crucial-variable-prefix
                             (symbol-name (car iter)))
@@ -255,17 +258,14 @@ mhtml--syntax-propertize
    sgml-syntax-propertize-rules))
 
 (defun mhtml-syntax-propertize (start end)
-  ;; First remove our special settings from the affected text.  They
-  ;; will be re-applied as needed.
-  (remove-list-of-text-properties start end
-                                  '(syntax-table local-map mhtml-submode))
-  (goto-char start)
-  ;; Be sure to look back one character, because START won't yet have
-  ;; been propertized.
-  (unless (bobp)
-    (let ((submode (get-text-property (1- (point)) 'mhtml-submode)))
-      (if submode
-          (mhtml--syntax-propertize-submode submode end))))
+  (let ((submode (get-text-property start 'mhtml-submode)))
+    ;; First remove our special settings from the affected text.  They
+    ;; will be re-applied as needed.
+    (remove-list-of-text-properties start end
+                                    '(syntax-table local-map mhtml-submode))
+    (goto-char start)
+    (if submode
+        (mhtml--syntax-propertize-submode submode end)))
   (sgml-syntax-propertize (point) end mhtml--syntax-propertize))
 
 (defun mhtml-indent-line ()
@@ -333,6 +333,18 @@ mhtml-mode
   ;: Hack
   (js--update-quick-match-re)
 
+  ;; Setup the appropriate js-mode value of auto-fill-function.
+  (setf (mhtml--submode-crucial-captured-locals mhtml--js-submode)
+        (push (cons 'auto-fill-function
+                    (if (and (boundp 'auto-fill-function) auto-fill-function)
+                        #'js-do-auto-fill
+                      nil))
+              (mhtml--submode-crucial-captured-locals mhtml--js-submode)))
+
+  ;; This mode might be using CC Mode's filling functionality.
+  (c-foreign-init-lit-pos-cache)
+  (add-hook 'before-change-functions #'c-foreign-truncate-lit-pos-cache nil t)
+
   ;; This is sort of a prog-mode as well as a text mode.
   (run-hooks 'prog-mode-hook))
 


> -- Simen

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello, Dmitry.

On Tue, Jun 23, 2020 at 03:02:48 +0300, Dmitry Gutov wrote:
> Hi Alan,

> On 22.06.2020 22:17, Alan Mackenzie wrote:
> > +(defun c-foreign-truncate-lit-pos-cache (beg _end)
> > +  "Truncate CC Mode's literal cache.
> > +
> > +This function should be added to the `before-change-functions'
> > +hook by major modes that use CC Mode's filling functionality
> > +without initializing CC Mode.  Currently (2020-06) these are
> > +js-mode and mhtml-mode."
> > +  (c-truncate-lit-pos-cache beg))

> Could you explain this part?

> Is that literal cache looked up once during filling, or is it used
> multiple times during the execution of c-fill-paragraph?

It's used several times during each filling operation.  This cache
persists between commands, too.

> If the latter (and it does serve as a cache this way), perhaps it
> could be cleared once, at the beginning of c-fill-paragraph, instead
> of adding a runtime cost to every edit?

The cost is tiny.  c-truncate-lit-pos-cache is a defsubst which does
nothing but three copies of

    (setq cache-limit (min beg cache-limit))

.  All the intricacies of manipulating the cache take place whilst it is
being used.

> Or if that's undesirable, js-fill-paragraph could do that.

No, it really has to be in a before-change-functions function, to keep
track of the bound of the valid cache.

> This way, I think it would automatically make it compatible with
> mmm-mode. Or at least more compatible.

Maybe putting the two c-foreign-* functions into mmm-mode would work.

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Dmitry Gutov
On 23.06.2020 11:36, Alan Mackenzie wrote:

>> If the latter (and it does serve as a cache this way), perhaps it
>> could be cleared once, at the beginning of c-fill-paragraph, instead
>> of adding a runtime cost to every edit?
>
> The cost is tiny.  c-truncate-lit-pos-cache is a defsubst which does
> nothing but three copies of
>
>      (setq cache-limit (min beg cache-limit))
>
> .  All the intricacies of manipulating the cache take place whilst it is
> being used.
>
>> Or if that's undesirable, js-fill-paragraph could do that.
>
> No, it really has to be in a before-change-functions function, to keep
> track of the bound of the valid cache.

So it's really fine if it's called from HTML/CSS hunks as well?

And there's no way to just "reset" it to an appropriate value?

>> This way, I think it would automatically make it compatible with
>> mmm-mode. Or at least more compatible.
>
> Maybe putting the two c-foreign-* functions into mmm-mode would work.

mmm-mode is a minor mode, it doesn't always deal with CC Mode.

And its configurations don't usually result in new major modes either.

I wouldn't say it's very hard to make it work, but I don't see a "neat"
way to do either.

Have you considered adding variables that hold the cache to
mhtml--crucial-variable-prefix as well? Would that make it work?



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello, Dmitry.

On Tue, Jun 23, 2020 at 17:23:53 +0300, Dmitry Gutov wrote:
> On 23.06.2020 11:36, Alan Mackenzie wrote:

> >> If the latter (and it does serve as a cache this way), perhaps it
> >> could be cleared once, at the beginning of c-fill-paragraph, instead
> >> of adding a runtime cost to every edit?

> > The cost is tiny.  c-truncate-lit-pos-cache is a defsubst which does
> > nothing but three copies of

> >      (setq cache-limit (min beg cache-limit))

> > .  All the intricacies of manipulating the cache take place whilst it
> > is being used.

> >> Or if that's undesirable, js-fill-paragraph could do that.

> > No, it really has to be in a before-change-functions function, to
> > keep track of the bound of the valid cache.

> So it's really fine if it's called from HTML/CSS hunks as well?

Not only fine, but necessary.  The literal cache contains entries that
record things like "C comment between positions 23 and 130".  If somebody
inserts text before that comment, or inside of it, that cache entry is no
longer valid, and must be invalidated.  Hence the necessity of the
before-change function.

> And there's no way to just "reset" it to an appropriate value?

No.  Not without killing its utility as a cache.

> >> This way, I think it would automatically make it compatible with
> >> mmm-mode. Or at least more compatible.

> > Maybe putting the two c-foreign-* functions into mmm-mode would work.

> mmm-mode is a minor mode, it doesn't always deal with CC Mode.

The question to consider here is whether any sub-mode of mmm-mode uses CC
Mode's comment filling without initialising CC Mode.  js-mode and
mhtml-mode do this.

> And its configurations don't usually result in new major modes either.

> I wouldn't say it's very hard to make it work, but I don't see a "neat"
> way to do either.

OK.

> Have you considered adding variables that hold the cache to
> mhtml--crucial-variable-prefix as well? Would that make it work?

Not without the before-change function, no.  I'm trying to see what the
point of putting these variables into mhtml's crucial variables would be.

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Dmitry Gutov
Hi Alan,

On 23.06.2020 19:28, Alan Mackenzie wrote:

>> So it's really fine if it's called from HTML/CSS hunks as well?
>
> Not only fine, but necessary.  The literal cache contains entries that
> record things like "C comment between positions 23 and 130".  If somebody
> inserts text before that comment, or inside of it, that cache entry is no
> longer valid, and must be invalidated.  Hence the necessity of the
> before-change function.

But isn't CC Mode confused by chunks of text with a totally different
syntax?

>> And there's no way to just "reset" it to an appropriate value?
>
> No.  Not without killing its utility as a cache.

What do you mean? Even if the cache is reset at the beginning of a
function, if the function refers to it multiple times, the first time
should refill the cache, and the rest of the calls will be able to make
use of it properly.

>>>> This way, I think it would automatically make it compatible with
>>>> mmm-mode. Or at least more compatible.
>
>>> Maybe putting the two c-foreign-* functions into mmm-mode would work.
>
>> mmm-mode is a minor mode, it doesn't always deal with CC Mode.
>
> The question to consider here is whether any sub-mode of mmm-mode uses CC
> Mode's comment filling without initialising CC Mode.  js-mode and
> mhtml-mode do this.

js-mode can be one of its submodes. c-mode as well, but none of CC Mode
family of major modes ever worked okay with it, I think.

js-mode mostly works, aside from features like this one.

>> Have you considered adding variables that hold the cache to
>> mhtml--crucial-variable-prefix as well? Would that make it work?
>
> Not without the before-change function, no.  I'm trying to see what the
> point of putting these variables into mhtml's crucial variables would be.

Hopefully, it would make the submode regions inside independent
"islands", so to speak. Each of them having its own cache structure
(used or not).

TBH I'm not sure if mhtml-mode does the save-and-restore dance which
would be necessary for this. mmm-mode does, though.



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello, Dmitry.

On Tue, Jun 23, 2020 at 20:59:22 +0300, Dmitry Gutov wrote:
> Hi Alan,

> On 23.06.2020 19:28, Alan Mackenzie wrote:

> >> So it's really fine if it's called from HTML/CSS hunks as well?

> > Not only fine, but necessary.  The literal cache contains entries
> > that record things like "C comment between positions 23 and 130".  If
> > somebody inserts text before that comment, or inside of it, that
> > cache entry is no longer valid, and must be invalidated.  Hence the
> > necessity of the before-change function.

> But isn't CC Mode confused by chunks of text with a totally different
> syntax?

It might well be, probably is.  But this isn't CC Mode we're talking
about - just a tiny part of its low level functionality, namely the bit
dealing with literals and filling them.

> >> And there's no way to just "reset" it to an appropriate value?

> > No.  Not without killing its utility as a cache.

> What do you mean? Even if the cache is reset at the beginning of a
> function, if the function refers to it multiple times, the first time
> should refill the cache, and the rest of the calls will be able to make
> use of it properly.

That's what I mean.  The cache persists over commands, reducing the
amount of recalculation needed, particularly for fast typing.  Refilling
it from scratch on every keypress would likely make it sluggish.

Anyhow, it works fine at the moment, so why change it?

[ .... ]

> >> mmm-mode is a minor mode, it doesn't always deal with CC Mode.

> > The question to consider here is whether any sub-mode of mmm-mode
> > uses CC Mode's comment filling without initialising CC Mode.  js-mode
> > and mhtml-mode do this.

> js-mode can be one of its submodes. c-mode as well, but none of CC Mode
> family of major modes ever worked okay with it, I think.

Having several major modes in a single buffer has always been problematic
in Emacs.  Personally, I think there needs to be amendments in the
low-level C code to support it properly, but I'm not able to do this work
on my own, and there doesn't seem to be enough enthusiasm on other
people's part to help out.

> js-mode mostly works, aside from features like this one.

With the current patch, comment filling should work fine in js-mode.

> >> Have you considered adding variables that hold the cache to
> >> mhtml--crucial-variable-prefix as well? Would that make it work?

> > Not without the before-change function, no.  I'm trying to see what the
> > point of putting these variables into mhtml's crucial variables would be.

> Hopefully, it would make the submode regions inside independent
> "islands", so to speak. Each of them having its own cache structure
> (used or not).

Ah, OK.  So, buffer positions would be offsets from the island start, or
something like that.

> TBH I'm not sure if mhtml-mode does the save-and-restore dance which
> would be necessary for this. mmm-mode does, though.

mhtml-mode does do saving and restoring of local variables.  I can't
judge how well in comparison with, say, mmm-mode.

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello, Dmitry.

On Wed, Jun 24, 2020 at 02:11:31 +0300, Dmitry Gutov wrote:
> On 23.06.2020 22:17, Alan Mackenzie wrote:

[ .... ]

> >>>> And there's no way to just "reset" it to an appropriate value?

> >>> No.  Not without killing its utility as a cache.

> >> What do you mean? Even if the cache is reset at the beginning of a
> >> function, if the function refers to it multiple times, the first time
> >> should refill the cache, and the rest of the calls will be able to make
> >> use of it properly.

> > That's what I mean.  The cache persists over commands, reducing the
> > amount of recalculation needed, particularly for fast typing.  Refilling
> > it from scratch on every keypress would likely make it sluggish.

> Not on every keypress. Only when js-fill-paragraph is called. One-time
> delay only when required.

But a substantial delay, involving (I think) analysing the code from BOB
each time.  The current working setup has a negligible delay at each
buffer change (and, of course, recalculation of cache entries only when
required).

> > Anyhow, it works fine at the moment, so why change it?

> The above scheme would require fewer references to CC Mode functions
> from outside. js-mode support would automatically transfer to mhtml-mode
> and mmm-mode with associated changes in them necessary.

It sounds like you want to use a facility without initialising it.  This
feels a bit unreasonable.

> One fewer before-change-functions element is also nothing to sneeze at.

There's nothing wrong with having functions in
before/after-change-functions.  It's a standard Emacs programming
technique.

[ .... ]

> >> js-mode mostly works, aside from features like this one.

> > With the current patch, comment filling should work fine in js-mode.

> Above, I meant that js-mode mostly works fine with mmm-mode. And my
> suggestion might make comment filling work there, too. Automatically.

It works automatically at the moment (with the current patch applied).  I
think you're saying again you don't want to be troubled by initialising
it.

> >>>> Have you considered adding variables that hold the cache to
> >>>> mhtml--crucial-variable-prefix as well? Would that make it work?

> >>> Not without the before-change function, no.  I'm trying to see what the
> >>> point of putting these variables into mhtml's crucial variables would be.

> >> Hopefully, it would make the submode regions inside independent
> >> "islands", so to speak. Each of them having its own cache structure
> >> (used or not).

> > Ah, OK.  So, buffer positions would be offsets from the island start, or
> > something like that.

> Not necessarily, but possibly. The key aspect is that the cache inside a
> particular submode is not affected by user actions outside of its
> bounds. Not directly, at least.

Well, when a cached holds buffer positions, something has to give -
either markers need to be used, or an offset from a variable position, or
something.

> But that's an mmm-mode feature anyway.

OK.

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Dmitry Gutov
Hi Alan,

On 24.06.2020 20:43, Alan Mackenzie wrote:

>>> That's what I mean.  The cache persists over commands, reducing the
>>> amount of recalculation needed, particularly for fast typing.  Refilling
>>> it from scratch on every keypress would likely make it sluggish.
>
>> Not on every keypress. Only when js-fill-paragraph is called. One-time
>> delay only when required.
>
> But a substantial delay, involving (I think) analysing the code from BOB
> each time.  The current working setup has a negligible delay at each
> buffer change (and, of course, recalculation of cache entries only when
> required).

I imagine that would not be a significant problem for the rare cases
that fill-paragraph is called in a JS region. Considering most of the
contents in mhtml-mode buffers are not JS code, on average, that should
tilt the scales in favor of parsing lazily, rather than affecting every
character insertion.

>>> Anyhow, it works fine at the moment, so why change it?
>
>> The above scheme would require fewer references to CC Mode functions
>> from outside. js-mode support would automatically transfer to mhtml-mode
>> and mmm-mode with associated changes in them necessary.
>
> It sounds like you want to use a facility without initialising it.  This
> feels a bit unreasonable.

That cache reset at the beginning of js-fill-paragraph could as well
re-initialize the cache.

>> One fewer before-change-functions element is also nothing to sneeze at.
>
> There's nothing wrong with having functions in
> before/after-change-functions.  It's a standard Emacs programming
> technique.

I'm not saying it's a terrible idea, but it has its downsides.

>>>> js-mode mostly works, aside from features like this one.
>
>>> With the current patch, comment filling should work fine in js-mode.
>
>> Above, I meant that js-mode mostly works fine with mmm-mode. And my
>> suggestion might make comment filling work there, too. Automatically.
>
> It works automatically at the moment (with the current patch applied).  I
> think you're saying again you don't want to be troubled by initialising
> it.

It doesn't automatically work in mmm-mode. With my suggestion, it very
likely would.

>>>>>> Have you considered adding variables that hold the cache to
>>>>>> mhtml--crucial-variable-prefix as well? Would that make it work?
>
>>>>> Not without the before-change function, no.  I'm trying to see what the
>>>>> point of putting these variables into mhtml's crucial variables would be.
>
>>>> Hopefully, it would make the submode regions inside independent
>>>> "islands", so to speak. Each of them having its own cache structure
>>>> (used or not).
>
>>> Ah, OK.  So, buffer positions would be offsets from the island start, or
>>> something like that.
>
>> Not necessarily, but possibly. The key aspect is that the cache inside a
>> particular submode is not affected by user actions outside of its
>> bounds. Not directly, at least.
>
> Well, when a cached holds buffer positions, something has to give -
> either markers need to be used, or an offset from a variable position, or
> something.

Makes sense. The alternative would be to index the positions from the
beginning of the submode region. But then the related feature must
respect narrowing, and it would require explicit integration from
mmm-mode and friends.



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello, Dmitry.

On Wed, Jun 24, 2020 at 21:28:09 +0300, Dmitry Gutov wrote:
> Hi Alan,

> On 24.06.2020 20:43, Alan Mackenzie wrote:

[ .... ]

> > But a substantial delay, involving (I think) analysing the code from
> > BOB each time.  The current working setup has a negligible delay at
> > each buffer change (and, of course, recalculation of cache entries
> > only when required).

> I imagine that would not be a significant problem for the rare cases
> that fill-paragraph is called in a JS region. Considering most of the
> contents in mhtml-mode buffers are not JS code, on average, that should
> tilt the scales in favor of parsing lazily, rather than affecting every
> character insertion.

The current patch does parse lazily.  You want to remove the benefit of
using this cache, no matter how small, for reasons I still can't grasp.
This removal will hurt performance, and possibly cause new bugs to solve.

[ .... ]

> > It sounds like you want to use a facility without initialising it.
> > This feels a bit unreasonable.

> That cache reset at the beginning of js-fill-paragraph could as well
> re-initialize the cache.

You're misusing the work "initialize" here.  If you initialise a variable
every time you read it, you might as well not have that variable.

[ .... ]

> >>>> js-mode mostly works, aside from features like this one.

> >>> With the current patch, comment filling should work fine in js-mode.

> >> Above, I meant that js-mode mostly works fine with mmm-mode. And my
> >> suggestion might make comment filling work there, too. Automatically.

> > It works automatically at the moment (with the current patch applied).  I
> > think you're saying again you don't want to be troubled by initialising
> > it.

> It doesn't automatically work in mmm-mode. With my suggestion, it very
> likely would.

It would work fine with the current patch, together with calls to
initialise the mechanism.  What precisely is the problem in mmm-mode?

[ .... ]

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello, Dmitry.

On Thu, Jun 25, 2020 at 19:48:57 +0300, Dmitry Gutov wrote:
> On 25.06.2020 19:33, Alan Mackenzie wrote:

> >> I imagine that would not be a significant problem for the rare cases
> >> that fill-paragraph is called in a JS region. Considering most of the
> >> contents in mhtml-mode buffers are not JS code, on average, that should
> >> tilt the scales in favor of parsing lazily, rather than affecting every
> >> character insertion.

> > The current patch does parse lazily.  You want to remove the benefit of
> > using this cache, no matter how small, for reasons I still can't grasp.

> That's not true. Like you confirmed, c-fill-paragraph refers to that
> cache multiple times. We'll only make sure the cache is reset in the
> beginning.

OK, first of all, I think I was mistaken in saying c-literal-limits is
called several times.  I think it's just called once per filling action.

But if it were called several times, it would likely need to be updated
at every buffer change between calls.

The purpose of this cache is to avoid repeated scanning from BOB.  Your
proposed continual splatting of it would remove the benefit of it
entirely.

> >>> It sounds like you want to use a facility without initialising it.
> >>> This feels a bit unreasonable.

> >> That cache reset at the beginning of js-fill-paragraph could as well
> >> re-initialize the cache.

> > You're misusing the work "initialize" here.  If you initialise a
> > variable every time you read it, you might as well not have that
> > variable.

> Like explained above, not *every* time.

> >> It doesn't automatically work in mmm-mode. With my suggestion, it very
> >> likely would.

> > It would work fine with the current patch, together with calls to
> > initialise the mechanism.  What precisely is the problem in mmm-mode?

> That there is no good place to plug in your new functions.

That would appear to be a deficiency in mmm-mode.

Does mmm-mode not call js-mode when that is one of the submodes?  If it
doesn't, then why not add a general init function-variable/hook/whatever
into which initialisations can be plugged?

> And, in general, to have per-mode before-change-functions contents.

There's no problem with before/after-change-functions.  They're the
canonical way to react to buffer changes.

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Dmitry Gutov
Hi Alan,

On 25.06.2020 21:07, Alan Mackenzie wrote:

>> That's not true. Like you confirmed, c-fill-paragraph refers to that
>> cache multiple times. We'll only make sure the cache is reset in the
>> beginning.
>
> OK, first of all, I think I was mistaken in saying c-literal-limits is
> called several times.  I think it's just called once per filling action.
>
> But if it were called several times, it would likely need to be updated
> at every buffer change between calls.
>
> The purpose of this cache is to avoid repeated scanning from BOB.  Your
> proposed continual splatting of it would remove the benefit of it
> entirely.

That's unfortunate.

Guess the only thing that remains for me here is to express a wish for a
syntax-ppss based design here.

Because mmm-mode knows how to deal with major modes based on it, as a group.

>>> It would work fine with the current patch, together with calls to
>>> initialise the mechanism.  What precisely is the problem in mmm-mode?
>
>> That there is no good place to plug in your new functions.
>
> That would appear to be a deficiency in mmm-mode.
>
> Does mmm-mode not call js-mode when that is one of the submodes?  If it
> doesn't, then why not add a general init function-variable/hook/whatever
> into which initialisations can be plugged?

It does not pick up each and every hook.

If it did, though, it would only call your before-change-functions
inside js-mode regions, but it would have ignored them in HTML and CSS
regions. Which doesn't appear to be what you want anyway.

>> And, in general, to have per-mode before-change-functions contents.
>
> There's no problem with before/after-change-functions.  They're the
> canonical way to react to buffer changes.

They're not very manageable, from mmm's point of view. And like the
current example shows, it's not obvious what to do with such hooks
outside of submode regions of major modes that added them.



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello, Dmitry.

On Thu, Jun 25, 2020 at 21:19:11 +0300, Dmitry Gutov wrote:
> Hi Alan,

> On 25.06.2020 21:07, Alan Mackenzie wrote:

> > The purpose of this cache is to avoid repeated scanning from BOB.
> > Your proposed continual splatting of it would remove the benefit of
> > it entirely.

> That's unfortunate.

Indeed.  Let's assume that keeping it working is a requirement here.

> Guess the only thing that remains for me here is to express a wish for a
> syntax-ppss based design here.

> Because mmm-mode knows how to deal with major modes based on it, as a group.

How about enhancing mmm-mode to handle any major mode, rather than a
restricted subset?

> >>> It would work fine with the current patch, together with calls to
> >>> initialise the mechanism.  What precisely is the problem in mmm-mode?

> >> That there is no good place to plug in your new functions.

> > That would appear to be a deficiency in mmm-mode.

> > Does mmm-mode not call js-mode when that is one of the submodes?  If it
> > doesn't, then why not add a general init function-variable/hook/whatever
> > into which initialisations can be plugged?

> It does not pick up each and every hook.

> If it did, though, it would only call your before-change-functions
> inside js-mode regions, but it would have ignored them in HTML and CSS
> regions. Which doesn't appear to be what you want anyway.

Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?

It's not very nice, but it helps to analyse in the abstract how we
reached the point we are at.  That abstract reason is js-mode using part
of CC Mode without initialising it.  This is bound to lead to trouble,
and it has lead to trouble.

> >> And, in general, to have per-mode before-change-functions contents.

> > There's no problem with before/after-change-functions.  They're the
> > canonical way to react to buffer changes.

> They're not very manageable, from mmm's point of view. And like the
> current example shows, it's not obvious what to do with such hooks
> outside of submode regions of major modes that added them.

Like I said earlier on in the thread, making several major modes in a
buffer work is problematic in Emacs, and we really want better support
from the C core for it.  Here we seem to want "global" and "mode-local"
before-change-functionses.

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Dmitry Gutov
On 25.06.2020 22:13, Alan Mackenzie wrote:

 >> That's unfortunate.

 > Indeed.  Let's assume that keeping it working is a requirement here.

Still, buffers that user mixed modes are usually not so big as some of
the files we have in src/*.c. So even forgoing caching might result in a
satisfying user experience 98% of the time.

>> Guess the only thing that remains for me here is to express a wish for a
>> syntax-ppss based design here.
>
>> Because mmm-mode knows how to deal with major modes based on it, as a group.
>
> How about enhancing mmm-mode to handle any major mode, rather than a
> restricted subset?

I don't know how. before-change-functions don't make it easy.

>> It does not pick up each and every hook.
>
>> If it did, though, it would only call your before-change-functions
>> inside js-mode regions, but it would have ignored them in HTML and CSS
>> regions. Which doesn't appear to be what you want anyway.
>
> Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
> js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?

That would be something every user that configures a submode class using
js-mode have to be aware of. That's not easy to document, or even if we
made sure it's documented, to be sure that users read it.

>>> There's no problem with before/after-change-functions.  They're the
>>> canonical way to react to buffer changes.
>
>> They're not very manageable, from mmm's point of view. And like the
>> current example shows, it's not obvious what to do with such hooks
>> outside of submode regions of major modes that added them.
>
> Like I said earlier on in the thread, making several major modes in a
> buffer work is problematic in Emacs, and we really want better support
> from the C core for it.  Here we seem to want "global" and "mode-local"
> before-change-functionses.

These do seem to be the options: some C core support (though I'm not
clear on the particulars of the proposed design), or switching from
ad-hoc caches to syntax-propertize-function and and associated
syntax-ppss cache.



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Alan Mackenzie
Hello, Dmitry.

On Thu, Jun 25, 2020 at 22:28:17 +0300, Dmitry Gutov wrote:
> On 25.06.2020 22:13, Alan Mackenzie wrote:

>  >> That's unfortunate.

>  > Indeed.  Let's assume that keeping it working is a requirement here.

> Still, buffers that user mixed modes are usually not so big as some of
> the files we have in src/*.c. So even forgoing caching might result in
> a satisfying user experience 98% of the time.

Sluggish performance isn't about "usually" and 98% of the time; it's
about unusual constellations and the other 2%.

[ .... ]

> >> If it did, though, it would only call your before-change-functions
> >> inside js-mode regions, but it would have ignored them in HTML and CSS
> >> regions. Which doesn't appear to be what you want anyway.

> > Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
> > js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?

> That would be something every user that configures a submode class using
> js-mode have to be aware of. That's not easy to document, or even if we
> made sure it's documented, to be sure that users read it.

Are you telling me that mmm-mode couldn't keep a watch out for js-mode,
leaving other libraries untroubled?  Again, the trouble here appears to
arise from using something (a mode) without first initialising it.

> >>> There's no problem with before/after-change-functions.  They're the
> >>> canonical way to react to buffer changes.

> >> They're not very manageable, from mmm's point of view. And like the
> >> current example shows, it's not obvious what to do with such hooks
> >> outside of submode regions of major modes that added them.

> > Like I said earlier on in the thread, making several major modes in a
> > buffer work is problematic in Emacs, and we really want better
> > support from the C core for it.  Here we seem to want "global" and
> > "mode-local" before-change-functionses.

> These do seem to be the options: some C core support (though I'm not
> clear on the particulars of the proposed design), or switching from
> ad-hoc caches to syntax-propertize-function and and associated
> syntax-ppss cache.

The syntax-propertize-function approach is poor design.  It restricts the
use of the syntax-table text property too much.  syntax-ppss has had a
troubled history and doesn't do the right thing in narrowed buffers.  It
advertises itself as a magic wand which does everything, but when you've
been enticed into committing your SW to it, you then find out it's less
than magic, and you've got to call ugly functions by hand at strange
times, and are restricted in how and when you can use it.

--
Alan Mackenzie (Nuremberg, Germany).



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Tom Tromey-4
In reply to this post by Alan Mackenzie
>>>>> "Alan" == Alan Mackenzie <[hidden email]> writes:

Alan> Having several major modes in a single buffer has always been problematic
Alan> in Emacs.  Personally, I think there needs to be amendments in the
Alan> low-level C code to support it properly, but I'm not able to do this work
Alan> on my own, and there doesn't seem to be enough enthusiasm on other
Alan> people's part to help out.

mhtml-mode was a stab at making this work.  It does require some more
changes in Emacs (there's another bug open about font-lock that requires
some changes to the font-lock code, but there are also a few more I
could list), but I didn't come across anything requiring C changes.

What changes are you thinking of?

Dmitry> TBH I'm not sure if mhtml-mode does the save-and-restore dance which
Dmitry> would be necessary for this. mmm-mode does, though.

It does.  Whatever happened to the idea of pulling mmm into Emacs core?
Maybe we could get rid of mhtml-mode.

Tom



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Dmitry Gutov
On 25.06.2020 23:53, Tom Tromey wrote:

> Dmitry> TBH I'm not sure if mhtml-mode does the save-and-restore dance which
> Dmitry> would be necessary for this. mmm-mode does, though.
>
> It does.

When moving between regions?

> Whatever happened to the idea of pulling mmm into Emacs core?
> Maybe we could get rid of mhtml-mode.

It's in ELPA now, which is a good place to be.

I'm maintaining it to keep it usable, but depending on your end goals,
it might require a more significant rewrite. Help welcome.



Reply | Threaded
Open this post in threaded view
|

bug#41897: 28.0.50; JavaScript comment filling with mhtml-mode

Dmitry Gutov
In reply to this post by Alan Mackenzie
On 25.06.2020 23:11, Alan Mackenzie wrote:

> Sluggish performance isn't about "usually" and 98% of the time; it's
> about unusual constellations and the other 2%.

Still, a slow-ish fill-paragraph is nowhere near as bad as, say,
slowdown during typing.

>>> Then why not do in mmm-mode what I'm doing in CC Mode, mhtml-mode and
>>> js-mode, i.e. add ad hoc code to handle precisely the case of js-mode?
>
>> That would be something every user that configures a submode class using
>> js-mode have to be aware of. That's not easy to document, or even if we
>> made sure it's documented, to be sure that users read it.
>
> Are you telling me that mmm-mode couldn't keep a watch out for js-mode,
> leaving other libraries untroubled?  Again, the trouble here appears to
> arise from using something (a mode) without first initialising it.

Sounds like special-casing js-mode, before-change-functions and this
particular function all together. Basically, like a magic constant in
the code.

This is ultimately doable, but I'm not sure how to write a patch for it
which wouldn't leave me feeling dirty after.

>> These do seem to be the options: some C core support (though I'm not
>> clear on the particulars of the proposed design), or switching from
>> ad-hoc caches to syntax-propertize-function and and associated
>> syntax-ppss cache.
>
> The syntax-propertize-function approach is poor design.  It restricts the
> use of the syntax-table text property too much.  syntax-ppss has had a
> troubled history and doesn't do the right thing in narrowed buffers.  It
> advertises itself as a magic wand which does everything, but when you've
> been enticed into committing your SW to it, you then find out it's less
> than magic, and you've got to call ugly functions by hand at strange
> times, and are restricted in how and when you can use it.

Despite certain edge cases, I've had a lot of success with it. Both in
major modes, and in mmm-mode thanks to it.



12