Subject: Re: bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes

From: Noam Postavsky <[email-masked]>
Date: Sat, 04 May 2019 23:52:16 -0400

Stefan Monnier <[email-masked]> writes:

> From where I stand, I think we simply shouldn't bother to try and
> handle "--" specially.  At least not before someone comes with a
> concrete use-case.

Agreed (I updated the commentary a bit to say that).  

>From 6985354ef612cf0e2db1987e51e5a9f06b396ad9 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <[email-masked]>
Date: Sun, 21 Apr 2019 22:44:50 -0400
Subject: [PATCH] Recognize single quote attribute values in nxml and sgml
 (Bug#35381)

* lisp/textmodes/sgml-mode.el (sgml-specials): Add single quote.
(sgml-syntax-propertize-rules): Handle single quote.
* test/lisp/nxml/nxml-mode-tests.el (nxml-mode-font-lock-quotes): New
test.
* test/lisp/textmodes/sgml-mode-tests.el
(sgml-delete-tag-bug-8203-should-not-delete-apostrophe): Now passes.
---
 lisp/textmodes/sgml-mode.el            | 34 +++++++++++++++-------------------
 test/lisp/nxml/nxml-mode-tests.el      | 20 ++++++++++++++++++++
 test/lisp/textmodes/sgml-mode-tests.el |  1 -
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 50b2077ef4..128e58810e 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -100,24 +100,20 @@ (defcustom sgml-mode-hook nil
   :group 'sgml
   :type 'hook)
 
-;; As long as Emacs's syntax can't be complemented with predicates to context
-;; sensitively confirm the syntax of characters, we have to live with this
-;; kludgy kind of tradeoff.
-(defvar sgml-specials '(?\")
+;; The official handling of "--" is complicated in SGML, and
+;; historically not well supported by browser HTML parsers.
+;; Recommendations for writing HTML comments is to use <!--...-->
+;; (where ... doesn't contain "--") to avoid the complications
+;; altogether (XML goes even further by requiring this in the spec).
+;; So there is probably no need to handle it "correctly".
+(defvar sgml-specials '(?\" ?\')
   "List of characters that have a special meaning for SGML mode.
 This list is used when first loading the `sgml-mode' library.
-The supported characters and potential disadvantages are:
+The supported characters are ?\\\", ?\\=', and ?-.
 
-  ?\\\"	Makes \" in text start a string.
-  ?\\='	Makes \\=' in text start a string.
-  ?-	Makes -- in text start a comment.
-
-When only one of ?\\\" or ?\\=' are included, \"\\='\" or \\='\"\\=', as can be found in
-DTDs, start a string.  To partially avoid this problem this also makes these
-self insert as named entities depending on `sgml-quick-keys'.
-
-Including ?- has the problem of affecting dashes that have nothing to do
-with comments, so we normally turn it off.")
+Including ?- makes double dashes into comment delimiters, but
+they are really only supposed to delimit comments within DTD
+definitions.  So we normally turn it off.")
 
 (defvar sgml-quick-keys nil
   "Use <, >, &, /, SPC and `sgml-specials' keys \"electrically\" when non-nil.
@@ -351,12 +347,12 @@ (eval-and-compile
      ("--[ \t\n]*\\(>\\)" (1 "> b"))
      ("\\(<\\)[?!]" (1 (prog1 "|>"
                          (sgml-syntax-propertize-inside end))))
-     ;; Double quotes outside of tags should not introduce strings.
+     ;; 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 ".")))))))
+     ("[\"']" (0 (if (prog1 (zerop (car (syntax-ppss (match-beginning 0))))
+                       (goto-char (match-end 0)))
+                     (string-to-syntax ".")))))))
 
 (defun sgml-syntax-propertize (start end)
   "Syntactic keywords for `sgml-mode'."
diff --git a/test/lisp/nxml/nxml-mode-tests.el b/test/lisp/nxml/nxml-mode-tests.el
index 57a731ad18..92744be619 100644
--- a/test/lisp/nxml/nxml-mode-tests.el
+++ b/test/lisp/nxml/nxml-mode-tests.el
@@ -58,5 +58,25 @@ (ert-deftest nxml-balanced-close-start-tag-inline ()
     (nxml-balanced-close-start-tag-inline)
     (should (equal (buffer-string) "<a><b c=\"\"></b></a>"))))
 
+(ert-deftest nxml-mode-font-lock-quotes ()
+  (with-temp-buffer
+    (nxml-mode)
+    (insert "<x a=\"dquote attr\" b='squote attr'>\"dquote text\"'squote text'</x>")
+    (font-lock-ensure)
+    (let ((squote-txt-pos (search-backward "squote text"))
+          (dquote-txt-pos (search-backward "dquote text"))
+          (squote-att-pos (search-backward "squote attr"))
+          (dquote-att-pos (search-backward "dquote attr")))
+      ;; Just make sure that each quote uses the same face for quoted
+      ;; attribute values, and a different face for quoted text
+      ;; outside tags.  Don't test `font-lock-string-face' vs
+      ;; `nxml-attribute-value' here.
+      (should (equal (get-text-property squote-att-pos 'face)
+                     (get-text-property dquote-att-pos 'face)))
+      (should (equal (get-text-property squote-txt-pos 'face)
+                     (get-text-property dquote-txt-pos 'face)))
+      (should-not (equal (get-text-property squote-txt-pos 'face)
+                         (get-text-property dquote-att-pos 'face))))))
+
 (provide 'nxml-mode-tests)
 ;;; nxml-mode-tests.el ends here
diff --git a/test/lisp/textmodes/sgml-mode-tests.el b/test/lisp/textmodes/sgml-mode-tests.el
index 20b5e27ff5..7318a667b3 100644
--- a/test/lisp/textmodes/sgml-mode-tests.el
+++ b/test/lisp/textmodes/sgml-mode-tests.el
@@ -125,7 +125,6 @@ (ert-deftest sgml-delete-tag-should-signal-error-when-deleting-too-much ()
      (should (string= content (buffer-string))))))
 
 (ert-deftest sgml-delete-tag-bug-8203-should-not-delete-apostrophe ()
-  :expected-result :failed
   (sgml-with-content
    "<title>Winter is comin'</title>"
    (sgml-delete-tag 1)
-- 
2.11.0

I think that plus the patches for #32003 and #32897 should cover the
nxml fixes I'm planning for emacs-26.  I'll wait for another few days
and then push.

https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Fix-nxml-get-inside-Bug-32003.patch;bug=32003;msg=8;att=1
https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Disable-extra-display-of-10-in-nxml-mode-Bug-32897.patch;att=1;msg=8;bug=32897