bug#71772: nxml-mode: avoid producing invalid XML/HTML w/ multline comment style

bug#71772: nxml-mode: avoid producing invalid XML/HTML w/ multline comment style

[Konstantin Kharlamov]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

After having set `comment-style` to 'multi-line in nxml-mode, I found
out it tries to pad comments with `!--`. The details why that happens
are described in the first patch. The two hyphens are invalid both in
XML¹ and HTML², so the patch makes it stop produce such padding.

The second patch is just a simple refactoring to reduce the amount of
code, but feel free to ignore it if you don't like it.

-----------------

1:
https://softwareengineering.stackexchange.com/questions/198481/why-cant-an-xml-comment-contain-two-hyphens#:~:text=Double%20dash%20indicates%20a%20change,does%20not%20end%20the%20comment
.
2:
https://stackoverflow.com/questions/26600092/why-are-double-hyphens-invalid-in-html-comments

[-- Attachment #2: 1.patch --]
[-- Type: text/x-patch, Size: 6011 bytes --]

From 17f42d41df8a7277e194e1cf9b72a84e3a84c10c Mon Sep 17 00:00:00 2001
From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
Date: Tue, 25 Jun 2024 12:26:12 +0300
Subject: [PATCH 1/2] Don't produce invalid XML with multi-line commenting
 style

Both XML and HTML forbid double hyphens inside comments. However,
nxml-mode was using a `!--' as a comment padding if `comment-style'
was set to any of the styles that supposed to add padding. This infix
was auto-derived due to `comment-continue' being nil. To fix that set
`comment-continue' explicitly. It's unclear what padding should be
used, but from looking at other editors it seems they don't typically
add padding in XML, so let's be simple for now and just set
`comment-continue' to empty string.

* lisp/nxml/nxml-mode.el (nxml-mode): make `comment-continue' a
buffer-local variable set to empty string.
---
 lisp/nxml/nxml-mode.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/nxml/nxml-mode.el b/lisp/nxml/nxml-mode.el
index 02391dc6968..a58d3f3c75f 100644
--- a/lisp/nxml/nxml-mode.el
+++ b/lisp/nxml/nxml-mode.el
@@ -531,6 +531,7 @@ nxml-mode
   (setq-local comment-end-skip "[ \t\r\n]*-->")
   (setq-local comment-line-break-function #'nxml-newline-and-indent)
   (setq-local comment-quote-nested-function #'nxml-comment-quote-nested)
+  (setq-local comment-continue "") ; avoid double-hyphens as a padding
   (save-excursion
     (save-restriction
       (widen)
-- 
2.45.2


From 76ff289a30f46c441a258c9aa133783ef0059136 Mon Sep 17 00:00:00 2001
From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
Date: Tue, 25 Jun 2024 12:45:10 +0300
Subject: [PATCH 2/2] nxml-mode: replace dozens of setq-local with a single one

* /lisp/nxml/nxml-mode.el (nxml-mode): place separate `setq-local'
statements under a single one.
---
 lisp/nxml/nxml-mode.el | 64 +++++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/lisp/nxml/nxml-mode.el b/lisp/nxml/nxml-mode.el
index a58d3f3c75f..05c684b0238 100644
--- a/lisp/nxml/nxml-mode.el
+++ b/lisp/nxml/nxml-mode.el
@@ -502,43 +502,43 @@ nxml-mode
 Many aspects this mode can be customized using
 \\[customize-group] nxml RET."
   ;; (kill-all-local-variables)
-  ;; If encoding does not allow non-break space character, use reference.
-  ;; FIXME: This duplicates code from sgml-mode, perhaps derive from it?
-  ;; FIXME: Perhaps use &nbsp; if possible (e.g. XHTML)?
-  (setq-local tildify-space-string
-              (if (equal (decode-coding-string
-                          (encode-coding-string " " buffer-file-coding-system)
-                          buffer-file-coding-system) " ")
-                  " " "&#160;"))
-  ;; FIXME: Use the fact that we're parsing the document already
-  ;; rather than using regex-based filtering.
-  (setq-local tildify-foreach-region-function
-              (apply-partially 'tildify-foreach-ignore-environments
-                               '(("<! *--" . "-- *>") ("<" . ">"))))
-  (setq-local mode-line-process '((nxml-degraded "/degraded")))
-  ;; We'll determine the fill prefix ourselves
-  (setq-local adaptive-fill-mode nil)
-  (setq-local forward-sexp-function #'nxml-forward-balanced-item)
-  (setq-local indent-line-function #'nxml-indent-line)
-  (setq-local fill-paragraph-function #'nxml-do-fill-paragraph)
-  ;; Comment support
-  ;; This doesn't seem to work too well;
-  ;; I think we should probably roll our own nxml-comment-dwim function.
-  (setq-local comment-indent-function #'nxml-indent-line)
-  (setq-local comment-start "<!--")
-  (setq-local comment-start-skip "<!--[ \t\r\n]*")
-  (setq-local comment-end "-->")
-  (setq-local comment-end-skip "[ \t\r\n]*-->")
-  (setq-local comment-line-break-function #'nxml-newline-and-indent)
-  (setq-local comment-quote-nested-function #'nxml-comment-quote-nested)
-  (setq-local comment-continue "") ; avoid double-hyphens as a padding
+  (setq-local
+   ;; If encoding does not allow non-break space character, use reference.
+   ;; FIXME: This duplicates code from sgml-mode, perhaps derive from it?
+   ;; FIXME: Perhaps use &nbsp; if possible (e.g. XHTML)?
+   tildify-space-string (if (equal (decode-coding-string
+                                    (encode-coding-string " " buffer-file-coding-system)
+                                    buffer-file-coding-system) " ")
+                            " " "&#160;")
+   ;; FIXME: Use the fact that we're parsing the document already
+   ;; rather than using regex-based filtering.
+   tildify-foreach-region-function (apply-partially
+                                    'tildify-foreach-ignore-environments
+                                    '(("<! *--" . "-- *>") ("<" . ">")))
+   mode-line-process '((nxml-degraded "/degraded"))
+   ;; We'll determine the fill prefix ourselves
+   adaptive-fill-mode nil
+   forward-sexp-function #'nxml-forward-balanced-item
+   indent-line-function #'nxml-indent-line
+   fill-paragraph-function #'nxml-do-fill-paragraph
+   ;; Comment support
+   ;; This doesn't seem to work too well;
+   ;; I think we should probably roll our own nxml-comment-dwim function.
+   comment-indent-function #'nxml-indent-line
+   comment-start "<!--"
+   comment-start-skip "<!--[ \t\r\n]*"
+   comment-end "-->"
+   comment-end-skip "[ \t\r\n]*-->"
+   comment-line-break-function #'nxml-newline-and-indent
+   comment-quote-nested-function #'nxml-comment-quote-nested
+   comment-continue "" ; avoid double-hyphens as a padding
+   syntax-ppss-table sgml-tag-syntax-table
+   syntax-propertize-function #'nxml-syntax-propertize)
   (save-excursion
     (save-restriction
       (widen)
       (with-silent-modifications
 	(nxml-scan-prolog))))
-  (setq-local syntax-ppss-table sgml-tag-syntax-table)
-  (setq-local syntax-propertize-function #'nxml-syntax-propertize)
   (add-function :filter-return (local 'filter-buffer-substring-function)
                 #'nxml--buffer-substring-filter)
   (add-hook 'change-major-mode-hook #'nxml-cleanup nil t)
-- 
2.45.2

bug#71772: Acknowledgement (nxml-mode: avoid producing invalid XML/HTML w/ multline comment style)

[Konstantin Kharlamov]

Oops, the first reference link has "text selection" in the URL, that
wasn't intentional. It was added by search engine, I didn't see it was
there when I copied it.

bug#71772: [PATCH] bug#71772: avoid producing invalid XML/HTML w/ multline comment style

[Konstantin Kharlamov]

(just changing the title to make it visible that it's a patch)