Re: html, css, and js modes working together

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <address@hidden> writes:

Tom> This patch changes the html, css, and js modes to work together a bit.

Here's the second version of this patch.  I'd appreciate comments once
again.

A big thanks to everyone for their help so far.

This version addresses most of the review comments.  It also cleans up
the implementation quite a bit (IMO anyway) and adds a couple new
features: a mode-line highlighter (e.g., it says "HTML+JS" in a <script>
element), and it uses the sub-mode's keymap when point is in a sub-mode
region.

I didn't implement Stefan's suggestion for capturing local variables.  I
did put a special indentation wrapper into css-mode.el to make this area
a bit cleaner.  But, I may still implement his idea; not sure yet.

It turns out there are a bunch of other variables that would be nice to
capture and set when point is in a sub-mode region.  Here I'm thinking
of comment-*, electric characters, font-lock-keywords ... so one
solution that suggests itself is to pull these settings out of the
define-derived-mode invocations and into something that can be reused
from mhtml-mode.

I called the new mode "mhtml-mode"; "m" for "multi".  Naming isn't
always my forte, I'd appreciate suggestions.  I still don't really
understand why a separate new mode is desirable, but I caved to it, and
it does at least solve the circular dependency problem.

One thing I noticed while digging around is that at least align.el looks
specifically for 'html-mode.  This spot (or spots, I didn't look for
more) should be changed to use derived-mode-p.  It's on my to-do list...

A few other to-do items:

* Tests

* Font-lock, as discussed.

* In a sub-mode, disable flyspell, or perhaps rather enable
  flyspell-prog-mode instead.

* ... which brings up the funny issue that mhtml-mode is both a text-
  and a prog-mode and arguably should derive from both.

* imenu and which-func support

* comment-* variables should change depending on the current region

* Electric characters should change depending on region

* ... your feature here?

FWIW I am not trying to tackle multiple major modes in full generality.
I just want to be able to edit mochitests in Firefox with the built-in
Emacs modes.

It's not clear how much of the above is really a requirement.  My
feeling is that improvements can go in even though the result doesn't
implement every possible multi-major-mode feature.

Tom> * Not sure but maybe I also need to define
Tom>   syntax-propertize-extend-region-functions now?

I looked into this and I think the default of
syntax-propertize-wholelines is sufficient.

Stefan> Hmm.. if we want to obey prog-indentation-context,
Stefan> don't we want something like
[...]

In the end I think not.  I think the region parts of that variable are
solely for prog-widen; it's up to the caller instead to simply not set
the variable if this behavior isn't needed.

Tom

diff --git a/lisp/emacs-lisp/smie.el b/lisp/emacs-lisp/smie.el
index 4d02b75..3cb70b5 100644
--- a/lisp/emacs-lisp/smie.el
+++ b/lisp/emacs-lisp/smie.el
@@ -123,6 +123,8 @@
 
 (eval-when-compile (require 'cl-lib))
 
+(require 'prog-mode)
+
 (defgroup smie nil
   "Simple Minded Indentation Engine."
   :group 'languages)
@@ -1455,7 +1457,7 @@ smie-indent-bob
   ;; Start the file at column 0.
   (save-excursion
     (forward-comment (- (point)))
-    (if (bobp) 0)))
+    (if (bobp) (prog-first-column))))
 
 (defun smie-indent-close ()
   ;; Align close paren with opening paren.
@@ -1838,17 +1840,25 @@ smie-auto-fill
         (funcall do-auto-fill)))))
 
 
-(defun smie-setup (grammar rules-function &rest keywords)
-  "Setup SMIE navigation and indentation.
-GRAMMAR is a grammar table generated by `smie-prec2->grammar'.
-RULES-FUNCTION is a set of indentation rules for use on `smie-rules-function'.
-KEYWORDS are additional arguments, which can use the following keywords:
-- :forward-token FUN
-- :backward-token FUN"
+(defmacro smie-with-rules (spec &rest body)
+  "Temporarily set up SMIE indentation and evaluate BODY.
+SPEC is of the form (GRAMMAR RULES-FUNCTION &rest KEYWORDS); see `smie-setup'.
+BODY is evaluated with the relevant SMIE variables temporarily bound."
+  (declare (indent 1))
+  `(smie-funcall-with-rules (list ,@spec) (lambda () . ,body)))
+
+(defun smie-funcall-with-rules (spec fun)
+  (let ((smie-rules-function smie-rules-function)
+        (smie-grammar smie-grammar)
+        (forward-sexp-function forward-sexp-function)
+        (smie-forward-token-function smie-forward-token-function)
+        (smie-backward-token-function smie-backward-token-function))
+    (smie--basic-setup (car spec) (cadr spec) (cddr spec))
+    (funcall fun)))
+
+(defun smie--basic-setup (grammar rules-function keywords)
   (setq-local smie-rules-function rules-function)
   (setq-local smie-grammar grammar)
-  (setq-local indent-line-function #'smie-indent-line)
-  (add-function :around (local 'normal-auto-fill-function) #'smie-auto-fill)
   (setq-local forward-sexp-function #'smie-forward-sexp-command)
   (while keywords
     (let ((k (pop keywords))
@@ -1858,7 +1868,18 @@ smie-setup
          (set (make-local-variable 'smie-forward-token-function) v))
         (`:backward-token
          (set (make-local-variable 'smie-backward-token-function) v))
-        (_ (message "smie-setup: ignoring unknown keyword %s" k)))))
+        (_ (message "smie-setup: ignoring unknown keyword %s" k))))))
+
+(defun smie-setup (grammar rules-function &rest keywords)
+  "Setup SMIE navigation and indentation.
+GRAMMAR is a grammar table generated by `smie-prec2->grammar'.
+RULES-FUNCTION is a set of indentation rules for use on `smie-rules-function'.
+KEYWORDS are additional arguments, which can use the following keywords:
+- :forward-token FUN
+- :backward-token FUN"
+  (smie--basic-setup grammar rules-function keywords)
+  (setq-local indent-line-function #'smie-indent-line)
+  (add-function :around (local 'normal-auto-fill-function) #'smie-auto-fill)
   (let ((ca (cdr (assq :smie-closer-alist grammar))))
     (when ca
       (setq-local smie-closer-alist ca)
diff --git a/lisp/files.el b/lisp/files.el
index b7d1048..77c1e41 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2422,7 +2422,7 @@ auto-mode-alist
    (lambda (elt)
      (cons (purecopy (car elt)) (cdr elt)))
    `(;; do this first, so that .html.pl is Polish html, not Perl
-     ("\\.[sx]?html?\\(\\.[a-zA-Z_]+\\)?\\'" . html-mode)
+     ("\\.[sx]?html?\\(\\.[a-zA-Z_]+\\)?\\'" . mhtml-mode)
      ("\\.svgz?\\'" . image-mode)
      ("\\.svgz?\\'" . xml-mode)
      ("\\.x[bp]m\\'" . image-mode)
@@ -2784,8 +2784,8 @@ magic-fallback-mode-alist
                comment-re "*"
                "\\(?:!DOCTYPE[ \t\r\n]+[^>]*>[ \t\r\n]*<[ \t\r\n]*" comment-re 
"*\\)?"
                "[Hh][Tt][Mm][Ll]"))
-     . html-mode)
-    ("<!DOCTYPE[ \t\r\n]+[Hh][Tt][Mm][Ll]" . html-mode)
+     . mhtml-mode)
+    ("<!DOCTYPE[ \t\r\n]+[Hh][Tt][Mm][Ll]" . mhtml-mode)
     ;; These two must come after html, because they are more general:
     ("<\\?xml " . xml-mode)
     (,(let* ((incomment-re "\\(?:[^-]\\|-[^-]\\)")
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index e42e014..aab5bd6 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -53,6 +53,7 @@
 (require 'moz nil t)
 (require 'json nil t)
 (require 'sgml-mode)
+(require 'prog-mode)
 
 (eval-when-compile
   (require 'cl-lib)
@@ -2102,7 +2103,7 @@ js--proper-indentation
 
           ((js--continued-expression-p)
            (+ js-indent-level js-expr-indent-offset))
-          (t 0))))
+          (t (prog-first-column)))))
 
 ;;; JSX Indentation
 
diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 19746c6..d837756 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -970,6 +970,13 @@ css-completion-at-point
                   (list sel-beg sel-end))
               ,(completion-table-merge prop-table sel-table)))))))
 
+(defun css-advertized-indent-line ()
+  "A wrapper for `smie-indent-line' that first installs the SMIE rules."
+  (smie-with-rules (css-smie-grammar #'css-smie-rules
+                                     :forward-token #'css-smie--forward-token
+                                     :backward-token 
#'css-smie--backward-token)
+    (smie-indent-line)))
+
 ;;;###autoload
 (define-derived-mode css-mode prog-mode "CSS"
   "Major mode to edit Cascading Style Sheets."
diff --git a/lisp/textmodes/mhtml-mode.el b/lisp/textmodes/mhtml-mode.el
new file mode 100644
index 0000000..0eaea56
--- /dev/null
+++ b/lisp/textmodes/mhtml-mode.el
@@ -0,0 +1,143 @@
+;;; mhtml-mode.el --- HTML editing mode that handles CSS and JS -*- 
lexical-binding:t -*-
+
+;; Copyright (C) 2017 Free Software Foundation, Inc.
+
+;; Keywords: wp, hypermedia, comm, languages
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(eval-and-compile
+  (require 'sgml-mode)
+  (require 'smie))
+(require 'js)
+(require 'css-mode)
+(require 'prog-mode)
+
+(cl-defstruct mhtml--submode
+  ;; Name of this mode.
+  name
+  ;; HTML end tag.
+  end-tag
+  ;; Syntax table.
+  syntax-table
+  ;; Propertize function.
+  propertize
+  ;; Indentation function.
+  indenter
+  ;; Keymap.
+  keymap)
+
+(defconst mhtml--css-submode
+  (make-mhtml--submode :name "CSS"
+                       :end-tag "</style>"
+                       :syntax-table css-mode-syntax-table
+                       :propertize css-syntax-propertize-function
+                       :indenter #'css-advertized-indent-line
+                       :keymap css-mode-map))
+
+(defconst mhtml--js-submode
+  (make-mhtml--submode :name "JS"
+                       :end-tag "</script>"
+                       :syntax-table js-mode-syntax-table
+                       :propertize #'js-syntax-propertize
+                       :indenter #'js-indent-line
+                       :keymap js-mode-map))
+
+(defun mhtml--submode-lighter ()
+  "Mode-line lighter indicating the current submode."
+  (let ((submode (get-text-property (point) 'mhtml-submode)))
+    (if submode
+        (mhtml--submode-name submode)
+      "")))
+
+(defun mhtml--syntax-propertize-submode (submode end)
+  (save-excursion
+    (when (search-forward (mhtml--submode-end-tag submode) end t)
+      (setq end (match-beginning 0))))
+  (set-text-properties (point) end
+                       (list 'mhtml-submode submode
+                             'syntax-table (mhtml--submode-syntax-table 
submode)
+                             ;; We want local-map here so that we act
+                             ;; more like the sub-mode and don't
+                             ;; override minor mode maps.
+                             'local-map (mhtml--submode-keymap submode)
+                             'cursor-sensor-functions
+                             (list (lambda (_window _old-point _action)
+                                     (force-mode-line-update)))))
+  (funcall (mhtml--submode-propertize submode) (point) end)
+  (goto-char end))
+
+(defun mhtml-syntax-propertize (start end)
+  (goto-char start)
+  (when (get-text-property (point) 'mhtml-submode)
+    (mhtml--syntax-propertize-submode (get-text-property (point) 
'mhtml-submode)
+                                      end))
+  (funcall
+   (syntax-propertize-rules
+    ("<style.*?>"
+     (0 (ignore
+         (goto-char (match-end 0))
+         (mhtml--syntax-propertize-submode mhtml--css-submode end))))
+    ("<script.*?>"
+     (0 (ignore
+         (goto-char (match-end 0))
+         (mhtml--syntax-propertize-submode mhtml--js-submode end))))
+    sgml-syntax-propertize-rules)
+   ;; Make sure to handle the situation where
+   ;; mhtml--syntax-propertize-submode moved point.
+   (point) end))
+
+(defun mhtml-indent-line ()
+  "Indent the current line as HTML, JS, or CSS, according to its context."
+  (interactive)
+  (let ((submode (save-excursion
+                   (back-to-indentation)
+                   (get-text-property (point) 'mhtml-submode))))
+    (if submode
+        (save-restriction
+          (let* ((region-start (previous-single-property-change (point)
+                                                                
'mhtml-submode))
+                 (base-indent (save-excursion
+                                (goto-char region-start)
+                                (sgml-calculate-indent))))
+            (narrow-to-region region-start (point-max))
+            (let ((prog-indentation-context (list base-indent
+                                                  (cons (point-min) nil)
+                                                  nil)))
+              (funcall (mhtml--submode-indenter submode)))))
+      ;; HTML.
+      (sgml-indent-line))))
+
+;;;###autoload
+(define-derived-mode mhtml-mode html-mode
+  '((sgml-xml-mode "XHTML+" "HTML+") (:eval (mhtml--submode-lighter)))
+  "Major mode based on `html-mode', but works with embedded JS and CSS.
+
+Code inside a <script> element is indented using the rules from
+`js-mode'; and code inside a <style> element is indented using
+the rules from `css-mode'."
+  (cursor-sensor-mode)
+  (setq-local indent-line-function #'mhtml-indent-line)
+  (setq-local parse-sexp-lookup-properties t)
+  (setq-local syntax-propertize-function #'mhtml-syntax-propertize)
+  (add-hook 'syntax-propertize-extend-region-functions
+            #'syntax-propertize-multiline 'append 'local))
+
+(provide 'mhtml-mode)
+
+;;; mhtml-mode.el ends here
diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index e148b06..8ad7cfb 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -341,19 +341,23 @@ sgml-font-lock-keywords-2
 (defvar sgml-font-lock-keywords sgml-font-lock-keywords-1
   "Rules for highlighting SGML code.  See also `sgml-tag-face-alist'.")
 
+(eval-and-compile
+  (defconst sgml-syntax-propertize-rules
+    (syntax-propertize-precompile-rules
+     ;; Use the `b' style of comments to avoid interference with the -- ... --
+     ;; comments recognized when `sgml-specials' includes ?-.
+     ;; FIXME: beware of <!--> blabla <!--> !!
+     ("\\(<\\)!--" (1 "< b"))
+     ("--[ \t\n]*\\(>\\)" (1 "> b"))
+     ;; Double quotes outside of tags should not introduce strings.
+     ;; Be careful to call `syntax-ppss' on a position before the one we're
+     ;; going to change, so as not to need to flush the data we just computed.
+     ("\"" (0 (if (prog1 (zerop (car (syntax-ppss (match-beginning 0))))
+                    (goto-char (match-end 0)))
+                  (string-to-syntax ".")))))))
+
 (defconst sgml-syntax-propertize-function
-  (syntax-propertize-rules
-   ;; Use the `b' style of comments to avoid interference with the -- ... --
-   ;; comments recognized when `sgml-specials' includes ?-.
-  ;; FIXME: beware of <!--> blabla <!--> !!
-   ("\\(<\\)!--" (1 "< b"))
-    ("--[ \t\n]*\\(>\\)" (1 "> b"))
-    ;; Double quotes outside of tags should not introduce strings.
-    ;; Be careful to call `syntax-ppss' on a position before the one we're
-    ;; going to change, so as not to need to flush the data we just computed.
-    ("\"" (0 (if (prog1 (zerop (car (syntax-ppss (match-beginning 0))))
-                   (goto-char (match-end 0)))
-           (string-to-syntax ".")))))
+  (syntax-propertize-rules sgml-syntax-propertize-rules)
   "Syntactic keywords for `sgml-mode'.")
 
 ;; internal
@@ -1284,13 +1288,24 @@ sgml-tag-text-p
       (let ((pps (parse-partial-sexp start end 2)))
        (and (= (nth 0 pps) 0))))))
 
+(defun sgml--find-<>-backward (limit)
+  "Search backward for a '<' or '>' character.
+The character must have open or close syntax.
+Returns t if found, nil otherwise."
+  (catch 'found
+    (while (re-search-backward "[<>]" limit 'move)
+      ;; If this character has "open" or "close" syntax, then we've
+      ;; found the one we want.
+      (when (memq (syntax-class (syntax-after (point))) '(4 5))
+        (throw 'found t)))))
+
 (defun sgml-parse-tag-backward (&optional limit)
   "Parse an SGML tag backward, and return information about the tag.
 Assume that parsing starts from within a textual context.
 Leave point at the beginning of the tag."
   (catch 'found
     (let (tag-type tag-start tag-end name)
-      (or (re-search-backward "[<>]" limit 'move)
+      (or (sgml--find-<>-backward limit)
          (error "No tag found"))
       (when (eq (char-after) ?<)
        ;; Oops!! Looks like we were not in a textual context after all!.