* bug#71772: nxml-mode: avoid producing invalid XML/HTML w/ multline comment style
@ 2024-06-25 9:58 Konstantin Kharlamov
[not found] ` <handler.71772.B.171930958114742.ack@debbugs.gnu.org>
0 siblings, 1 reply; 6+ messages in thread
From: Konstantin Kharlamov @ 2024-06-25 9:58 UTC (permalink / raw)
To: 71772
[-- 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 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) "Â ")
- "Â " " "))
- ;; 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 if possible (e.g. XHTML)?
+ tildify-space-string (if (equal (decode-coding-string
+ (encode-coding-string "Â " buffer-file-coding-system)
+ buffer-file-coding-system) "Â ")
+ "Â " " ")
+ ;; 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#71772: Acknowledgement (nxml-mode: avoid producing invalid XML/HTML w/ multline comment style)
[not found] ` <handler.71772.B.171930958114742.ack@debbugs.gnu.org>
@ 2024-06-26 4:27 ` Konstantin Kharlamov
2024-06-28 7:34 ` bug#71772: [PATCH] bug#71772: avoid producing invalid XML/HTML w/ multline comment style Konstantin Kharlamov
0 siblings, 1 reply; 6+ messages in thread
From: Konstantin Kharlamov @ 2024-06-26 4:27 UTC (permalink / raw)
To: 71772
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#71772: [PATCH] bug#71772: avoid producing invalid XML/HTML w/ multline comment style
2024-06-26 4:27 ` bug#71772: Acknowledgement (nxml-mode: avoid producing invalid XML/HTML w/ multline comment style) Konstantin Kharlamov
@ 2024-06-28 7:34 ` Konstantin Kharlamov
2024-07-20 21:34 ` Konstantin Kharlamov
0 siblings, 1 reply; 6+ messages in thread
From: Konstantin Kharlamov @ 2024-06-28 7:34 UTC (permalink / raw)
To: 71772
(just changing the title to make it visible that it's a patch)
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#71772: [PATCH] bug#71772: avoid producing invalid XML/HTML w/ multline comment style
2024-06-28 7:34 ` bug#71772: [PATCH] bug#71772: avoid producing invalid XML/HTML w/ multline comment style Konstantin Kharlamov
@ 2024-07-20 21:34 ` Konstantin Kharlamov
2024-07-25 7:34 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: Konstantin Kharlamov @ 2024-07-20 21:34 UTC (permalink / raw)
To: 71772
Gentle ping. This is a simple bugfix and a pre-release will be in a few
days, so would be nice to have it there.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#71772: [PATCH] bug#71772: avoid producing invalid XML/HTML w/ multline comment style
2024-07-20 21:34 ` Konstantin Kharlamov
@ 2024-07-25 7:34 ` Eli Zaretskii
2024-07-25 9:02 ` Stefan Kangas
0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2024-07-25 7:34 UTC (permalink / raw)
To: Konstantin Kharlamov, Stefan Kangas; +Cc: 71772
> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
> Date: Sun, 21 Jul 2024 00:34:22 +0300
>
> Gentle ping. This is a simple bugfix and a pre-release will be in a few
> days, so would be nice to have it there.
Stefan, would you please look into this?
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#71772: [PATCH] bug#71772: avoid producing invalid XML/HTML w/ multline comment style
2024-07-25 7:34 ` Eli Zaretskii
@ 2024-07-25 9:02 ` Stefan Kangas
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Kangas @ 2024-07-25 9:02 UTC (permalink / raw)
To: Eli Zaretskii, Konstantin Kharlamov; +Cc: 71772
close 71772 30.1
thanks
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Konstantin Kharlamov <Hi-Angel@yandex.ru>
>> Date: Sun, 21 Jul 2024 00:34:22 +0300
>>
>> Gentle ping. This is a simple bugfix and a pre-release will be in a few
>> days, so would be nice to have it there.
>
> Stefan, would you please look into this?
The first patch looks good to me and seems safe. So I've installed it
on emacs-30 as commit 082300300fe.
The second patch seems to be mostly stylistic, and I think we do prefer
the explicit `setq-local's there? So I'm not installing that for now.
I don't really have a strong opinion though, so if anyone feels
differently then please install it on master.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-07-25 9:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-25 9:58 bug#71772: nxml-mode: avoid producing invalid XML/HTML w/ multline comment style Konstantin Kharlamov
[not found] ` <handler.71772.B.171930958114742.ack@debbugs.gnu.org>
2024-06-26 4:27 ` bug#71772: Acknowledgement (nxml-mode: avoid producing invalid XML/HTML w/ multline comment style) Konstantin Kharlamov
2024-06-28 7:34 ` bug#71772: [PATCH] bug#71772: avoid producing invalid XML/HTML w/ multline comment style Konstantin Kharlamov
2024-07-20 21:34 ` Konstantin Kharlamov
2024-07-25 7:34 ` Eli Zaretskii
2024-07-25 9:02 ` Stefan Kangas
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).