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

[Alan Mackenzie]

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 <acm@muc.de> 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).