Fuco1 / smartparens

Minor mode for Emacs that deals with parens pairs and tries to be smart about it.
GNU General Public License v3.0
1.82k stars 195 forks source link

XML sexp parsing doesn't correctly handle empty elements #1124

Open Bob131 opened 2 years ago

Bob131 commented 2 years ago

Expected behavior

Given a buffer with the following contents (where | indicates the point)

<tag>
  <tag/>
  |
</tag>

Executing M-: (sp-get-enclosing-sexp) RET should return (:beg 1 :end 25 :op "<tag>" :cl "</tag>" :prefix "" :suffix "").

Actual behavior

It returns (:beg 9 :end 25 :op "<tag/>" :cl "</tag>" :prefix "" :suffix "").

Environment & version information

Bob131 commented 2 years ago

I dug into this a bit, and it seems like the issue is partly the search loop in sp-get-sgml-tag and partly that sp--sgml-opening-p is not quite correct. This rough diff fixes the issue:

diff --git a/smartparens.el b/smartparens.el
index fd58064..2000ef6 100644
--- a/smartparens.el
+++ b/smartparens.el
@@ -5409,6 +5409,19 @@ and newline."
 (defun sp--sgml-opening-p (tag)
   (not (equal "/" (substring tag 1 2))))

+(defun sp--looking-at-sgml-opening-p ()
+  (let ((sexp (sp-get-paired-expression)))
+    (and sexp
+         (equal (plist-get sexp :op) "<")
+         (equal (plist-get sexp :cl) ">")
+         (not (= (char-after (1+ (plist-get sexp :beg)))
+                 ?/))
+         (not (= (char-before (1- (plist-get sexp :end)))
+                 ?/)))))
+
+(defun sp--looking-at-sgml-closing-p ()
+  (looking-at-p "</"))
+
 (defun sp--sgml-ignore-tag (tag)
   "Return non-nil if tag should be ignored in search, nil otherwise."
   (member tag '("!--" "!DOCTYPE")))
@@ -5446,9 +5459,11 @@ and newline."
               (goto-char (match-end 0))))
             (while (> depth 0)
               (if (funcall search-fn needle nil t)
-                  (if (sp--sgml-opening-p (match-string 0))
-                      (if forward (setq depth (1+ depth)) (setq depth (1- depth)))
-                    (if forward (setq depth (1- depth)) (setq depth (1+ depth))))
+                  (cond
+                   ((sp--looking-at-sgml-opening-p)
+                    (if forward (setq depth (1+ depth)) (setq depth (1- depth))))
+                   ((sp--looking-at-sgml-closing-p)
+                    (if forward (setq depth (1- depth)) (setq depth (1+ depth)))))
                 (setq depth -1)))
             (if (eq depth -1)
                 (progn (sp-message :no-matching-tag) nil)
Bob131 commented 2 years ago

Hmm, that doesn't work in the case that the initial match at https://github.com/Fuco1/smartparens/blob/37f77bf2e2199be9fe27e981317b02cfd0e8c70e/smartparens.el#L5424 is an empty element. I guess sp-get-sgml-tag needs to be changed to test when the point is inside an empty element first, and if not search forward for a tag that isn't an empty element.