* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes @ 2019-04-22 16:08 Noam Postavsky 2019-04-23 16:09 ` Stefan Monnier 0 siblings, 1 reply; 14+ messages in thread From: Noam Postavsky @ 2019-04-22 16:08 UTC (permalink / raw) To: 35381; +Cc: stefan monnier [-- Attachment #1: Type: text/plain, Size: 677 bytes --] X-Debbugs-CC: Stefan Monnier <monnier@iro.umontreal.ca> Tags: patch 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. This is a regression since Emacs 25.3. 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?). There's also the difference that attribute values are now fontified with font-lock-string-face rather than nxml-attribute-value, though I'm not sure that's worth fixing. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Possible patch --] [-- Type: text/x-diff, Size: 3822 bytes --] From a3d2dbcd0c4e272802a45a7c751877b6982fb257 Mon Sep 17 00:00:00 2001 From: Noam Postavsky <npostavs@gmail.com> Date: Sun, 21 Apr 2019 22:44:50 -0400 Subject: [PATCH] Make nxml-mode recognize single quote attribute again * lisp/nxml/nxml-mode.el (nxml-syntax-propertize): New function. (nxml-tag-syntax-table): New constant. (nxml-mode): Set them as syntax-propertize-function and syntax-ppss-table, respectively. * test/lisp/nxml/nxml-mode-tests.el (nxml-mode-font-lock-quotes): New test. --- lisp/nxml/nxml-mode.el | 22 ++++++++++++++++++++-- test/lisp/nxml/nxml-mode-tests.el | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/lisp/nxml/nxml-mode.el b/lisp/nxml/nxml-mode.el index ab035b927e..94fbe4b781 100644 --- a/lisp/nxml/nxml-mode.el +++ b/lisp/nxml/nxml-mode.el @@ -426,6 +426,24 @@ (defun nxml-parent-document-set (parent-document) (defvar tildify-space-string) (defvar tildify-foreach-region-function) +(defun nxml-syntax-propertize (start end) + "Syntactic keywords for `nxml-mode'." + (goto-char start) + (sgml-syntax-propertize-inside end) + (funcall + (syntax-propertize-rules + sgml-syntax-propertize-rules + ;; Like the " rule in `sgml-syntax-propertize-rules', but for '. + ("'" (0 (if (prog1 (zerop (car (syntax-ppss (match-beginning 0)))) + (goto-char (match-end 0))) + (string-to-syntax "."))))) + start end)) + +(defconst nxml-tag-syntax-table + (let ((table (make-syntax-table sgml-tag-syntax-table))) + (modify-syntax-entry ?\' "\"'" table)) + "Syntax table used to parse XML tags.") + ;;;###autoload (define-derived-mode nxml-mode text-mode "nXML" ;; We use C-c C-i instead of \\[nxml-balanced-close-start-tag-inline] @@ -517,8 +535,8 @@ (define-derived-mode nxml-mode text-mode "nXML" (with-silent-modifications (nxml-with-invisible-motion (nxml-scan-prolog))))) - (setq-local syntax-ppss-table sgml-tag-syntax-table) - (setq-local syntax-propertize-function #'sgml-syntax-propertize) + (setq-local syntax-ppss-table nxml-tag-syntax-table) + (setq-local syntax-propertize-function #'nxml-syntax-propertize) (add-hook 'change-major-mode-hook #'nxml-cleanup nil t) ;; Emacs 23 handles the encoding attribute on the xml declaration 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 -- 2.11.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-04-22 16:08 bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes Noam Postavsky @ 2019-04-23 16:09 ` Stefan Monnier 2019-04-24 1:28 ` Noam Postavsky 0 siblings, 1 reply; 14+ messages in thread From: Stefan Monnier @ 2019-04-23 16:09 UTC (permalink / raw) To: Noam Postavsky; +Cc: 35381 > 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. 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? > 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. > There's also the difference that attribute values are now fontified with > font-lock-string-face rather than nxml-attribute-value, though I'm not > sure that's worth fixing. I don't see where else font-lock-string-face would be used in nxml-mode, so I see no need to use a special face like nxml-attribute-value for attributed values. > +(defun nxml-syntax-propertize (start end) > + "Syntactic keywords for `nxml-mode'." > + (goto-char start) > + (sgml-syntax-propertize-inside end) > + (funcall > + (syntax-propertize-rules > + sgml-syntax-propertize-rules > + ;; Like the " rule in `sgml-syntax-propertize-rules', but for '. > + ("'" (0 (if (prog1 (zerop (car (syntax-ppss (match-beginning 0)))) > + (goto-char (match-end 0))) > + (string-to-syntax "."))))) > + start end)) > + > +(defconst nxml-tag-syntax-table > + (let ((table (make-syntax-table sgml-tag-syntax-table))) > + (modify-syntax-entry ?\' "\"'" table)) > + "Syntax table used to parse XML tags.") > + > ;;;###autoload > (define-derived-mode nxml-mode text-mode "nXML" > ;; We use C-c C-i instead of \\[nxml-balanced-close-start-tag-inline] > @@ -517,8 +535,8 @@ (define-derived-mode nxml-mode text-mode "nXML" > (with-silent-modifications > (nxml-with-invisible-motion > (nxml-scan-prolog))))) > - (setq-local syntax-ppss-table sgml-tag-syntax-table) > - (setq-local syntax-propertize-function #'sgml-syntax-propertize) > + (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. Stefan ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-04-23 16:09 ` Stefan Monnier @ 2019-04-24 1:28 ` Noam Postavsky 2019-04-24 14:52 ` Stefan Monnier 0 siblings, 1 reply; 14+ messages in thread From: Noam Postavsky @ 2019-04-24 1:28 UTC (permalink / raw) To: Stefan Monnier; +Cc: 35381 [-- Attachment #1: Type: text/plain, Size: 2597 bytes --] 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. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: patch --] [-- Type: text/x-diff, Size: 4405 bytes --] 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-04-24 1:28 ` Noam Postavsky @ 2019-04-24 14:52 ` Stefan Monnier 2019-04-25 2:24 ` Noam Postavsky 0 siblings, 1 reply; 14+ messages in thread From: Stefan Monnier @ 2019-04-24 14:52 UTC (permalink / raw) To: Noam Postavsky; +Cc: 35381 > 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)* "'" Thanks for checking. > 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 Ah, yes, that makes sense. > 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. Great, thanks. > --- 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: I think this "disadvantages" part of the docstring is out of date. Can you update it while you're at it? Also, we probably want to mark this var as obsolete. Stefan ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-04-24 14:52 ` Stefan Monnier @ 2019-04-25 2:24 ` Noam Postavsky 2019-04-25 12:47 ` Stefan Monnier 0 siblings, 1 reply; 14+ messages in thread From: Noam Postavsky @ 2019-04-25 2:24 UTC (permalink / raw) To: Stefan Monnier; +Cc: 35381 Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >> +(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: > > I think this "disadvantages" part of the docstring is out of date. > Can you update it while you're at it? > > Also, we probably want to mark this var as obsolete. I'm not sure about this, is the disadvantage of the ?- also out of date? There's some FIXME on the "--" rule of sgml-syntax-propertize-rules, -;; 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 '(?\" ?\') "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: - - ?\\\" Makes \" in text start a string. - ?\\=' Makes \\=' in text start a string. - ?- Makes -- in text start a comment. - -When only one of ?\\\" or ?\\=' are included, \"\\='\" or \\='\"\\=', as can be found in -DTDs, start a string. To partially avoid this problem this also makes these -self insert as named entities depending on `sgml-quick-keys'. +The supported characters are ?\\\", ?\\=', and ?-. Including ?- has the problem of affecting dashes that have nothing to do with comments, so we normally turn it off.") ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-04-25 2:24 ` Noam Postavsky @ 2019-04-25 12:47 ` Stefan Monnier 2019-04-26 11:32 ` Noam Postavsky 0 siblings, 1 reply; 14+ messages in thread From: Stefan Monnier @ 2019-04-25 12:47 UTC (permalink / raw) To: Noam Postavsky; +Cc: 35381 > I'm not sure about this, is the disadvantage of the ?- also out > of date? If not, we should just fix it with appropriate syntax-propertize tricks. > Including ?- has the problem of affecting dashes that have nothing to do > with comments, so we normally turn it off.") Actually, I think the issue with -- comments is not just that they should not be treated as such when appearing outside tags, but that even within tags it's often not to be considered as comments. E.g. according to my flaky memory --...-- is not considered as a comment in HTML nor in XML (both inside and outside of tags). Stefan ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-04-25 12:47 ` Stefan Monnier @ 2019-04-26 11:32 ` Noam Postavsky 2019-04-29 21:23 ` Stefan Monnier 0 siblings, 1 reply; 14+ messages in thread From: Noam Postavsky @ 2019-04-26 11:32 UTC (permalink / raw) To: Stefan Monnier; +Cc: 35381 Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >> I'm not sure about this, is the disadvantage of the ?- also out >> of date? > > If not, we should just fix it with appropriate syntax-propertize tricks. > >> Including ?- has the problem of affecting dashes that have nothing to do >> with comments, so we normally turn it off.") > > Actually, I think the issue with -- comments is not just that they > should not be treated as such when appearing outside tags, but that even > within tags it's often not to be considered as comments. > > E.g. according to my flaky memory --...-- is not considered as a comment > in HTML nor in XML (both inside and outside of tags). XML says comments should always be <!--...--> where ... contains no "--". https://www.w3.org/TR/xml/#sec-comments For SGML, it seems that --...-- is a comment within DTD definitions. Otherwise only <!-- starts a comment, but "--" can toggle interpretation of angle brackets and other special characters. But browsers may or may not handle the toggling thing correctly, so the recommendation for HTML is to stick <!--...--> just like XML. It's all rather confusing. http://www.flightlab.com/~joe/sgml/comments.html https://www.w3.org/TR/WD-html40-970917/intro/sgmltut.html -;; 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. +;; TODO: Add syntax-propertize rules to handle "--" properly, and +;; eventually get rid of this variable altogether. (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: +The supported characters are ?\\\", ?\\=', and ?-. - ?\\\" Makes \" in text start a string. - ?\\=' Makes \\=' in text start a string. - ?- Makes -- in text start a comment. - -When only one of ?\\\" or ?\\=' are included, \"\\='\" or \\='\"\\=', as can be found in -DTDs, start a string. To partially avoid this problem this also makes these -self insert as named entities depending on `sgml-quick-keys'. - -Including ?- has the problem of affecting dashes that have nothing to do -with comments, so we normally turn it off.") +Including ?- makes double dashes into comment delimiters, but +they are really only supposed to delimit comments within DTD +definitions. So we normally turn it off.") ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-04-26 11:32 ` Noam Postavsky @ 2019-04-29 21:23 ` Stefan Monnier 2019-05-05 3:52 ` Noam Postavsky 0 siblings, 1 reply; 14+ messages in thread From: Stefan Monnier @ 2019-04-29 21:23 UTC (permalink / raw) To: Noam Postavsky; +Cc: 35381 > XML says comments should always be <!--...--> where ... contains no > "--". That's also what I remember. > For SGML, it seems that --...-- is a comment within DTD definitions. But only within the DTD definition, so in practice I think it means it's not a comment in SGML files. > Otherwise only <!-- starts a comment, but "--" can toggle interpretation > of angle brackets and other special characters. But browsers may or may > not handle the toggling thing correctly, so the recommendation for HTML > is to stick <!--...--> just like XML. It's all rather confusing. Confusing is a pretty good description. From where I stand, I think we simply shouldn't bother to try and handle "--" specially. At least not before someone comes with a concrete use-case. Stefan ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-04-29 21:23 ` Stefan Monnier @ 2019-05-05 3:52 ` Noam Postavsky 2019-05-05 13:06 ` Stefan Monnier 2019-05-09 11:45 ` Noam Postavsky 0 siblings, 2 replies; 14+ messages in thread From: Noam Postavsky @ 2019-05-05 3:52 UTC (permalink / raw) To: Stefan Monnier; +Cc: 35381 [-- Attachment #1: Type: text/plain, Size: 264 bytes --] Stefan Monnier <monnier@IRO.UMontreal.CA> writes: > From where I stand, I think we simply shouldn't bother to try and > handle "--" specially. At least not before someone comes with a > concrete use-case. Agreed (I updated the commentary a bit to say that). [-- Attachment #2: patch --] [-- Type: text/plain, Size: 5665 bytes --] From 6985354ef612cf0e2db1987e51e5a9f06b396ad9 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 | 34 +++++++++++++++------------------- test/lisp/nxml/nxml-mode-tests.el | 20 ++++++++++++++++++++ test/lisp/textmodes/sgml-mode-tests.el | 1 - 3 files changed, 35 insertions(+), 20 deletions(-) diff --git a/lisp/textmodes/sgml-mode.el b/lisp/textmodes/sgml-mode.el index 50b2077ef4..128e58810e 100644 --- a/lisp/textmodes/sgml-mode.el +++ b/lisp/textmodes/sgml-mode.el @@ -100,24 +100,20 @@ (defcustom sgml-mode-hook nil :group 'sgml :type 'hook) -;; 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 '(?\") +;; The official handling of "--" is complicated in SGML, and +;; historically not well supported by browser HTML parsers. +;; Recommendations for writing HTML comments is to use <!--...--> +;; (where ... doesn't contain "--") to avoid the complications +;; altogether (XML goes even further by requiring this in the spec). +;; So there is probably no need to handle it "correctly". +(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: +The supported characters are ?\\\", ?\\=', and ?-. - ?\\\" Makes \" in text start a string. - ?\\=' Makes \\=' in text start a string. - ?- Makes -- in text start a comment. - -When only one of ?\\\" or ?\\=' are included, \"\\='\" or \\='\"\\=', as can be found in -DTDs, start a string. To partially avoid this problem this also makes these -self insert as named entities depending on `sgml-quick-keys'. - -Including ?- has the problem of affecting dashes that have nothing to do -with comments, so we normally turn it off.") +Including ?- makes double dashes into comment delimiters, but +they are really only supposed to delimit comments within DTD +definitions. So we normally turn it off.") (defvar sgml-quick-keys nil "Use <, >, &, /, SPC and `sgml-specials' keys \"electrically\" when non-nil. @@ -351,12 +347,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 [-- Attachment #3: Type: text/plain, Size: 405 bytes --] I think that plus the patches for #32003 and #32897 should cover the nxml fixes I'm planning for emacs-26. I'll wait for another few days and then push. https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Fix-nxml-get-inside-Bug-32003.patch;bug=32003;msg=8;att=1 https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Disable-extra-display-of-10-in-nxml-mode-Bug-32897.patch;att=1;msg=8;bug=32897 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-05-05 3:52 ` Noam Postavsky @ 2019-05-05 13:06 ` Stefan Monnier 2019-05-05 13:49 ` Noam Postavsky 2019-05-09 11:45 ` Noam Postavsky 1 sibling, 1 reply; 14+ messages in thread From: Stefan Monnier @ 2019-05-05 13:06 UTC (permalink / raw) To: Noam Postavsky; +Cc: 35381 > I think that plus the patches for #32003 and #32897 should cover the > nxml fixes I'm planning for emacs-26. I'll wait for another few days > and then push. LGTM. > https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Fix-nxml-get-inside-Bug-32003.patch;bug=32003;msg=8;att=1 You might like to mention in nxml-get-inside what "normal strings" are used for (AFAIK that's used exclusively for attributes). > https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Disable-extra-display-of-10-in-nxml-mode-Bug-32897.patch;att=1;msg=8;bug=32897 - (when nxml-char-ref-extra-display + (when (and ;; Displaying literal newline is unhelpful. + (not eql n ?\n) + nxml-char-ref-extra-display) There's a bunch of parens missing. Stefan ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-05-05 13:06 ` Stefan Monnier @ 2019-05-05 13:49 ` Noam Postavsky 2019-05-05 14:20 ` martin rudalics 0 siblings, 1 reply; 14+ messages in thread From: Noam Postavsky @ 2019-05-05 13:49 UTC (permalink / raw) To: Stefan Monnier; +Cc: 35381 Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >> https://debbugs.gnu.org/cgi/bugreport.cgi?filename=0001-Disable-extra-display-of-10-in-nxml-mode-Bug-32897.patch;att=1;msg=8;bug=32897 > > - (when nxml-char-ref-extra-display > + (when (and ;; Displaying literal newline is unhelpful. > + (not eql n ?\n) > + nxml-char-ref-extra-display) > > There's a bunch of parens missing. How do you mean? ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-05-05 13:49 ` Noam Postavsky @ 2019-05-05 14:20 ` martin rudalics 2019-05-05 14:36 ` Noam Postavsky 0 siblings, 1 reply; 14+ messages in thread From: martin rudalics @ 2019-05-05 14:20 UTC (permalink / raw) To: Noam Postavsky, Stefan Monnier; +Cc: 35381 > How do you mean? I suppose Stefan means to write (not (eql n ?\n)) instead of (not eql n ?\n) martin ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-05-05 14:20 ` martin rudalics @ 2019-05-05 14:36 ` Noam Postavsky 0 siblings, 0 replies; 14+ messages in thread From: Noam Postavsky @ 2019-05-05 14:36 UTC (permalink / raw) To: martin rudalics; +Cc: 35381, Stefan Monnier martin rudalics <rudalics@gmx.at> writes: >> How do you mean? > > I suppose Stefan means to write > > (not (eql n ?\n)) > > instead of > > (not eql n ?\n) Oh right, I had actually fixed that locally already, but forgot to update the bug thread. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes 2019-05-05 3:52 ` Noam Postavsky 2019-05-05 13:06 ` Stefan Monnier @ 2019-05-09 11:45 ` Noam Postavsky 1 sibling, 0 replies; 14+ messages in thread From: Noam Postavsky @ 2019-05-09 11:45 UTC (permalink / raw) To: Stefan Monnier; +Cc: 35381 tags 35381 fixed close 35381 26.3 quit > I think that plus the patches for #32003 and #32897 should cover the > nxml fixes I'm planning for emacs-26. I'll wait for another few days > and then push. Done. 7dab3ee7ab 2019-05-09T06:42:40-04:00 "Recognize single quote attribute values in nxml and sgml (Bug#35381)" https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=7dab3ee7ab54b3c2e7bc24170376054786c01d6f ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-05-09 11:45 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-22 16:08 bug#35381: 26.2; nxml-mode doesn't fontify attributes in single quotes Noam Postavsky 2019-04-23 16:09 ` Stefan Monnier 2019-04-24 1:28 ` Noam Postavsky 2019-04-24 14:52 ` Stefan Monnier 2019-04-25 2:24 ` Noam Postavsky 2019-04-25 12:47 ` Stefan Monnier 2019-04-26 11:32 ` Noam Postavsky 2019-04-29 21:23 ` Stefan Monnier 2019-05-05 3:52 ` Noam Postavsky 2019-05-05 13:06 ` Stefan Monnier 2019-05-05 13:49 ` Noam Postavsky 2019-05-05 14:20 ` martin rudalics 2019-05-05 14:36 ` Noam Postavsky 2019-05-09 11:45 ` Noam Postavsky
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).