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

[Noam Postavsky]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> In an nxml-mode buffer, put the following text:
>>
>> <x a="foo" b='bar'/>
>>
>> Only "foo" is fontified in as a string, while bar is not.
>
> IIRC this is technically correct because IIRC in XML (contrary to SGML)
> only "..."  is allowed and not '...'.  Don't take my word for it, tho,
> it's just a vague recollection.

Well, I was pretty sure that XML allows both quotes, but just to make
things definitive, https://www.w3.org/TR/xml/#NT-AttValue:

    [10]    AttValue       ::= '"' ([^<&"] | Reference)* '"'
                            |  "'" ([^<&'] | Reference)* "'"

I found this for SGML http://xml.coverpages.org/sgmlsyn/sgmlsyn.htm#P34:

    [34] attribute value literal =

        ( lit , "
        replaceable character data [46] *,
        lit ) | "
        ( lita , '
        replaceable character data [46] *,
        lita ) '

> This said, it's probably a good idea to understand '...' in nxml-mode,
> since there are various circumstances where you may want to use
> nxml-mode to edit files in a format that's more permissive than strict XML.
>
>> This is a regression since Emacs 25.3.
>
> Even more reason to allow '...'.
> Have you investigated the source of the regression?

I didn't actually checkout and compile before+after, but I'm pretty sure
it's [56e1097584], same as Bug#32003.

[56e1097584]: 2016-01-16 15:03:42 -0500
  lisp/nxml: Use syntax-tables for comments
  
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=56e1097584c13f2b6db85592769db1c6c36e9419

>> The patch below seems solves problem, though I'm not entirely sure
>> about it: there are some confusing comments in sgml-mode.el about
>> drawbacks and tradeoffs of recognizing single quotes that I'm not
>> really following (are they applicable to SGML only, and not XML?).
>
> IIRC the comments aren't related to the issue of accepting '...' itself,
> but to some of the side-effects of doing it naively in cases such as
>
>     <em>That's right!</em>
>
> but we're no so naive any more, so I believe that we can support this
> without the downsides.

>> +  (setq-local syntax-ppss-table nxml-tag-syntax-table)
>> +  (setq-local syntax-propertize-function #'nxml-syntax-propertize)
>
> Hmm... I think it would be better to change `sgml-syntax-propertize` so
> it does what we need.  After all, it's more important to support '...'
> for SGML  than for XML.

s/more/equally/, but otherwise yes.  Patching sgml-mode is shorter and
even fixes Bug#8203 as well.  Still good for emacs-26 I hope, since this
is for an (nxml-mode) regression in 26.1.

>From 4288bf689b9dcdf32c7074d54e76c34ff115dea9 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
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            | 10 +++++-----
 test/lisp/nxml/nxml-mode-tests.el      | 20 ++++++++++++++++++++
 test/lisp/textmodes/sgml-mode-tests.el |  1 -
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el
index 50b2077ef4..0eaad1a1ed 100644
--- a/lisp/textmodes/sgml-mode.el
+++ b/lisp/textmodes/sgml-mode.el
@@ -103,7 +103,7 @@ (defcustom sgml-mode-hook nil
 ;; 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 '(?\")
+(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:
@@ -351,12 +351,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