bug#6267: 23.1; invalid XML in nested comments produced by comment-region

bug#6267: 23.1; invalid XML in nested comments produced by comment-region

[Patrik Hägglund H]

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

In xml mode, mark the following:

  <!-- foo -->
  <bar/>

and execute comment-region:

  <!-- <\!-- foo -\-> -->
  <!-- <bar/> -->

This is invalid XML because '--' is not allowed inside comments. See http://www.w3.org/TR/REC-xml/#sec-comments. (This is enforced by for example XML::Simple in perl 5.10.)

For example, as an alternative, the following is valid:

  <!-- <!-\- foo -\-> -->
  <!-- <bar/> -->

The offending code seems to be comment-quote-nested in newcomment.el. Should this code be changed, or the code for the xml mode, or both?


In GNU Emacs 23.1.1 (i686-pc-linux-gnu, GTK+ Version 2.8.11)
 of 2009-09-08 on deacxl001
Windowing system distributor `The Cygwin/X Project', version 11.0.10800000
configured using `configure  '--prefix=/app/emacs/23.1/LMWP2''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US
  value of $XMODIFIERS: nil
  locale-coding-system: iso-latin-1-unix
  default-enable-multibyte-characters: t

Major mode: XML

Minor modes in effect:
  tooltip-mode: t
  tool-bar-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-g g 3 0 6 <return> <down> C-SPC <down> <down> M-;
<up> <up> <right> <right> <right> <right> <backspace>
C-a C-SPC <down> <down> M-; C-x C-s M-x r e p o r t
- b u g <return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Mark set [2 times]
Mark activated
Saving file /vobs/radio_testbeds_adv/platform/testapps/build/template/dul_a/ulma_1/make/template.xml...
Wrote /vobs/radio_testbeds_adv/platform/testapps/build/template/dul_a/ulma_1/make/template.xml


[-- Attachment #2: Type: text/html, Size: 3704 bytes --]

bug#6267: 23.1; invalid XML in nested comments produced by comment-region

[Ivan Andrus]

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

I have a patch for this (and the duplicate bug #20001) that I would like
some feedback on from a regular developer.  I think the approach is
reasonable, but any feedback would be welcome.  I have commmit access,
so I can commit it, but I've probably forgotten some important piece.

It's not user facing (except fixing a bug), so I believe I don't need a
NEWS entry.  I'm not sure if or where it should be documented in the
manual.  There doesn't seem to be documentation for
`comment-region-function' (at least I didn't find any with C-h S) which
seemed the closest variable that I could think of.

Thanks,
Ivan


Allow major-modes full control over quoting nested comments

* newcomment.el (comment-quote-nested-function): New variable.
(comment-quote-nested-default): New function.
(comment-quote-nested): Use `comment-quote-nested-function'.
---
 lisp/newcomment.el | 66
++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 22 deletions(-)

diff --git a/lisp/newcomment.el b/lisp/newcomment.el
index 60f35c8..4c01742 100644
--- a/lisp/newcomment.el
+++ b/lisp/newcomment.el
@@ -179,6 +179,11 @@ comments always start in column zero.")
   "Non-nil if nested comments should be quoted.
 This should be locally set by each major mode if needed.")

+(defvar comment-quote-nested-function #'comment-quote-nested-default
+  "Function to quote nested comments in a region.
+It takes the same arguments as `comment-quote-nested-default',
+and is called with the buffer narrowed to a single comment.")
+
 (defvar comment-continue nil
   "Continuation string to insert for multiline comments.
 This string will be added at the beginning of each line except the very
@@ -412,28 +417,45 @@ function should first call this function explicitly."
 If UNP is non-nil, unquote nested comment markers."
   (setq cs (comment-string-strip cs t t))
   (setq ce (comment-string-strip ce t t))
-  (when (and comment-quote-nested (> (length ce) 0))
-    (let ((re (concat (comment-quote-re ce unp)
-       "\\|" (comment-quote-re cs unp))))
-      (goto-char (point-min))
-      (while (re-search-forward re nil t)
- (goto-char (match-beginning 0))
- (forward-char 1)
- (if unp (delete-char 1) (insert "\\"))
- (when (= (length ce) 1)
-   ;; If the comment-end is a single char, adding a \ after that
-   ;; "first" char won't deactivate it, so we turn such a CE
-   ;; into !CS.  I.e. for pascal, we turn } into !{
-   (if (not unp)
-       (when (string= (match-string 0) ce)
- (replace-match (concat "!" cs) t t))
-     (when (and (< (point-min) (match-beginning 0))
-        (string= (buffer-substring (1- (match-beginning 0))
-   (1- (match-end 0)))
- (concat "!" cs)))
-       (backward-char 2)
-       (delete-char (- (match-end 0) (match-beginning 0)))
-       (insert ce))))))))
+  (when (and comment-quote-nested
+      comment-quote-nested-function
+      (> (length ce) 0))
+    (funcall comment-quote-nested-function cs ce unp)))
+
+(defun comment-quote-nested-default (cs ce unp)
+  "Quote comment delimiters in the buffer.
+It expects to be called with the buffer narrowed to a single comment.
+It is used as a default for `comment-quote-nested-function'.
+
+The arguments CS and CE are regular expressions matching comment
+starting and ending delimiters respectively.
+
+If UNP is non-nil, comments are unquoted instead.
+
+To quote the delimiters, a \\ is inserted after the first
+character of CS or CE.  If CE is a single character it will
+change CE into !CS."
+  (let ((re (concat (comment-quote-re ce unp)
+     "\\|" (comment-quote-re cs unp))))
+    (goto-char (point-min))
+    (while (re-search-forward re nil t)
+      (goto-char (match-beginning 0))
+      (forward-char 1)
+      (if unp (delete-char 1) (insert "\\"))
+      (when (= (length ce) 1)
+ ;; If the comment-end is a single char, adding a \ after that
+ ;; "first" char won't deactivate it, so we turn such a CE
+ ;; into !CS.  I.e. for pascal, we turn } into !{
+ (if (not unp)
+     (when (string= (match-string 0) ce)
+       (replace-match (concat "!" cs) t t))
+   (when (and (< (point-min) (match-beginning 0))
+      (string= (buffer-substring (1- (match-beginning 0))
+ (1- (match-end 0)))
+       (concat "!" cs)))
+     (backward-char 2)
+     (delete-char (- (match-end 0) (match-beginning 0)))
+     (insert ce)))))))

 ;;;;
 ;;;; Navigation
-- 
2.5.2



Properly quote nested xml comments (Bug#6267) (Bug#20001)

* nxml-mode.el (nxml-comment-quote-nested): New function
(nxml-mode): Set comment-quote-nested-function
---
 lisp/nxml/nxml-mode.el | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/lisp/nxml/nxml-mode.el b/lisp/nxml/nxml-mode.el
index 6c5c85b..dc151a3 100644
--- a/lisp/nxml/nxml-mode.el
+++ b/lisp/nxml/nxml-mode.el
@@ -546,6 +546,8 @@ Many aspects this mode can be customized using
   (setq comment-end-skip "[ \t\r\n]*-->")
   (make-local-variable 'comment-line-break-function)
   (setq comment-line-break-function 'nxml-newline-and-indent)
+  (make-local-variable 'comment-quote-nested-function)
+  (setq comment-quote-nested-function 'nxml-comment-quote-nested)
   (use-local-map nxml-mode-map)
   (save-excursion
     (save-restriction
@@ -1350,6 +1352,18 @@ of the inserted start-tag or nil if none was
inserted."
      start-tag-indent)))))
     inserted-start-tag-pos))

+(defun nxml-comment-quote-nested (cs ce unp)
+  "Quote nested comments in buffer.
+See `comment-quote-nested-function' for more information.")
+  (goto-char (point-min))
+  (save-match-data
+    (while (re-search-forward "-[\\]*-" nil t)
+      (goto-char (match-beginning 0))
+      (forward-char 1)
+      (if unp
+   (delete-char 1)
+ (insert "\\")))))
+
 ;;; Indentation

 (defun nxml-indent-line ()
-- 
2.5.2

[-- Attachment #2: Type: text/html, Size: 8310 bytes --]

bug#20001: bug#6267: 23.1; invalid XML in nested comments produced by comment-region

[Stefan Monnier]

The approach looks fine, thank you.

> +(defvar comment-quote-nested-function #'comment-quote-nested-default
[...]
> +  (when (and comment-quote-nested
> +      comment-quote-nested-function

comment-quote-nested-function will always be a function so it should
never be nil, so there's no need to test it here.

> +The arguments CS and CE are regular expressions matching comment
> +starting and ending delimiters respectively.

AFAICT this is not true.  They're just strings, not regexps.

> +  (make-local-variable 'comment-quote-nested-function)
> +  (setq comment-quote-nested-function 'nxml-comment-quote-nested)

I know the rest of the file doesn't use it, but please in new code use:

     (setq-local comment-quote-nested-function #'nxml-comment-quote-nested)


-- Stefan