unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: [patch] use font-lock
       [not found] <200805231711.30830.danc@merrillpress.com>
@ 2008-05-23 21:52 ` Lennart Borgman (gmail)
  2008-05-23 22:24   ` [emacs-nxml-mode] " Daniel Colascione
                     ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Lennart Borgman (gmail) @ 2008-05-23 21:52 UTC (permalink / raw)
  To: emacs-nxml-mode, Daniel Colascione; +Cc: Eric M. Ludlam, Emacs Devel

Daniel Colascione wrote:
> 
> 
> I've converted nxml to font-lock. I used the existing fontification 
> machinery
> and put it inside a cc-mode-style matcher. Efficiency and output are the
> same, but:
> 
> 1) hi-lock-mode works now
> 2) all conventional font-locking functions work as expected. For 
> example, you
> can turn fontification on and off with M-x font-lock-mode.
> 3) font-lock-add-keywords DTRT
> 4) multiple-major-modes modes should be able to use nXML fontification now,
> though this remains untested

Hi Daniel,

This sounds very interesting. As you might know in nXhtml I am using 
nxml-mode with mumamo (which is a framework for multiple major modes in 
a buffer). It works good even though mumamo requires that major modes 
uses font-lock.

You might wonder how that can be the case. To make it work I implemented 
a workaround where I use the parsing capabilities from nxml-mode to 
check that the files follows the DTD specified syntax, but syntax 
highlighting from another mode (xml-mode/html-mode) that supports font-lock.

There is one very disturbing thing with my solution: I can't stop 
nxml-mode from parsing the whole buffer. It parses also those parts 
where mumamo has assigned another major mode. (I hoped that someone some 
day might have the time and skill to look into this, but I did not have 
them.)

Does you solution handle this problem? If it does, then how does it 
handle it? Does font-lock-fontify-region-function handle also the 
parsing of the xml code? That would be great, but it seems difficult.

Another thing that would be great would be integration with CEDET. As 
you have probably seen nxml-mode is a part of CVS Emacs and CEDET will 
hopefully soon be. Eric Ludlam has done very much work on CEDET recently.

If the completion offered by nxml-mode could be used together with CEDET 
that would be very good. (nXhtml currently offer this in a visible way 
separately, but I believe the long term solution is to go with CEDET - 
at least as an option.)

BTW, there is a problem with hi-lock. It uses text properties which may 
be hidden by overlays. IMO it should use overlays with high priorities. 
(That seems to be the easiest solution.)

> I've also added a new function:
> nxml-debug-region: Interactive function. Activate the region and call
> nxml-debug-region. The new region is what nxml thinks should be 
> re-fontified
> if the original region is changed.
> 
> The new code probably works only on Emacs 22. Lightly tested, but it 
> seems to
> handle corner highlighting cases fine. I removed a bunch of code that
> was "for redisplay", since I'm assuming font-lock handles those funky bits.
> 
> In addition to the patch, I've attached a set of files that demonstrates
> extending nXML mode to work with the Genshi template engine. The examples
> ought to work with some slight modification of the embedded paths.

Genshi was new to me. I will add it to mumamo.el.

How did you do the integration with xhtml?


> (Does the Relax NG compact syntax offer a way to say "include the next file
> for this document type"?)


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

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/emacs-nxml-mode/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/emacs-nxml-mode/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:emacs-nxml-mode-digest@yahoogroups.com 
    mailto:emacs-nxml-mode-fullfeatured@yahoogroups.com

<*> To unsubscribe from this group, send an email to:
    emacs-nxml-mode-unsubscribe@yahoogroups.com

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [emacs-nxml-mode] [patch] use font-lock
  2008-05-23 21:52 ` [patch] use font-lock Lennart Borgman (gmail)
@ 2008-05-23 22:24   ` Daniel Colascione
       [not found]   ` <200805231824.18563.danc@merrillpress.com>
  2008-05-24  2:39   ` [emacs-nxml-mode] " Stefan Monnier
  2 siblings, 0 replies; 24+ messages in thread
From: Daniel Colascione @ 2008-05-23 22:24 UTC (permalink / raw)
  To: emacs-nxml-mode

On Friday 23 May 2008, Lennart Borgman (gmail) wrote:
> You might wonder how that can be the case. To make it work I implemented
> a workaround where I use the parsing capabilities from nxml-mode to
> check that the files follows the DTD specified syntax, but syntax
> highlighting from another mode (xml-mode/html-mode) that supports
> font-lock.

I saw that you used that approach, and decided to just alter nXML-mode 
instead.

I don't really understand why nXML was written to not use font-lock in the 
first place. cc-mode had used the complex-matcher approach for a long time, 
portably and with few problems. But from having read the font-lock 
documentation, one wouldn't suppose this kind of power was available. The 
trick of making a matcher that does all the fontification itself and just 
returns 'nil' is not documented. Perhaps it should be.

> There is one very disturbing thing with my solution: I can't stop
> nxml-mode from parsing the whole buffer. It parses also those parts
> where mumamo has assigned another major mode. (I hoped that someone some
> day might have the time and skill to look into this, but I did not have
> them.)

> Does you solution handle this problem? If it does, then how does it
> handle it? Does font-lock-fontify-region-function handle also the
> parsing of the xml code? That would be great, but it seems difficult.

I haven't looked at mumamo mode in detail. How does mumamo isolate major modes 
to particular areas of the buffer? If you're just narrowing the buffer, I 
think going through nXML and removing all the calls to WIDEN should be 
sufficient. (Or replacing them with some kind of mumamo-widen.)

cc-mode also widens the buffer. What's the difference?

> Another thing that would be great would be integration with CEDET. As
> you have probably seen nxml-mode is a part of CVS Emacs and CEDET will
> hopefully soon be. Eric Ludlam has done very much work on CEDET recently.

That's good news. I've followed the "IDE features" thread on emacs-devel with 
excitement.

> If the completion offered by nxml-mode could be used together with CEDET
> that would be very good. (nXhtml currently offer this in a visible way
> separately, but I believe the long term solution is to go with CEDET -
> at least as an option.)

I don't use CEDET, but if it's getting into Emacs, I might as well give it 
another whirl. IIRC, CEDET requires a buffer parser to just generate a set of 
tags. What else is required?

> BTW, there is a problem with hi-lock. It uses text properties which may
> be hidden by overlays. IMO it should use overlays with high priorities.
> (That seems to be the easiest solution.)

IMHO, font-lock itself should use overlays. But ignoring that distinction, 
hi-lock dynamically adding font-lock keywords is the right way to go. 

HIGHLIGHT elements already support a kind of priority through the OVERRIDE 
flag, which can be t, prepend, or append, and through ordering on 
font-lock-keywords.

> Genshi was new to me. I will add it to mumamo.el.
>
> How did you do the integration with xhtml?

I extended the XHTML schema to support Genshi and XInclude elements and 
attributes; I include the original XHTML schema and augment it. 

It'd be nice if nXML extended Relax NG to support some kind of schema plugin 
mechanism, basically automating what's in qtmstr-xhtml.rnc: e.g. some way of 
saying "elements ns:a, ns:b, and ns:c from mylanguage.rnc are at block level 
and can contain block-level elements, and ns:d is inline, but can't contain 
any children. Oh, and attributes ns:foo and ns:bar can attach to all 
elements".

As it is, the best approach for Genshi, IMHO, would be some kind of minor 
mode. This mode would add the appropriate font-lock keywords and generate a 
temporary schema like the one in qtmstr-xhtml.rnc, only pointing to the 
correct xhtml.rnc.




^ permalink raw reply	[flat|nested] 24+ messages in thread

* [patch] use font-lock
@ 2008-05-23 22:26 Daniel Colascione
  2008-05-24 20:38 ` Stefan Monnier
  0 siblings, 1 reply; 24+ messages in thread
From: Daniel Colascione @ 2008-05-23 22:26 UTC (permalink / raw)
  To: emacs-devel

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

[Originally sent to the nxml mailing list]

I've converted nxml to font-lock. I used the existing fontification machinery 
and put it inside a cc-mode-style matcher. Efficiency and output are the 
same, but:

1) hi-lock-mode works now
2) all conventional font-locking functions work as expected. For example, you 
can turn fontification on and off with M-x font-lock-mode.
3) font-lock-add-keywords DTRT
4) multiple-major-modes modes should be able to use nXML fontification now, 
though this remains untested

I've also added a new function:
nxml-debug-region: Interactive function. Activate the region and call 
nxml-debug-region. The new region is what nxml thinks should be re-fontified 
if the original region is changed.

The new code probably works only on Emacs 22. Lightly tested, but it seems to 
handle corner highlighting cases fine. I removed a bunch of code that 
was "for redisplay", since I'm assuming font-lock handles those funky bits.

In addition to the patch, I've attached a set of files that demonstrates 
extending nXML mode to work with the Genshi template engine. The examples 
ought to work with some slight modification of the embedded paths.

(Does the Relax NG compact syntax offer a way to say "include the next file 
for this document type"?)



[-- Attachment #2: nxml-font-lock.patch --]
[-- Type: text/x-diff, Size: 16255 bytes --]

Index: rng-auto.el
===================================================================
--- rng-auto.el	(revision 44)
+++ rng-auto.el	(working copy)
@@ -106,12 +106,9 @@
 (autoload (quote nxml-mode) "nxml-mode" "\
 Major mode for editing XML.
 
-Syntax highlighting is performed unless the variable
-`nxml-syntax-highlight-flag' is nil.
-
 \\[nxml-finish-element] finishes the current element by inserting an end-tag.
 C-c C-i closes a start-tag with `>' and then inserts a balancing end-tag
-leaving point between the start-tag and end-tag. 
+leaving point between the start-tag and end-tag.
 \\[nxml-balanced-close-start-tag-block] is similar but for block rather than inline elements:
 the start-tag, point, and end-tag are all left on separate lines.
 If `nxml-slash-auto-complete-flag' is non-nil, then inserting a `</'
Index: nxml-mode.el
===================================================================
--- nxml-mode.el	(revision 44)
+++ nxml-mode.el	(working copy)
@@ -26,11 +26,6 @@
 
 ;; See nxml-rap.el for description of parsing strategy.
 
-;; The font locking here is independent of font-lock.el.  We want to
-;; do more sophisticated handling of changes and we want to use the
-;; same xmltok rather than regexps for parsing so that we parse
-;; consistently and correctly.
-
 ;;; Code:
 
 (when (featurep 'mucs)
@@ -55,11 +50,6 @@
   :group 'nxml
   :group 'font-lock-highlighting-faces)
 
-(defcustom nxml-syntax-highlight-flag t
-  "*Non-nil means nxml-mode should perform syntax highlighting."
-  :group 'nxml
-  :type 'boolean)
-
 (defcustom nxml-char-ref-display-glyph-flag t
   "*Non-nil means display glyph following character reference.
 The glyph is displayed in `nxml-glyph-face'.  The hook
@@ -99,8 +89,6 @@
   :group 'nxml
   :type 'integer)
 
-(defvar nxml-fontify-chunk-size 500)
-
 (defcustom nxml-bind-meta-tab-to-complete-flag (not window-system)
   "*Non-nil means bind M-TAB in `nxml-mode-map' to `nxml-complete'.
 C-return will be bound to `nxml-complete' in any case.
@@ -463,20 +451,22 @@
     map)
   "Keymap for nxml-mode.")
 
+(defvar nxml-font-lock-keywords
+  '(nxml-fontify-matcher)
+  "Default font lock keywords for nxml-mode")
+
 (defsubst nxml-set-face (start end face)
   (when (and face (< start end))
-    (put-text-property start end 'face face)))
+    (font-lock-append-text-property start end 'face face)))
 
-(defun nxml-clear-face (start end)
-  (remove-text-properties start end '(face nil))
-  (nxml-clear-char-ref-extra-display start end))
+(defsubst nxml-debug (format &rest args)
+  ;(apply #'message format args)
+  )
 
-(defsubst nxml-set-fontified (start end)
-  (put-text-property start end 'fontified t))
+(defsubst nxml-debug-change (name start end)
+  ;(nxml-debug "%s: %S" name (buffer-substring-no-properties start end))
+  )
 
-(defsubst nxml-clear-fontified (start end)
-  (remove-text-properties start end '(fontified nil)))
-
 ;;;###autoload
 (defun nxml-mode ()
   ;; We use C-c C-i instead of \\[nxml-balanced-close-start-tag-inline]
@@ -484,9 +474,6 @@
   ;; not mnemonic.
   "Major mode for editing XML.
 
-Syntax highlighting is performed unless the variable
-`nxml-syntax-highlight-flag' is nil.
-
 \\[nxml-finish-element] finishes the current element by inserting an end-tag.
 C-c C-i closes a start-tag with `>' and then inserts a balancing end-tag
 leaving point between the start-tag and end-tag.
@@ -570,13 +557,9 @@
       (nxml-clear-dependent-regions (point-min) (point-max))
       (setq nxml-scan-end (copy-marker (point-min) nil))
       (nxml-with-unmodifying-text-property-changes
-	(when nxml-syntax-highlight-flag
-	  (nxml-clear-fontified (point-min) (point-max)))
-	(nxml-clear-inside (point-min) (point-max))
+        (nxml-clear-inside (point-min) (point-max))
 	(nxml-with-invisible-motion
 	  (nxml-scan-prolog)))))
-  (when nxml-syntax-highlight-flag
-    (add-hook 'fontification-functions 'nxml-fontify nil t))
   (add-hook 'after-change-functions 'nxml-after-change nil t)
   (add-hook 'write-contents-hooks 'nxml-prepare-to-save)
   (when (not (and (buffer-file-name) (file-exists-p (buffer-file-name))))
@@ -585,6 +568,18 @@
       (setq buffer-file-coding-system nxml-default-buffer-file-coding-system))
     (when nxml-auto-insert-xml-declaration-flag
       (nxml-insert-xml-declaration)))
+
+  (setq font-lock-defaults
+        '(nxml-font-lock-keywords
+          t    ; keywords-only; we highlight comments and strings here
+          nil  ; font-lock-keywords-case-fold-search. XML is case sensitive
+          nil  ; no special syntax table
+          nil  ; no automatic syntactic fontification
+          (font-lock-extend-after-change-region-function
+           . nxml-extend-after-change-region)
+          (font-lock-extend-region-functions . (nxml-extend-region))
+          (font-lock-unfontify-region-function . nxml-unfontify-region)))
+
   (run-hooks 'nxml-mode-hook))
 
 (defun nxml-degrade (context err)
@@ -598,85 +593,91 @@
     (save-restriction
       (widen)
       (nxml-with-unmodifying-text-property-changes
-	(nxml-clear-face (point-min) (point-max))
-	(nxml-set-fontified (point-min) (point-max))
-	(nxml-clear-inside (point-min) (point-max)))
+        (nxml-clear-inside (point-min) (point-max)))
       (setq mode-name "nXML/degraded"))))
 
 ;;; Change management
 
+(defun nxml-debug-region (start end)
+  (interactive "r")
+  (let ((font-lock-beg start)
+        (font-lock-end end))
+    (nxml-extend-region)
+    (goto-char font-lock-beg)
+    (set-mark font-lock-end)))
+
 (defun nxml-after-change (start end pre-change-length)
-  ;; Work around bug in insert-file-contents.
-  (when (> end (1+ (buffer-size)))
-    (setq start 1)
-    (setq end (1+ (buffer-size))))
-  (unless nxml-degraded
+  ; in font-lock mode, nxml-after-change1 is called via
+  ; nxml-extend-after-change-region instead so that the updated
+  ; book-keeping information is available for fontification.
+
+  (unless (or font-lock-mode nxml-degraded)
     (condition-case err
-	(save-excursion
-	  (save-restriction
-	    (widen)
-	    (save-match-data
-	      (nxml-with-invisible-motion
-		(nxml-with-unmodifying-text-property-changes
-		  (nxml-after-change1 start end pre-change-length))))))
+        (save-excursion
+          (save-restriction
+            (widen)
+            (save-match-data
+              (nxml-with-invisible-motion
+                (nxml-with-unmodifying-text-property-changes
+                  (nxml-after-change1
+                   start end pre-change-length))))))
+
       (error
        (nxml-degrade 'nxml-after-change err)))))
 
 (defun nxml-after-change1 (start end pre-change-length)
-  (setq nxml-last-fontify-end nil)
+  "after-change book-keeping. returns a cons containing a
+possibly-enlarged change region. you must still call
+nxml-extend-region on this expanded region to obtain the full
+extent of the area needing refontification.
+
+For book-keeping, call this function even when fontification is
+disabled."
+  ;; Work around bug in insert-file-contents, apparently
+  (when (> end (1+ (buffer-size)))
+    (setq start 1)
+    (setq end (1+ (buffer-size))))
+
   (let ((pre-change-end (+ start pre-change-length)))
     (setq start
 	  (nxml-adjust-start-for-dependent-regions start
-						   end
-						   pre-change-length))
+                                                   end
+                                                   pre-change-length))
+
+    ;; If the prolog might have changed, rescan the prolog
     (when (<= start
-	      ;; Add 2 so as to include the < and following char
-	      ;; that start the instance, since changing these
-	      ;; can change where the prolog ends.
+	      ;; Add 2 so as to include the < and following char that
+	      ;; start the instance (document element), since changing
+	      ;; these can change where the prolog ends.
 	      (+ nxml-prolog-end 2))
-      ;; end must be extended to at least the end of the old prolog
+      ;; end must be extended to at least the end of the old prolog in
+      ;; case the new prolog is shorter
       (when (< pre-change-end nxml-prolog-end)
 	(setq end
 	      ;; don't let end get out of range even if pre-change-length
 	      ;; is bogus
 	      (min (point-max)
 		   (+ end (- nxml-prolog-end pre-change-end)))))
+
       (nxml-scan-prolog)))
-  (cond ((<= end nxml-prolog-end)
-	 (setq end nxml-prolog-end)
-	 (goto-char start)
-	 ;; This is so that Emacs redisplay works
-	 (setq start (line-beginning-position)))
-	((and (<= start nxml-scan-end)
-	      (> start (point-min))
-	      (nxml-get-inside (1- start)))
-	 ;; The closing delimiter might have been removed.
-	 ;; So we may need to redisplay from the beginning
-	 ;; of the token.
-	 (goto-char (1- start))
-	 (nxml-move-outside-backwards)
-	 ;; This is so that Emacs redisplay works
-	 (setq start (line-beginning-position))
-	 (setq end (max (nxml-scan-after-change (point) end)
-			end)))
-	(t
-	 (goto-char start)
-	 ;; This is both for redisplay and to move back
-	 ;; past any incomplete opening delimiters
-	 (setq start (line-beginning-position))
-	 (setq end (max (nxml-scan-after-change start end)
-			end))))
-  (when nxml-syntax-highlight-flag
-    (when (>= start end)
-      ;; Must clear at least one char so as to trigger redisplay.
-      (cond ((< start (point-max))
-	     (setq end (1+ start)))
-	    (t
-	     (setq end (point-max))
-	     (goto-char end)
-	     (setq start (line-beginning-position)))))
-    (nxml-clear-fontified start end)))
 
+  (when (> end nxml-prolog-end)
+    (when (and (<= start nxml-scan-end)
+               (> start (point-min))
+               (nxml-get-inside (1- start)))
+      ;; The closing delimiter might have been removed.
+      ;; So we may need to redisplay from the beginning
+      ;; of the token.
+      (goto-char (1- start))
+      (nxml-move-outside-backwards)
+      (setq start (point)))
+
+    (setq end (max (nxml-scan-after-change start end)
+                   end)))
+
+  (nxml-debug-change "nxml-after-change1" start end)
+  (cons start end))
+
 ;;; Encodings
 
 (defun nxml-insert-xml-declaration ()
@@ -862,59 +863,113 @@
 
 ;;; Fontification
 
-(defun nxml-fontify (start)
-  (condition-case err
-      (save-excursion
-	(save-restriction
-	  (widen)
-	  (save-match-data
-	    (nxml-with-invisible-motion
-	      (nxml-with-unmodifying-text-property-changes
-		(if (or nxml-degraded
-			;; just in case we get called in the wrong buffer
-			(not nxml-prolog-end))
-		    (nxml-set-fontified start (point-max))
-		  (nxml-fontify1 start)))))))
-    (error
-     (nxml-degrade 'nxml-fontify err))))
+(defun nxml-unfontify-region (start end)
+  (font-lock-default-unfontify-region start end)
+  (nxml-clear-char-ref-extra-display start end))
 
-(defun nxml-fontify1 (start)
-  (cond ((< start nxml-prolog-end)
-	 (nxml-fontify-prolog)
-	 (nxml-set-fontified (point-min)
-			     nxml-prolog-end))
-	(t
-	 (goto-char start)
-	 (when (not (eq nxml-last-fontify-end start))
-	   (when (not (equal (char-after) ?\<))
-	     (search-backward "<" nxml-prolog-end t))
-	   (nxml-ensure-scan-up-to-date)
-	   (nxml-move-outside-backwards))
-	 (let ((start (point)))
-	   (nxml-do-fontify (min (point-max)
-				 (+ start nxml-fontify-chunk-size)))
-	   (setq nxml-last-fontify-end (point))
-	   (nxml-set-fontified start nxml-last-fontify-end)))))
+(defun nxml-extend-region ()
+  "Extend the region to hold the minimum area we can fontify with
+nXML. Called with font-lock-beg and font-lock-end dynamically bound."
+  (let ((start font-lock-beg)
+        (end font-lock-end))
 
-(defun nxml-fontify-buffer ()
-  (interactive)
-  (save-excursion
-    (save-restriction
-      (widen)
-      (nxml-with-invisible-motion
-	(goto-char (point-min))
-	(nxml-with-unmodifying-text-property-changes
-	  (nxml-fontify-prolog)
-	  (goto-char nxml-prolog-end)
-	  (nxml-do-fontify))))))
+    (nxml-debug-change "nxml-extend-region(input)" start end)
 
+    (when (< start nxml-prolog-end)
+      (setq start (point-min)))
+
+    (cond ((<= end nxml-prolog-end)
+           (setq end nxml-prolog-end))
+
+          (t
+           (goto-char start)
+           ;; some font-lock backends (like Emacs 22 jit-lock) snap
+           ;; the region to the beginning of the line no matter what
+           ;; we say here. To mitigate the resulting excess
+           ;; fontification, ignore leading whitespace.
+           (skip-syntax-forward " ")
+
+           ;; find the beginning of the previous tag
+           (when (not (equal (char-after) ?\<))
+             (search-backward "<" nxml-prolog-end t))
+           (nxml-ensure-scan-up-to-date)
+           (nxml-move-outside-backwards)
+           (setq start (point))
+
+           (while (< (point) end)
+             (nxml-tokenize-forward))
+
+           (setq end (point))))
+
+    (when (or (< start font-lock-beg)
+              (> end font-lock-end))
+      (setq font-lock-beg start
+            font-lock-end end)
+      (nxml-debug-change "nxml-extend-region" start end)
+      t)))
+
+(defun nxml-extend-after-change-region (start end pre-change-length)
+  (unless nxml-degraded
+    (setq nxml-last-fontify-end nil)
+
+    (when (> end (1+ (buffer-size)))
+    (setq start 1)
+    (setq end (1+ (buffer-size))))
+
+    (condition-case err
+	(save-excursion
+	  (save-restriction
+	    (widen)
+	    (save-match-data
+	      (nxml-with-invisible-motion
+		(nxml-with-unmodifying-text-property-changes
+                  (nxml-extend-after-change-region1 start end pre-change-length))))))
+      (error
+       (nxml-degrade 'nxml-extend-after-change-region err)))))
+
+(defun nxml-extend-after-change-region1 (start end pre-change-length)
+  (let* ((region (nxml-after-change1 start end pre-change-length))
+         (font-lock-beg (car region))
+         (font-lock-end (cdr region)))
+
+    (nxml-extend-region)
+    (cons font-lock-beg font-lock-end)))
+
+(defun nxml-fontify-matcher (bound)
+  "Called as font-lock keyword matcher."
+
+  (unless nxml-degraded
+    (nxml-debug-change "nxml-fontify-matcher" (point) bound)
+
+    (when (< (point) nxml-prolog-end)
+      (goto-char (point-min))
+      (nxml-fontify-prolog)
+      (goto-char nxml-prolog-end))
+
+    (when (not (eq nxml-last-fontify-end (point)))
+      (when (not (equal (char-after) ?\<))
+        (search-backward "<" nxml-prolog-end t))
+      (nxml-ensure-scan-up-to-date)
+      (nxml-move-outside-backwards))
+
+    (let (xmltok-dependent-regions
+          xmltok-errors)
+      (while (and (< (point) bound)
+                  (nxml-tokenize-forward))
+        (nxml-apply-fontify-rule)))
+
+    (setq nxml-last-fontify-end (point)))
+
+  ;; Since we did the fontification internally, tell font-lock to not
+  ;; do anything itself.
+  nil)
+
 (defun nxml-fontify-prolog ()
   "Fontify the prolog.
 The buffer is assumed to be prepared for fontification.
 This does not set the fontified property, but it does clear
 faces appropriately."
   (let ((regions nxml-prolog-regions))
-    (nxml-clear-face (point-min) nxml-prolog-end)
     (while regions
       (let ((region (car regions)))
 	(nxml-apply-fontify-rule (aref region 0)
@@ -922,17 +977,6 @@
 				 (aref region 2)))
       (setq regions (cdr regions)))))
 
-(defun nxml-do-fontify (&optional bound)
-  "Fontify at least as far as bound.
-Leave point after last fontified position."
-  (unless bound (setq bound (point-max)))
-  (let (xmltok-dependent-regions
-	xmltok-errors)
-    (while (and (< (point) bound)
-		(nxml-tokenize-forward))
-      (nxml-clear-face xmltok-start (point))
-      (nxml-apply-fontify-rule))))
-
 ;; Vectors identify a substring of the token to be highlighted in some face.
 
 ;; Token types returned by xmltok-forward.
@@ -2582,13 +2626,7 @@
 	       (> (prefix-numeric-value arg) 0))))
     (when (not (eq new nxml-char-ref-extra-display))
       (setq nxml-char-ref-extra-display new)
-      (save-excursion
-	(save-restriction
-	  (widen)
-	  (if nxml-char-ref-extra-display
-	      (nxml-with-unmodifying-text-property-changes
-		(nxml-clear-fontified (point-min) (point-max)))
-	    (nxml-clear-char-ref-extra-display (point-min) (point-max))))))))
+      (font-lock-fontify-buffer))))
 
 (put 'nxml-char-ref 'evaporate t)
 

[-- Attachment #3: qtmstr-nxml-mode.el --]
[-- Type: text/plain, Size: 1137 bytes --]

;;;; Customization for nXML mode

(setq rng-schema-locating-files
      (append '("~/emacs/nxml-schema/schemas.xml")
              rng-schema-locating-files-default))

(defface nxml-template
  '((t (:bold t
        :foreground "blue"
        :background "#ddddff")))
  "Face used to highlight embedded template constructs"
  :group 'nxml-highlighting-faces)

(defun qtmstr-nxml-hook ()
  (when (string-match "\\.html$" buffer-file-name)
    (let ((py-prefix-re
           (regexp-opt
            '(
              "py:if"
              "py:choose"
              "py:when"
              "py:otherwise"
              "py:for"
              "py:def"
              "py:match"
              "py:with"
              "py:attrs"
              "py:content"
              "py:replace"
              "py:strip"))))

      (font-lock-add-keywords
       nil
       `(("\\$\\([a-zA-Z_][a-zA-Z0-9_]*\\)" 1
          'nxml-template prepend)

         ("\\${\\([^}\"]+\\)}" 1
          'nxml-template prepend)

         (,(concat py-prefix-re "=\"\\([^\"]*\\)\"") 1
          'nxml-template prepend))))))

(add-hook 'nxml-mode-hook #'qtmstr-nxml-hook)



[-- Attachment #4: genshi.rnc --]
[-- Type: text/plain, Size: 1265 bytes --]

namespace py = "http://genshi.edgewall.org/"

genshi.expr-type    = xsd:string { minLength = "1" }
genshi.with-type    = xsd:string { minLength = "1" }
genshi.choose-type  = xsd:string
genshi.def-type   = xsd:string
genshi.xpath-type = xsd:anyURI

genshi.attrib = attribute py:if        { genshi.expr-type   }?,
                attribute py:choose    { genshi.choose-type }?,
                attribute py:when      { genshi.expr-type   }?,
                attribute py:otherwise { genshi.expr-type   }?,
                attribute py:for       { genshi.expr-type   }?,
                attribute py:def       { genshi.def-type    }?,
                attribute py:match     { genshi.xpath-type  }?,
                attribute py:with      { genshi.with-type   }?,
                attribute py:attrs     { genshi.expr-type   }?,
                attribute py:content   { genshi.expr-type   }?,
                attribute py:replace   { genshi.expr-type   }?,
                attribute py:strip     { genshi.expr-type   }?

genshi.if.attlist   = attribute expr { genshi.expr-type }
genshi.for.attlist  = attribute each { genshi.expr-type }
genshi.def.attlist  = attribute each { genshi.expr-type }
genshi.with.attlist = attribute vars { genshi.with-type }

               

[-- Attachment #5: qtmstr-xhtml.rnc --]
[-- Type: text/x-c++src, Size: 2110 bytes --]

namespace py = "http://genshi.edgewall.org/"
namespace xi = "http://www.w3.org/2001/XInclude"

include "genshi.rnc"
include "xinclude.rnc"
include "../nxml/schema/xhtml.rnc"

start |= head|body|p|\div|h1|h2|h3|h4|h5|h6|hr|pre|dl|ol|ul|table|form

Common.attrib &= genshi.attrib
head.attlist  &= genshi.attrib
html.attlist  &= genshi.attrib

Head.class = base | isindex | link | meta | script | title | style |
             if-head | for-head | def-head | with-head

Head.model = Head.class*

head.content &= Head.model*

if-inline   = element py:if   { genshi.if.attlist, Inline.model }
if-block    = element py:if   { genshi.if.attlist, Block.model }
if-head     = element py:if   { genshi.if.attlist, Head.model }
for-inline  = element py:for  { genshi.for.attlist, Inline.model }
for-block   = element py:for  { genshi.for.attlist, Block.model }
for-head    = element py:for  { genshi.for.attlist, Head.model }
def-inline  = element py:def  { genshi.def.attlist, Inline.model }
def-block   = element py:def  { genshi.def.attlist, Block.model }
def-head    = element py:def  { genshi.def.attlist, Head.model }
with-inline = element py:with { genshi.with.attlist, Inline.model }
with-block  = element py:with { genshi.with.attlist, Block.model }
with-head   = element py:with { genshi.with.attlist, Head.model }

Inline.class |= if-inline | for-inline | def-inline | with-inline
Block.class  |= if-block | for-block | def-block | with-block

xi-inline = element xi:include {
                xinclude.include.attlist,
                element xi:fallback { genshi.attrib,
                    (xi-inline | Inline.model)*
                }?
            }

xi-block  = element xi:include { xinclude.include.attlist,
                element xi:fallback { genshi.attrib,
                    (xi-block | Block.model)*
                }?
            }

xi-head   = element xi:include { xinclude.include.attlist,
                element xi:fallback { genshi.attrib,
                    (xi-head | Head.model)*
                }?
            }

Inline.class |= xi-inline
Block.class  |= xi-block
Head.class   |= xi-head

[-- Attachment #6: schemas.xml --]
[-- Type: text/xml, Size: 136 bytes --]

<locatingRules xmlns="http://thaiopensource.com/ns/locating-rules/1.0">
  <typeId id="XHTML" uri="qtmstr-xhtml.rnc" />
</locatingRules>

[-- Attachment #7: xinclude.rnc --]
[-- Type: text/x-c++src, Size: 345 bytes --]

namespace xi = "http://www.w3.org/2001/XInclude"
namespace local = ""

xinclude.include.attlist =
    attribute href     { xsd:anyURI }?,
    attribute parse    { xsd:string }?,
    attribute xpointer { xsd:string }?,
    attribute encoding { xsd:string }?,
    attribute accept   { xsd:string }?,
    attribute accept-language { xsd:string }?


[-- Attachment #8: HelloWorldPage.html --]
[-- Type: text/html, Size: 184 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
       [not found]   ` <200805231824.18563.danc@merrillpress.com>
@ 2008-05-23 22:50     ` Lennart Borgman (gmail)
  2008-05-24 15:03       ` Daniel Colascione
  0 siblings, 1 reply; 24+ messages in thread
From: Lennart Borgman (gmail) @ 2008-05-23 22:50 UTC (permalink / raw)
  To: emacs-nxml-mode, Daniel Colascione; +Cc: Eric M. Ludlam, Emacs Devel

Daniel Colascione wrote:
> 
> 
> On Friday 23 May 2008, Lennart Borgman (gmail) wrote:
>  > You might wonder how that can be the case. To make it work I implemented
>  > a workaround where I use the parsing capabilities from nxml-mode to
>  > check that the files follows the DTD specified syntax, but syntax
>  > highlighting from another mode (xml-mode/html-mode) that supports
>  > font-lock.
> 
> I saw that you used that approach, and decided to just alter nXML-mode
> instead.
> 
> I don't really understand why nXML was written to not use font-lock in the
> first place. cc-mode had used the complex-matcher approach for a long time,
> portably and with few problems. But from having read the font-lock
> documentation, one wouldn't suppose this kind of power was available. The
> trick of making a matcher that does all the fontification itself and just
> returns 'nil' is not documented. Perhaps it should be.

Do you mean font-lock-fontify-region-function?

>  > There is one very disturbing thing with my solution: I can't stop
>  > nxml-mode from parsing the whole buffer. It parses also those parts
>  > where mumamo has assigned another major mode. (I hoped that someone some
>  > day might have the time and skill to look into this, but I did not have
>  > them.)
> 
>  > Does you solution handle this problem? If it does, then how does it
>  > handle it? Does font-lock-fontify-region-function handle also the
>  > parsing of the xml code? That would be great, but it seems difficult.
> 
> I haven't looked at mumamo mode in detail. How does mumamo isolate major 
> modes
> to particular areas of the buffer? If you're just narrowing the buffer, I
> think going through nXML and removing all the calls to WIDEN should be
> sufficient. (Or replacing them with some kind of mumamo-widen.)

Mumamo now defines what I call "multi major modes". (Previously there 
was a minor mode called mumamo-mode, but that is obsolete.)

Mumamo defines its own font-lock-fontify-region-function that does the 
job. In this function it narrows the buffer to chunks where each chunk 
has its own major mode.

So the main question is: does the parsing happens in the 
font-lock-fontify-region-function that you have defined in nxml-mode? (I 
guess you have defined such a function?)

One thing I think need to be taken care of is nxml-mode starting state 
in chunks handled by mumamo. Is there a way to do that?


> cc-mode also widens the buffer. What's the difference?

Sorry, I do not understand what you mean here.


> I don't use CEDET, but if it's getting into Emacs, I might as well give it
> another whirl. IIRC, CEDET requires a buffer parser to just generate a 
> set of
> tags. What else is required?

I do not know. Maybe this discussion should be carried over to Emacs 
devel which I think Eric Ludlam reads?


>  > BTW, there is a problem with hi-lock. It uses text properties which may
>  > be hidden by overlays. IMO it should use overlays with high priorities.
>  > (That seems to be the easiest solution.)
> 
> IMHO, font-lock itself should use overlays.

That would probably be a performance problem.

> But ignoring that distinction,
> hi-lock dynamically adding font-lock keywords is the right way to go.

Yes, it is very nice.

>  > Genshi was new to me. I will add it to mumamo.el.
>  >
>  > How did you do the integration with xhtml?
> 
> I extended the XHTML schema to support Genshi and XInclude elements and
> attributes; I include the original XHTML schema and augment it.

Thanks, very nice. That mean you can handle <?pyton ... ?> and for 
example py:content="var".

mumamo can help in two ways here:

1) inside <?pyton ... ?> it can use a genshi-mode major mode if there is 
one.

2) inside {% pyton ... %} it can do the same.


> It'd be nice if nXML extended Relax NG to support some kind of schema 
> plugin
> mechanism, basically automating what's in qtmstr-xhtml.rnc: e.g. some 
> way of
> saying "elements ns:a, ns:b, and ns:c from mylanguage.rnc are at block 
> level
> and can contain block-level elements, and ns:d is inline, but can't contain
> any children. Oh, and attributes ns:foo and ns:bar can attach to all
> elements".

Good idea IMO.


> As it is, the best approach for Genshi, IMHO, would be some kind of minor
> mode. This mode would add the appropriate font-lock keywords and generate a
> temporary schema like the one in qtmstr-xhtml.rnc, only pointing to the
> correct xhtml.rnc.

Maybe. But with mumamo it could be a major mode, written with those 
tools that there already are in Emacs.

This could be combined with the schema extending mechanism you suggest.

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

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/emacs-nxml-mode/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/emacs-nxml-mode/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:emacs-nxml-mode-digest@yahoogroups.com 
    mailto:emacs-nxml-mode-fullfeatured@yahoogroups.com

<*> To unsubscribe from this group, send an email to:
    emacs-nxml-mode-unsubscribe@yahoogroups.com

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [emacs-nxml-mode] [patch] use font-lock
  2008-05-23 21:52 ` [patch] use font-lock Lennart Borgman (gmail)
  2008-05-23 22:24   ` [emacs-nxml-mode] " Daniel Colascione
       [not found]   ` <200805231824.18563.danc@merrillpress.com>
@ 2008-05-24  2:39   ` Stefan Monnier
  2 siblings, 0 replies; 24+ messages in thread
From: Stefan Monnier @ 2008-05-24  2:39 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: Emacs Devel, Eric M. Ludlam, emacs-nxml-mode

>> I've converted nxml to font-lock.  I used the existing fontification
>> machinery and put it inside a cc-mode-style matcher.  Efficiency and
>> output are the same, but:
>> 
>> 1) hi-lock-mode works now
>> 2) all conventional font-locking functions work as expected.
>>    For example, you can turn fontification on and off with
>>    M-x font-lock-mode.
>> 3) font-lock-add-keywords DTRT
>> 4) multiple-major-modes modes should be able to use nXML fontification now,
>>    though this remains untested

Sounds great.  Would you submit your patch to emacs-devel@gnu.org?


        Stefan


PS: BTW, is the Emacs CVS now the "official" nxml repository?




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-05-23 22:50     ` Lennart Borgman (gmail)
@ 2008-05-24 15:03       ` Daniel Colascione
  2008-05-24 16:57         ` Lennart Borgman (gmail)
  0 siblings, 1 reply; 24+ messages in thread
From: Daniel Colascione @ 2008-05-24 15:03 UTC (permalink / raw)
  To: emacs-nxml-mode; +Cc: Daniel Colascione, Eric M. Ludlam, Emacs Devel

On Friday 23 May 2008 06:50:19 pm Lennart Borgman (gmail) wrote:
> Do you mean font-lock-fontify-region-function?

Sort of. I'm talking about using a function as a matcher keyword in 
font-lock-keywords. This function always returns nil, but while searching, 
sets the face property itself between (point) and its one parameter.

The advantage of using the function-matcher approach is that it interacts 
nicely with other users of font-lock-keywords. cc-mode has used it forever, 
and it seems to work well.

AFAICS, this trick is undocumented.

> Mumamo defines its own font-lock-fontify-region-function that does the
> job. In this function it narrows the buffer to chunks where each chunk
> has its own major mode.
>
> So the main question is: does the parsing happens in the
> font-lock-fontify-region-function that you have defined in nxml-mode? (I
> guess you have defined such a function?)

Yes, in that the after-change function that updates bookkeeping information is 
called from the font-lock machinery (via 
font-lock-extend-after-change-function). But it doesn't matter here, because 
at least nxml still keeps some buffer-global information, like the end of the 
XML prolog (which is assume to extend from the start of the buffer to 
nxml-prolog-end).

> One thing I think need to be taken care of is nxml-mode starting state
> in chunks handled by mumamo. Is there a way to do that?

I don't see an easy way to do that. The problem is that nxml needs to widen 
the buffer to see whether the prolog has changed, whether it's scanned as far 
as a given change, and so on. And then it assumes the buffer is contiguous. 
But I didn't write it; there may be a way to deal with the problem.

I'm just speculating here, but what if there were some indirect overlay 
property? It would have the behavior of using, as the overlay content, the 
contents of another buffer. Moving the point in the parent would move the 
point in the child. The child buffer would have its own font-lock setup, and 
the resulting highlighting would show up in the parent.

Using a system like that, you could stitch together multiple child buffers to 
form a parent that looked like it had multiple major modes. And each major 
mode of a child buffer wouldn't know or care about the parent.

> > IMHO, font-lock itself should use overlays.
>
> That would probably be a performance problem.

And a compatibility nightmare, you're right. But I think it would have been a 
better conceptual model.


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

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/emacs-nxml-mode/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/emacs-nxml-mode/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:emacs-nxml-mode-digest@yahoogroups.com 
    mailto:emacs-nxml-mode-fullfeatured@yahoogroups.com

<*> To unsubscribe from this group, send an email to:
    emacs-nxml-mode-unsubscribe@yahoogroups.com

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-05-24 15:03       ` Daniel Colascione
@ 2008-05-24 16:57         ` Lennart Borgman (gmail)
  0 siblings, 0 replies; 24+ messages in thread
From: Lennart Borgman (gmail) @ 2008-05-24 16:57 UTC (permalink / raw)
  To: emacs-nxml-mode
  Cc: Daniel Colascione, Eric M. Ludlam, Emacs Devel, Steve Yegge

Daniel Colascione wrote:
> On Friday 23 May 2008 06:50:19 pm Lennart Borgman (gmail) wrote:
>  > Do you mean font-lock-fontify-region-function?
> 
> Sort of. I'm talking about using a function as a matcher keyword in
> font-lock-keywords. This function always returns nil, but while searching,
> sets the face property itself between (point) and its one parameter.
> 
> The advantage of using the function-matcher approach is that it interacts
> nicely with other users of font-lock-keywords. cc-mode has used it forever,
> and it seems to work well.
> 
> AFAICS, this trick is undocumented.

Is not that documented in `font-lock-keywords'? But maybe you mean it 
could be more explicit?


>  > Mumamo defines its own font-lock-fontify-region-function that does the
>  > job. In this function it narrows the buffer to chunks where each chunk
>  > has its own major mode.
>  >
>  > So the main question is: does the parsing happens in the
>  > font-lock-fontify-region-function that you have defined in nxml-mode? (I
>  > guess you have defined such a function?)
> 
> Yes, in that the after-change function that updates bookkeeping 
> information is
> called from the font-lock machinery (via
> font-lock-extend-after-change-function).

I am not sure what you mean here. Mumamo calls font-lock-fontify-region 
(after trying to set the variables needed for the chunks major mode). 
Does that work together with your approach?


>  > One thing I think need to be taken care of is nxml-mode starting state
>  > in chunks handled by mumamo. Is there a way to do that?
> 
> I don't see an easy way to do that. The problem is that nxml needs to widen
> the buffer to see whether the prolog has changed, whether it's scanned 
> as far
> as a given change, and so on. And then it assumes the buffer is contiguous.
> But I didn't write it; there may be a way to deal with the problem.

It is a general problem. Js2 has a similar problem. (I am cc:ing Steve 
Yegge too ;-)


> I'm just speculating here, but what if there were some indirect overlay
> property? It would have the behavior of using, as the overlay content, the
> contents of another buffer. Moving the point in the parent would move the
> point in the child. The child buffer would have its own font-lock setup, 
> and
> the resulting highlighting would show up in the parent.
> 
> Using a system like that, you could stitch together multiple child 
> buffers to
> form a parent that looked like it had multiple major modes. And each major
> mode of a child buffer wouldn't know or care about the parent.

I have been thinking about different such possibilities. AFAICS it would 
be much simpler if the parsing modes (like nxml-mode) are aware of what 
should be parsed in the buffers. Other possibilities tends to be very 
complex.

So I would suggest that something like mumamo divides the buffer into 
chunks with different major modes. Then the parsing mode and mumamo (or 
something similar) should communicate to tell what chunks should be 
parsed and what chunks should not be parsed. (The major mode itself is 
not enought to tell if a chunk should be parsed or not. For example a 
<style> ... </style> chunk in xhtml can be parsed by nxml-mode. Ie 
nxml-mode does not care much about the content of it.)

Doing it that way I see two problems:

1) The parser must be able to somehow support a "parse region" 
functionality.

2) Parsing states for the different chunks must be initialized somehow. 
I would suggest carrying over the last state from the previous chunk if 
needed. (Sometimes assuming that the parsing state at the beginning of 
the chunk is the "zero state" is enough, sometimes not.)

As you noticed the parser must somehow start from the beginning of the file.

Font lock could cheat a bit and start in the middle of a file so parsing 
and font-lock style syntax highlighting do not have the same needs. 
Maybe that can be handled by doing the syntax highlighting in two 
passes, the first pass similar to font-lock currents way and the second 
pass doing parsing?

This would allow editing with the benefit of syntax highlighting 
(font-lock style) while the parser works in the background, doing the 
second pass. When the support of the parser is needed (for example for 
completion) the user of course would have to wait for the parser to 
catch up.



How would such a scheme fit js2 and the changed nxml-mode? (I guess 
there is more work to support it in both modes and a framework similar 
to jit-lock would have to be used for the parsing.)

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

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/emacs-nxml-mode/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/emacs-nxml-mode/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:emacs-nxml-mode-digest@yahoogroups.com 
    mailto:emacs-nxml-mode-fullfeatured@yahoogroups.com

<*> To unsubscribe from this group, send an email to:
    emacs-nxml-mode-unsubscribe@yahoogroups.com

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-05-23 22:26 Daniel Colascione
@ 2008-05-24 20:38 ` Stefan Monnier
  2008-05-25 20:36   ` Daniel Colascione
  0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2008-05-24 20:38 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

> I've converted nxml to font-lock. I used the existing fontification machinery 
> and put it inside a cc-mode-style matcher. Efficiency and output are the 
> same, but:

Looks pretty good.  See some comments below.

 
> +  "after-change book-keeping. returns a cons containing a
> +possibly-enlarged change region. you must still call
> +nxml-extend-region on this expanded region to obtain the full
> +extent of the area needing refontification.
> +
> +For book-keeping, call this function even when fontification is
> +disabled."
> +  ;; Work around bug in insert-file-contents, apparently
> +  (when (> end (1+ (buffer-size)))
> +    (setq start 1)
> +    (setq end (1+ (buffer-size))))

Please use (point-max) rather than (1+ (buffer-size)).
Could you expand on this "bug in insert-file-contents"?
Maybe we can fix it.

> +(defun nxml-fontify-matcher (bound)
> +  "Called as font-lock keyword matcher."
> +
> +  (unless nxml-degraded
> +    (nxml-debug-change "nxml-fontify-matcher" (point) bound)
> +
> +    (when (< (point) nxml-prolog-end)
> +      (goto-char (point-min))
> +      (nxml-fontify-prolog)
> +      (goto-char nxml-prolog-end))

Fontifying outside of (point)...bound is likely to lead to problems.  
Either we should change nxml-fontify-prolog so it can be told to only
fontify some part of the prolog, or we need to extend the region
according in nxml-extend-region rather than in nxml-fontify-matcher.

> +    (when (not (eq nxml-last-fontify-end (point)))
> +      (when (not (equal (char-after) ?\<))
> +        (search-backward "<" nxml-prolog-end t))
> +      (nxml-ensure-scan-up-to-date)
> +      (nxml-move-outside-backwards))

This should be done in nxml-extend-region instead.  And it indeed seems
to be done there, so it should be removed.  If it's still needed here,
despite nxml-extend-region, then we probably have a bug somewhere (for
the same reason as above:
Fontifying outside of (point)...bound is likely to lead to problems.)


        Stefan




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-05-24 20:38 ` Stefan Monnier
@ 2008-05-25 20:36   ` Daniel Colascione
  2008-05-26 14:52     ` Stefan Monnier
  0 siblings, 1 reply; 24+ messages in thread
From: Daniel Colascione @ 2008-05-25 20:36 UTC (permalink / raw)
  To: emacs-devel; +Cc: Stefan Monnier

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

On Saturday 24 May 2008 04:38:42 pm Stefan Monnier wrote:
> Looks pretty good.  See some comments below.
...
> > +  ;; Work around bug in insert-file-contents, apparently
...
> Could you expand on this "bug in insert-file-contents"?
> Maybe we can fix it.

I think the only person who can comment on this piece is the person who wrote 
it. I just copied it from a similar passage in the original nxml code. In my 
updated patch, I've removed the code that works around this alleged bug 
without ill effect.

> > +    (when (< (point) nxml-prolog-end)
> > +      (goto-char (point-min))
> > +      (nxml-fontify-prolog)
> > +      (goto-char nxml-prolog-end))
>
> Fontifying outside of (point)...bound is likely to lead to problems.

I think this code is okay though, since nxml-extend-region *does* snap the 
fontification region to (point-min), and nxml is written to fontify the 
prolog by moving forward over the whole thing, fontifying along the way.

> > +    (when (not (eq nxml-last-fontify-end (point)))
> > +      (when (not (equal (char-after) ?\<))
> > +        (search-backward "<" nxml-prolog-end t))
> > +      (nxml-ensure-scan-up-to-date)
> > +      (nxml-move-outside-backwards))
>
> This should be done in nxml-extend-region instead. 

OTOH, this chunk shouldn't be there. You're right. Removing this code exposed 
a bug, which is corrected in my updated patch. The original nxml mode also 
had this bug, but because nxml-mode always fontified whole lines, the bug 
didn't cause problems.

Also, 
    (let (xmltok-dependent-regions
          xmltok-errors)
      (while (and (< (point) bound)
                  (nxml-tokenize-forward))
        (nxml-apply-fontify-rule)))

will cause problems if the last nxml token extends beyond bound. 
nxml-extend-region ensures that it doesn't, but can we count on it?


[-- Attachment #2: nxml-mode-2.patch --]
[-- Type: text/x-diff, Size: 19717 bytes --]

Index: nxml-util.el
===================================================================
--- nxml-util.el	(revision 44)
+++ nxml-util.el	(working copy)
@@ -24,6 +24,35 @@
 
 ;;; Code:
 
+(defconst nxml-debug nil
+  "enable nxml debugging. effective only at compile time")
+
+(eval-when-compile
+  (require 'cl))
+
+(defsubst nxml-debug (format &rest args)
+  (when nxml-debug
+    (apply #'message format args)))
+
+(defmacro nxml-debug-change (name start end)
+  (when nxml-debug
+    `(nxml-debug "%s: %S" ,name
+                (buffer-substring-no-properties ,start ,end))))
+
+(defmacro nxml-debug-set-inside (start end)
+  (when nxml-debug
+    `(let ((overlay (make-overlay ,start ,end)))
+       (overlay-put overlay 'face '(:background "red"))
+       (overlay-put overlay 'nxml-inside-debug t)
+       (nxml-debug-change "nxml-set-inside" ,start ,end))))
+
+(defmacro nxml-debug-clear-inside (start end)
+  (when nxml-debug
+    `(loop for overlay in (overlays-in ,start ,end)
+           if (overlay-get overlay 'nxml-inside-debug)
+           do (delete-overlay overlay)
+           finally (nxml-debug-change "nxml-clear-inside" ,start ,end))))
+
 (defun nxml-make-namespace (str)
   "Return a symbol for the namespace URI STR.
 STR must be a string. If STR is the empty string, return nil.
@@ -37,12 +66,21 @@
 This is the inverse of `nxml-make-namespace'."
   (and ns (substring (symbol-name ns) 1)))
 
-(defconst nxml-xml-namespace-uri 
+(defconst nxml-xml-namespace-uri
   (nxml-make-namespace "http://www.w3.org/XML/1998/namespace"))
 
 (defconst nxml-xmlns-namespace-uri
   (nxml-make-namespace "http://www.w3.org/2000/xmlns/"))
 
+(defmacro nxml-with-degradation-on-error (context &rest body)
+  (if (not nxml-debug)
+      (let ((error-symbol (gensym "err")))
+        `(condition-case ,error-symbol
+             (progn ,@body)
+           (error
+            (nxml-degrade ,context ,error-symbol))))
+    `(progn ,@body)))
+
 (defmacro nxml-with-unmodifying-text-property-changes (&rest body)
   "Evaluate BODY without any text property changes modifying the buffer.
 Any text properties changes happen as usual but the changes are not treated as
Index: nxml-rap.el
===================================================================
--- nxml-rap.el	(revision 44)
+++ nxml-rap.el	(working copy)
@@ -110,9 +110,11 @@
   (get-text-property pos 'nxml-inside))
 
 (defsubst nxml-clear-inside (start end)
+  (nxml-debug-clear-inside start end)
   (remove-text-properties start end '(nxml-inside nil)))
 
 (defsubst nxml-set-inside (start end type)
+  (nxml-debug-set-inside start end)
   (put-text-property start end 'nxml-inside type))
 
 (defun nxml-inside-end (pos)
@@ -137,12 +139,10 @@
   "Restore `nxml-scan-end' invariants after a change.
 The change happened between START and END.
 Return position after which lexical state is unchanged.
-END must be > nxml-prolog-end."
+END must be > nxml-prolog-end. START must be outside
+any 'inside' regions and at the beginning of a token."
   (if (>= start nxml-scan-end)
       nxml-scan-end
-    (goto-char start)
-    (nxml-move-outside-backwards)
-    (setq start (point))
     (let ((inside-remove-start start)
 	  xmltok-errors
 	  xmltok-dependent-regions)
@@ -211,7 +211,7 @@
 	      (setq adjusted-start ostart)))))
       (setq overlays (cdr overlays)))
     adjusted-start))
-		  
+
 (defun nxml-mark-parse-dependent-regions ()
   (while xmltok-dependent-regions
     (apply 'nxml-mark-parse-dependent-region
@@ -297,6 +297,20 @@
       (set-marker nxml-scan-end (point)))
     xmltok-type))
 
+(defun nxml-move-tag-backwards (bound)
+  "Move point backwards outside any 'inside' regions or tags, up
+to nxml-prolog-end. Point will either be at bound or a '<'
+character starting a tag outside any 'inside' regions. Ignores
+dependent regions. As a precondition, point must be >= bound."
+  (nxml-move-outside-backwards)
+  (when (not (equal (char-after) ?<))
+    (if (search-backward "<" bound t)
+        (progn
+          (nxml-move-outside-backwards)
+          (when (not (equal (char-after) ?<))
+            (search-backward "<" bound t)))
+      (goto-char bound))))
+
 (defun nxml-move-outside-backwards ()
   "Move point to first character of the containing special thing.
 Leave point unmoved if it is not inside anything special."
Index: rng-auto.el
===================================================================
--- rng-auto.el	(revision 44)
+++ rng-auto.el	(working copy)
@@ -106,12 +106,9 @@
 (autoload (quote nxml-mode) "nxml-mode" "\
 Major mode for editing XML.
 
-Syntax highlighting is performed unless the variable
-`nxml-syntax-highlight-flag' is nil.
-
 \\[nxml-finish-element] finishes the current element by inserting an end-tag.
 C-c C-i closes a start-tag with `>' and then inserts a balancing end-tag
-leaving point between the start-tag and end-tag. 
+leaving point between the start-tag and end-tag.
 \\[nxml-balanced-close-start-tag-block] is similar but for block rather than inline elements:
 the start-tag, point, and end-tag are all left on separate lines.
 If `nxml-slash-auto-complete-flag' is non-nil, then inserting a `</'
Index: nxml-mode.el
===================================================================
--- nxml-mode.el	(revision 44)
+++ nxml-mode.el	(working copy)
@@ -26,11 +26,6 @@
 
 ;; See nxml-rap.el for description of parsing strategy.
 
-;; The font locking here is independent of font-lock.el.  We want to
-;; do more sophisticated handling of changes and we want to use the
-;; same xmltok rather than regexps for parsing so that we parse
-;; consistently and correctly.
-
 ;;; Code:
 
 (when (featurep 'mucs)
@@ -55,11 +50,6 @@
   :group 'nxml
   :group 'font-lock-highlighting-faces)
 
-(defcustom nxml-syntax-highlight-flag t
-  "*Non-nil means nxml-mode should perform syntax highlighting."
-  :group 'nxml
-  :type 'boolean)
-
 (defcustom nxml-char-ref-display-glyph-flag t
   "*Non-nil means display glyph following character reference.
 The glyph is displayed in `nxml-glyph-face'.  The hook
@@ -99,8 +89,6 @@
   :group 'nxml
   :type 'integer)
 
-(defvar nxml-fontify-chunk-size 500)
-
 (defcustom nxml-bind-meta-tab-to-complete-flag (not window-system)
   "*Non-nil means bind M-TAB in `nxml-mode-map' to `nxml-complete'.
 C-return will be bound to `nxml-complete' in any case.
@@ -463,20 +451,14 @@
     map)
   "Keymap for nxml-mode.")
 
+(defvar nxml-font-lock-keywords
+  '(nxml-fontify-matcher)
+  "Default font lock keywords for nxml-mode")
+
 (defsubst nxml-set-face (start end face)
   (when (and face (< start end))
-    (put-text-property start end 'face face)))
+    (font-lock-append-text-property start end 'face face)))
 
-(defun nxml-clear-face (start end)
-  (remove-text-properties start end '(face nil))
-  (nxml-clear-char-ref-extra-display start end))
-
-(defsubst nxml-set-fontified (start end)
-  (put-text-property start end 'fontified t))
-
-(defsubst nxml-clear-fontified (start end)
-  (remove-text-properties start end '(fontified nil)))
-
 ;;;###autoload
 (defun nxml-mode ()
   ;; We use C-c C-i instead of \\[nxml-balanced-close-start-tag-inline]
@@ -484,9 +466,6 @@
   ;; not mnemonic.
   "Major mode for editing XML.
 
-Syntax highlighting is performed unless the variable
-`nxml-syntax-highlight-flag' is nil.
-
 \\[nxml-finish-element] finishes the current element by inserting an end-tag.
 C-c C-i closes a start-tag with `>' and then inserts a balancing end-tag
 leaving point between the start-tag and end-tag.
@@ -570,13 +549,9 @@
       (nxml-clear-dependent-regions (point-min) (point-max))
       (setq nxml-scan-end (copy-marker (point-min) nil))
       (nxml-with-unmodifying-text-property-changes
-	(when nxml-syntax-highlight-flag
-	  (nxml-clear-fontified (point-min) (point-max)))
-	(nxml-clear-inside (point-min) (point-max))
+        (nxml-clear-inside (point-min) (point-max))
 	(nxml-with-invisible-motion
 	  (nxml-scan-prolog)))))
-  (when nxml-syntax-highlight-flag
-    (add-hook 'fontification-functions 'nxml-fontify nil t))
   (add-hook 'after-change-functions 'nxml-after-change nil t)
   (add-hook 'write-contents-hooks 'nxml-prepare-to-save)
   (when (not (and (buffer-file-name) (file-exists-p (buffer-file-name))))
@@ -585,6 +560,19 @@
       (setq buffer-file-coding-system nxml-default-buffer-file-coding-system))
     (when nxml-auto-insert-xml-declaration-flag
       (nxml-insert-xml-declaration)))
+
+  (setq font-lock-defaults
+        '(nxml-font-lock-keywords
+          t    ; keywords-only; we highlight comments and strings here
+          nil  ; font-lock-keywords-case-fold-search. XML is case sensitive
+          nil  ; no special syntax table
+          nil  ; no automatic syntactic fontification
+          (font-lock-extend-after-change-region-function
+           . nxml-extend-after-change-region)
+          (font-lock-extend-region-functions . (nxml-extend-region))
+          (jit-lock-contextually . t)
+          (font-lock-unfontify-region-function . nxml-unfontify-region)))
+
   (run-hooks 'nxml-mode-hook))
 
 (defun nxml-degrade (context err)
@@ -598,85 +586,76 @@
     (save-restriction
       (widen)
       (nxml-with-unmodifying-text-property-changes
-	(nxml-clear-face (point-min) (point-max))
-	(nxml-set-fontified (point-min) (point-max))
-	(nxml-clear-inside (point-min) (point-max)))
+        (nxml-clear-inside (point-min) (point-max)))
       (setq mode-name "nXML/degraded"))))
 
 ;;; Change management
 
+(defun nxml-debug-region (start end)
+  (interactive "r")
+  (let ((font-lock-beg start)
+        (font-lock-end end))
+    (nxml-extend-region)
+    (goto-char font-lock-beg)
+    (set-mark font-lock-end)))
+
 (defun nxml-after-change (start end pre-change-length)
-  ;; Work around bug in insert-file-contents.
-  (when (> end (1+ (buffer-size)))
-    (setq start 1)
-    (setq end (1+ (buffer-size))))
-  (unless nxml-degraded
-    (condition-case err
-	(save-excursion
-	  (save-restriction
-	    (widen)
-	    (save-match-data
-	      (nxml-with-invisible-motion
-		(nxml-with-unmodifying-text-property-changes
-		  (nxml-after-change1 start end pre-change-length))))))
-      (error
-       (nxml-degrade 'nxml-after-change err)))))
+  ; in font-lock mode, nxml-after-change1 is called via
+  ; nxml-extend-after-change-region instead so that the updated
+  ; book-keeping information is available for fontification.
 
+  (unless (or font-lock-mode nxml-degraded)
+    (nxml-with-degradation-on-error 'nxml-after-change
+        (save-excursion
+          (save-restriction
+            (widen)
+            (save-match-data
+              (nxml-with-invisible-motion
+                (nxml-with-unmodifying-text-property-changes
+                  (nxml-after-change1
+                   start end pre-change-length)))))))))
+
 (defun nxml-after-change1 (start end pre-change-length)
-  (setq nxml-last-fontify-end nil)
+  "after-change book-keeping. returns a cons containing a
+possibly-enlarged change region. you must still call
+nxml-extend-region on this expanded region to obtain the full
+extent of the area needing refontification.
+
+For book-keeping, call this function even when fontification is
+disabled."
   (let ((pre-change-end (+ start pre-change-length)))
     (setq start
 	  (nxml-adjust-start-for-dependent-regions start
-						   end
-						   pre-change-length))
+                                                   end
+                                                   pre-change-length))
+
+    ;; If the prolog might have changed, rescan the prolog
     (when (<= start
-	      ;; Add 2 so as to include the < and following char
-	      ;; that start the instance, since changing these
-	      ;; can change where the prolog ends.
+	      ;; Add 2 so as to include the < and following char that
+	      ;; start the instance (document element), since changing
+	      ;; these can change where the prolog ends.
 	      (+ nxml-prolog-end 2))
-      ;; end must be extended to at least the end of the old prolog
+      ;; end must be extended to at least the end of the old prolog in
+      ;; case the new prolog is shorter
       (when (< pre-change-end nxml-prolog-end)
 	(setq end
 	      ;; don't let end get out of range even if pre-change-length
 	      ;; is bogus
 	      (min (point-max)
 		   (+ end (- nxml-prolog-end pre-change-end)))))
+
       (nxml-scan-prolog)))
-  (cond ((<= end nxml-prolog-end)
-	 (setq end nxml-prolog-end)
-	 (goto-char start)
-	 ;; This is so that Emacs redisplay works
-	 (setq start (line-beginning-position)))
-	((and (<= start nxml-scan-end)
-	      (> start (point-min))
-	      (nxml-get-inside (1- start)))
-	 ;; The closing delimiter might have been removed.
-	 ;; So we may need to redisplay from the beginning
-	 ;; of the token.
-	 (goto-char (1- start))
-	 (nxml-move-outside-backwards)
-	 ;; This is so that Emacs redisplay works
-	 (setq start (line-beginning-position))
-	 (setq end (max (nxml-scan-after-change (point) end)
-			end)))
-	(t
-	 (goto-char start)
-	 ;; This is both for redisplay and to move back
-	 ;; past any incomplete opening delimiters
-	 (setq start (line-beginning-position))
-	 (setq end (max (nxml-scan-after-change start end)
-			end))))
-  (when nxml-syntax-highlight-flag
-    (when (>= start end)
-      ;; Must clear at least one char so as to trigger redisplay.
-      (cond ((< start (point-max))
-	     (setq end (1+ start)))
-	    (t
-	     (setq end (point-max))
-	     (goto-char end)
-	     (setq start (line-beginning-position)))))
-    (nxml-clear-fontified start end)))
 
+  (when (> end nxml-prolog-end)
+    (goto-char start)
+    (nxml-move-tag-backwards (point-min))
+    (setq start (point))
+    (setq end (max (nxml-scan-after-change start end)
+                   end)))
+
+  (nxml-debug-change "nxml-after-change1" start end)
+  (cons start end))
+
 ;;; Encodings
 
 (defun nxml-insert-xml-declaration ()
@@ -862,59 +841,102 @@
 
 ;;; Fontification
 
-(defun nxml-fontify (start)
-  (condition-case err
-      (save-excursion
-	(save-restriction
-	  (widen)
-	  (save-match-data
-	    (nxml-with-invisible-motion
-	      (nxml-with-unmodifying-text-property-changes
-		(if (or nxml-degraded
-			;; just in case we get called in the wrong buffer
-			(not nxml-prolog-end))
-		    (nxml-set-fontified start (point-max))
-		  (nxml-fontify1 start)))))))
-    (error
-     (nxml-degrade 'nxml-fontify err))))
+(defun nxml-unfontify-region (start end)
+  (font-lock-default-unfontify-region start end)
+  (nxml-clear-char-ref-extra-display start end))
 
-(defun nxml-fontify1 (start)
-  (cond ((< start nxml-prolog-end)
-	 (nxml-fontify-prolog)
-	 (nxml-set-fontified (point-min)
-			     nxml-prolog-end))
-	(t
-	 (goto-char start)
-	 (when (not (eq nxml-last-fontify-end start))
-	   (when (not (equal (char-after) ?\<))
-	     (search-backward "<" nxml-prolog-end t))
-	   (nxml-ensure-scan-up-to-date)
-	   (nxml-move-outside-backwards))
-	 (let ((start (point)))
-	   (nxml-do-fontify (min (point-max)
-				 (+ start nxml-fontify-chunk-size)))
-	   (setq nxml-last-fontify-end (point))
-	   (nxml-set-fontified start nxml-last-fontify-end)))))
+(defun nxml-extend-region ()
+  "Extend the region to hold the minimum area we can fontify with
+nXML. Called with font-lock-beg and font-lock-end dynamically bound."
+  (let ((start font-lock-beg)
+        (end font-lock-end))
 
-(defun nxml-fontify-buffer ()
-  (interactive)
-  (save-excursion
-    (save-restriction
-      (widen)
-      (nxml-with-invisible-motion
-	(goto-char (point-min))
-	(nxml-with-unmodifying-text-property-changes
-	  (nxml-fontify-prolog)
-	  (goto-char nxml-prolog-end)
-	  (nxml-do-fontify))))))
+    (nxml-debug-change "nxml-extend-region(input)" start end)
 
+    (when (< start nxml-prolog-end)
+      (setq start (point-min)))
+
+    (cond ((<= end nxml-prolog-end)
+           (setq end nxml-prolog-end))
+
+          (t
+           (goto-char start)
+           ;; some font-lock backends (like Emacs 22 jit-lock) snap
+           ;; the region to the beginning of the line no matter what
+           ;; we say here. To mitigate the resulting excess
+           ;; fontification, ignore leading whitespace.
+           (skip-syntax-forward " ")
+
+           ;; find the beginning of the previous tag
+           (when (not (equal (char-after) ?\<))
+             (search-backward "<" nxml-prolog-end t))
+           (nxml-ensure-scan-up-to-date)
+           (nxml-move-outside-backwards)
+           (setq start (point))
+
+           (while (< (point) end)
+             (nxml-tokenize-forward))
+
+           (setq end (point))))
+
+    (when (or (< start font-lock-beg)
+              (> end font-lock-end))
+      (setq font-lock-beg start
+            font-lock-end end)
+      (nxml-debug-change "nxml-extend-region" start end)
+      t)))
+
+(defun nxml-extend-after-change-region (start end pre-change-length)
+  (unless nxml-degraded
+    (setq nxml-last-fontify-end nil)
+
+    (nxml-with-degradation-on-error 'nxml-extend-after-change-region
+	(save-excursion
+	  (save-restriction
+	    (widen)
+	    (save-match-data
+	      (nxml-with-invisible-motion
+		(nxml-with-unmodifying-text-property-changes
+                  (nxml-extend-after-change-region1
+                   start end pre-change-length)))))))))
+
+(defun nxml-extend-after-change-region1 (start end pre-change-length)
+  (let* ((region (nxml-after-change1 start end pre-change-length))
+         (font-lock-beg (car region))
+         (font-lock-end (cdr region)))
+
+    (nxml-extend-region)
+    (cons font-lock-beg font-lock-end)))
+
+(defun nxml-fontify-matcher (bound)
+  "Called as font-lock keyword matcher."
+
+  (unless nxml-degraded
+    (nxml-debug-change "nxml-fontify-matcher" (point) bound)
+
+    (when (< (point) nxml-prolog-end)
+      (goto-char (point-min))
+      (nxml-fontify-prolog)
+      (goto-char nxml-prolog-end))
+
+    (let (xmltok-dependent-regions
+          xmltok-errors)
+      (while (and (< (point) bound)
+                  (nxml-tokenize-forward))
+        (nxml-apply-fontify-rule)))
+
+    (setq nxml-last-fontify-end (point)))
+
+  ;; Since we did the fontification internally, tell font-lock to not
+  ;; do anything itself.
+  nil)
+
 (defun nxml-fontify-prolog ()
   "Fontify the prolog.
 The buffer is assumed to be prepared for fontification.
 This does not set the fontified property, but it does clear
 faces appropriately."
   (let ((regions nxml-prolog-regions))
-    (nxml-clear-face (point-min) nxml-prolog-end)
     (while regions
       (let ((region (car regions)))
 	(nxml-apply-fontify-rule (aref region 0)
@@ -922,17 +944,6 @@
 				 (aref region 2)))
       (setq regions (cdr regions)))))
 
-(defun nxml-do-fontify (&optional bound)
-  "Fontify at least as far as bound.
-Leave point after last fontified position."
-  (unless bound (setq bound (point-max)))
-  (let (xmltok-dependent-regions
-	xmltok-errors)
-    (while (and (< (point) bound)
-		(nxml-tokenize-forward))
-      (nxml-clear-face xmltok-start (point))
-      (nxml-apply-fontify-rule))))
-
 ;; Vectors identify a substring of the token to be highlighted in some face.
 
 ;; Token types returned by xmltok-forward.
@@ -2582,13 +2593,7 @@
 	       (> (prefix-numeric-value arg) 0))))
     (when (not (eq new nxml-char-ref-extra-display))
       (setq nxml-char-ref-extra-display new)
-      (save-excursion
-	(save-restriction
-	  (widen)
-	  (if nxml-char-ref-extra-display
-	      (nxml-with-unmodifying-text-property-changes
-		(nxml-clear-fontified (point-min) (point-max)))
-	    (nxml-clear-char-ref-extra-display (point-min) (point-max))))))))
+      (font-lock-fontify-buffer))))
 
 (put 'nxml-char-ref 'evaporate t)
 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-05-25 20:36   ` Daniel Colascione
@ 2008-05-26 14:52     ` Stefan Monnier
  2008-05-27 15:13       ` Daniel Colascione
  0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2008-05-26 14:52 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

>> Looks pretty good.  See some comments below.
> ...
>> > +  ;; Work around bug in insert-file-contents, apparently
> ...
>> Could you expand on this "bug in insert-file-contents"?
>> Maybe we can fix it.
> I think the only person who can comment on this piece is the person
> who wrote it.

Sorry, I didn't notice it came from the original code.

>> > +    (when (< (point) nxml-prolog-end)
>> > +      (goto-char (point-min))
>> > +      (nxml-fontify-prolog)
>> > +      (goto-char nxml-prolog-end))
>> 
>> Fontifying outside of (point)...bound is likely to lead to problems.

> I think this code is okay though, since nxml-extend-region *does* snap the 
> fontification region to (point-min), and nxml is written to fontify the 
> prolog by moving forward over the whole thing, fontifying along the way.

I see, then please add a comment explaining it.  You could also replace
the (goto-char (point-min)) by (assert (bobp)) to make sure that
nxml-extend-region did its job.

>> > +    (when (not (eq nxml-last-fontify-end (point)))
>> > +      (when (not (equal (char-after) ?\<))
>> > +        (search-backward "<" nxml-prolog-end t))
>> > +      (nxml-ensure-scan-up-to-date)
>> > +      (nxml-move-outside-backwards))
>> 
>> This should be done in nxml-extend-region instead. 

> OTOH, this chunk shouldn't be there.  You're right.

Great!  I love it when I'm right!

>     (let (xmltok-dependent-regions
>           xmltok-errors)
>       (while (and (< (point) bound)
>                   (nxml-tokenize-forward))
>         (nxml-apply-fontify-rule)))

> will cause problems if the last nxml token extends beyond bound.
> nxml-extend-region ensures that it doesn't, but can we count on it?

Presumably you can count on it.  Especially since no other
region-extend-function is run after nxml-extend-region.
You could also change the code to

    (let (xmltok-dependent-regions
          xmltok-errors)
      (while (and (nxml-tokenize-forward)
                  (< (point) bound))
        (nxml-apply-fontify-rule)))

so that in case of an error, the result is to fontify too-little rather
than too-much.  This tends to be more visible, so such a bug would
presumably be caught earlier.


        Stefan




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-05-26 14:52     ` Stefan Monnier
@ 2008-05-27 15:13       ` Daniel Colascione
  2008-05-27 15:37         ` Stefan Monnier
  0 siblings, 1 reply; 24+ messages in thread
From: Daniel Colascione @ 2008-05-27 15:13 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

On Monday 26 May 2008, Stefan Monnier wrote:
> >> Looks pretty good.  See some comments below.

Addressed. See new version of the patch.


[-- Attachment #2: nxml-mode-3.patch --]
[-- Type: text/x-diff, Size: 19797 bytes --]

Index: nxml-mode.el
===================================================================
--- nxml-mode.el	(revision 44)
+++ nxml-mode.el	(working copy)
@@ -26,11 +26,6 @@
 
 ;; See nxml-rap.el for description of parsing strategy.
 
-;; The font locking here is independent of font-lock.el.  We want to
-;; do more sophisticated handling of changes and we want to use the
-;; same xmltok rather than regexps for parsing so that we parse
-;; consistently and correctly.
-
 ;;; Code:
 
 (when (featurep 'mucs)
@@ -55,11 +50,6 @@
   :group 'nxml
   :group 'font-lock-highlighting-faces)
 
-(defcustom nxml-syntax-highlight-flag t
-  "*Non-nil means nxml-mode should perform syntax highlighting."
-  :group 'nxml
-  :type 'boolean)
-
 (defcustom nxml-char-ref-display-glyph-flag t
   "*Non-nil means display glyph following character reference.
 The glyph is displayed in `nxml-glyph-face'.  The hook
@@ -99,8 +89,6 @@
   :group 'nxml
   :type 'integer)
 
-(defvar nxml-fontify-chunk-size 500)
-
 (defcustom nxml-bind-meta-tab-to-complete-flag (not window-system)
   "*Non-nil means bind M-TAB in `nxml-mode-map' to `nxml-complete'.
 C-return will be bound to `nxml-complete' in any case.
@@ -463,20 +451,14 @@
     map)
   "Keymap for nxml-mode.")
 
+(defvar nxml-font-lock-keywords
+  '(nxml-fontify-matcher)
+  "Default font lock keywords for nxml-mode")
+
 (defsubst nxml-set-face (start end face)
   (when (and face (< start end))
-    (put-text-property start end 'face face)))
+    (font-lock-append-text-property start end 'face face)))
 
-(defun nxml-clear-face (start end)
-  (remove-text-properties start end '(face nil))
-  (nxml-clear-char-ref-extra-display start end))
-
-(defsubst nxml-set-fontified (start end)
-  (put-text-property start end 'fontified t))
-
-(defsubst nxml-clear-fontified (start end)
-  (remove-text-properties start end '(fontified nil)))
-
 ;;;###autoload
 (defun nxml-mode ()
   ;; We use C-c C-i instead of \\[nxml-balanced-close-start-tag-inline]
@@ -484,9 +466,6 @@
   ;; not mnemonic.
   "Major mode for editing XML.
 
-Syntax highlighting is performed unless the variable
-`nxml-syntax-highlight-flag' is nil.
-
 \\[nxml-finish-element] finishes the current element by inserting an end-tag.
 C-c C-i closes a start-tag with `>' and then inserts a balancing end-tag
 leaving point between the start-tag and end-tag.
@@ -570,13 +549,9 @@
       (nxml-clear-dependent-regions (point-min) (point-max))
       (setq nxml-scan-end (copy-marker (point-min) nil))
       (nxml-with-unmodifying-text-property-changes
-	(when nxml-syntax-highlight-flag
-	  (nxml-clear-fontified (point-min) (point-max)))
-	(nxml-clear-inside (point-min) (point-max))
+        (nxml-clear-inside (point-min) (point-max))
 	(nxml-with-invisible-motion
 	  (nxml-scan-prolog)))))
-  (when nxml-syntax-highlight-flag
-    (add-hook 'fontification-functions 'nxml-fontify nil t))
   (add-hook 'after-change-functions 'nxml-after-change nil t)
   (add-hook 'write-contents-hooks 'nxml-prepare-to-save)
   (when (not (and (buffer-file-name) (file-exists-p (buffer-file-name))))
@@ -585,6 +560,19 @@
       (setq buffer-file-coding-system nxml-default-buffer-file-coding-system))
     (when nxml-auto-insert-xml-declaration-flag
       (nxml-insert-xml-declaration)))
+
+  (setq font-lock-defaults
+        '(nxml-font-lock-keywords
+          t    ; keywords-only; we highlight comments and strings here
+          nil  ; font-lock-keywords-case-fold-search. XML is case sensitive
+          nil  ; no special syntax table
+          nil  ; no automatic syntactic fontification
+          (font-lock-extend-after-change-region-function
+           . nxml-extend-after-change-region)
+          (font-lock-extend-region-functions . (nxml-extend-region))
+          (jit-lock-contextually . t)
+          (font-lock-unfontify-region-function . nxml-unfontify-region)))
+
   (run-hooks 'nxml-mode-hook))
 
 (defun nxml-degrade (context err)
@@ -598,85 +586,77 @@
     (save-restriction
       (widen)
       (nxml-with-unmodifying-text-property-changes
-	(nxml-clear-face (point-min) (point-max))
-	(nxml-set-fontified (point-min) (point-max))
-	(nxml-clear-inside (point-min) (point-max)))
+        (nxml-clear-inside (point-min) (point-max)))
       (setq mode-name "nXML/degraded"))))
 
 ;;; Change management
 
+(defun nxml-debug-region (start end)
+  (interactive "r")
+  (let ((font-lock-beg start)
+        (font-lock-end end))
+    (nxml-extend-region)
+    (goto-char font-lock-beg)
+    (set-mark font-lock-end)))
+
 (defun nxml-after-change (start end pre-change-length)
-  ;; Work around bug in insert-file-contents.
-  (when (> end (1+ (buffer-size)))
-    (setq start 1)
-    (setq end (1+ (buffer-size))))
-  (unless nxml-degraded
-    (condition-case err
-	(save-excursion
-	  (save-restriction
-	    (widen)
-	    (save-match-data
-	      (nxml-with-invisible-motion
-		(nxml-with-unmodifying-text-property-changes
-		  (nxml-after-change1 start end pre-change-length))))))
-      (error
-       (nxml-degrade 'nxml-after-change err)))))
+  ; in font-lock mode, nxml-after-change1 is called via
+  ; nxml-extend-after-change-region instead so that the updated
+  ; book-keeping information is available for fontification.
 
+  (unless (or font-lock-mode nxml-degraded)
+    (nxml-with-degradation-on-error 'nxml-after-change
+        (save-excursion
+          (save-restriction
+            (widen)
+            (save-match-data
+              (nxml-with-invisible-motion
+                (nxml-with-unmodifying-text-property-changes
+                  (nxml-after-change1
+                   start end pre-change-length)))))))))
+
 (defun nxml-after-change1 (start end pre-change-length)
-  (setq nxml-last-fontify-end nil)
+  "after-change book-keeping. returns a cons containing a
+possibly-enlarged change region. you must still call
+nxml-extend-region on this expanded region to obtain the full
+extent of the area needing refontification.
+
+For book-keeping, call this function even when fontification is
+disabled."
   (let ((pre-change-end (+ start pre-change-length)))
     (setq start
 	  (nxml-adjust-start-for-dependent-regions start
-						   end
-						   pre-change-length))
+                                                   end
+                                                   pre-change-length))
+
+    ;; If the prolog might have changed, rescan the prolog
     (when (<= start
-	      ;; Add 2 so as to include the < and following char
-	      ;; that start the instance, since changing these
-	      ;; can change where the prolog ends.
+	      ;; Add 2 so as to include the < and following char that
+	      ;; start the instance (document element), since changing
+	      ;; these can change where the prolog ends.
 	      (+ nxml-prolog-end 2))
-      ;; end must be extended to at least the end of the old prolog
+      ;; end must be extended to at least the end of the old prolog in
+      ;; case the new prolog is shorter
       (when (< pre-change-end nxml-prolog-end)
 	(setq end
 	      ;; don't let end get out of range even if pre-change-length
 	      ;; is bogus
 	      (min (point-max)
 		   (+ end (- nxml-prolog-end pre-change-end)))))
-      (nxml-scan-prolog)))
-  (cond ((<= end nxml-prolog-end)
-	 (setq end nxml-prolog-end)
-	 (goto-char start)
-	 ;; This is so that Emacs redisplay works
-	 (setq start (line-beginning-position)))
-	((and (<= start nxml-scan-end)
-	      (> start (point-min))
-	      (nxml-get-inside (1- start)))
-	 ;; The closing delimiter might have been removed.
-	 ;; So we may need to redisplay from the beginning
-	 ;; of the token.
-	 (goto-char (1- start))
-	 (nxml-move-outside-backwards)
-	 ;; This is so that Emacs redisplay works
-	 (setq start (line-beginning-position))
-	 (setq end (max (nxml-scan-after-change (point) end)
-			end)))
-	(t
-	 (goto-char start)
-	 ;; This is both for redisplay and to move back
-	 ;; past any incomplete opening delimiters
-	 (setq start (line-beginning-position))
-	 (setq end (max (nxml-scan-after-change start end)
-			end))))
-  (when nxml-syntax-highlight-flag
-    (when (>= start end)
-      ;; Must clear at least one char so as to trigger redisplay.
-      (cond ((< start (point-max))
-	     (setq end (1+ start)))
-	    (t
-	     (setq end (point-max))
-	     (goto-char end)
-	     (setq start (line-beginning-position)))))
-    (nxml-clear-fontified start end)))
 
+      (nxml-scan-prolog)
+      (setq start (point-min))))
+
+  (when (> end nxml-prolog-end)
+    (goto-char start)
+    (nxml-move-tag-backwards (point-min))
+    (setq start (point))
+    (setq end (max (nxml-scan-after-change start end)
+                   end)))
+
+  (nxml-debug-change "nxml-after-change1" start end)
+  (cons start end))
+
 ;;; Encodings
 
 (defun nxml-insert-xml-declaration ()
@@ -862,59 +842,102 @@
 
 ;;; Fontification
 
-(defun nxml-fontify (start)
-  (condition-case err
-      (save-excursion
-	(save-restriction
-	  (widen)
-	  (save-match-data
-	    (nxml-with-invisible-motion
-	      (nxml-with-unmodifying-text-property-changes
-		(if (or nxml-degraded
-			;; just in case we get called in the wrong buffer
-			(not nxml-prolog-end))
-		    (nxml-set-fontified start (point-max))
-		  (nxml-fontify1 start)))))))
-    (error
-     (nxml-degrade 'nxml-fontify err))))
+(defun nxml-unfontify-region (start end)
+  (font-lock-default-unfontify-region start end)
+  (nxml-clear-char-ref-extra-display start end))
 
-(defun nxml-fontify1 (start)
-  (cond ((< start nxml-prolog-end)
-	 (nxml-fontify-prolog)
-	 (nxml-set-fontified (point-min)
-			     nxml-prolog-end))
-	(t
-	 (goto-char start)
-	 (when (not (eq nxml-last-fontify-end start))
-	   (when (not (equal (char-after) ?\<))
-	     (search-backward "<" nxml-prolog-end t))
-	   (nxml-ensure-scan-up-to-date)
-	   (nxml-move-outside-backwards))
-	 (let ((start (point)))
-	   (nxml-do-fontify (min (point-max)
-				 (+ start nxml-fontify-chunk-size)))
-	   (setq nxml-last-fontify-end (point))
-	   (nxml-set-fontified start nxml-last-fontify-end)))))
+(defun nxml-extend-region ()
+  "Extend the region to hold the minimum area we can fontify with
+nXML. Called with font-lock-beg and font-lock-end dynamically bound."
+  (let ((start font-lock-beg)
+        (end font-lock-end))
 
-(defun nxml-fontify-buffer ()
-  (interactive)
-  (save-excursion
-    (save-restriction
-      (widen)
-      (nxml-with-invisible-motion
-	(goto-char (point-min))
-	(nxml-with-unmodifying-text-property-changes
-	  (nxml-fontify-prolog)
-	  (goto-char nxml-prolog-end)
-	  (nxml-do-fontify))))))
+    (nxml-debug-change "nxml-extend-region(input)" start end)
 
+    (when (< start nxml-prolog-end)
+      (setq start (point-min)))
+
+    (cond ((<= end nxml-prolog-end)
+           (setq end nxml-prolog-end))
+
+          (t
+           (goto-char start)
+           ;; some font-lock backends (like Emacs 22 jit-lock) snap
+           ;; the region to the beginning of the line no matter what
+           ;; we say here. To mitigate the resulting excess
+           ;; fontification, ignore leading whitespace.
+           (skip-syntax-forward " ")
+
+           ;; find the beginning of the previous tag
+           (when (not (equal (char-after) ?\<))
+             (search-backward "<" nxml-prolog-end t))
+           (nxml-ensure-scan-up-to-date)
+           (nxml-move-outside-backwards)
+           (setq start (point))
+
+           (while (< (point) end)
+             (nxml-tokenize-forward))
+
+           (setq end (point))))
+
+    (when (or (< start font-lock-beg)
+              (> end font-lock-end))
+      (setq font-lock-beg start
+            font-lock-end end)
+      (nxml-debug-change "nxml-extend-region" start end)
+      t)))
+
+(defun nxml-extend-after-change-region (start end pre-change-length)
+  (unless nxml-degraded
+    (setq nxml-last-fontify-end nil)
+
+    (nxml-with-degradation-on-error 'nxml-extend-after-change-region
+	(save-excursion
+	  (save-restriction
+	    (widen)
+	    (save-match-data
+	      (nxml-with-invisible-motion
+		(nxml-with-unmodifying-text-property-changes
+                  (nxml-extend-after-change-region1
+                   start end pre-change-length)))))))))
+
+(defun nxml-extend-after-change-region1 (start end pre-change-length)
+  (let* ((region (nxml-after-change1 start end pre-change-length))
+         (font-lock-beg (car region))
+         (font-lock-end (cdr region)))
+
+    (nxml-extend-region)
+    (cons font-lock-beg font-lock-end)))
+
+(defun nxml-fontify-matcher (bound)
+  "Called as font-lock keyword matcher."
+
+  (unless nxml-degraded
+    (nxml-debug-change "nxml-fontify-matcher" (point) bound)
+
+    (when (< (point) nxml-prolog-end)
+      (assert (bobp))
+      (nxml-fontify-prolog)
+      (goto-char nxml-prolog-end))
+
+    (let (xmltok-dependent-regions
+          xmltok-errors)
+      (while (and (nxml-tokenize-forward)
+                  (<= (point) bound)) ; intervals are open-ended
+        (nxml-apply-fontify-rule)))
+
+    (setq nxml-last-fontify-end (point)))
+
+  ;; Since we did the fontification internally, tell font-lock to not
+  ;; do anything itself.
+  nil)
+
 (defun nxml-fontify-prolog ()
   "Fontify the prolog.
 The buffer is assumed to be prepared for fontification.
 This does not set the fontified property, but it does clear
 faces appropriately."
   (let ((regions nxml-prolog-regions))
-    (nxml-clear-face (point-min) nxml-prolog-end)
     (while regions
       (let ((region (car regions)))
 	(nxml-apply-fontify-rule (aref region 0)
@@ -922,17 +945,6 @@
 				 (aref region 2)))
       (setq regions (cdr regions)))))
 
-(defun nxml-do-fontify (&optional bound)
-  "Fontify at least as far as bound.
-Leave point after last fontified position."
-  (unless bound (setq bound (point-max)))
-  (let (xmltok-dependent-regions
-	xmltok-errors)
-    (while (and (< (point) bound)
-		(nxml-tokenize-forward))
-      (nxml-clear-face xmltok-start (point))
-      (nxml-apply-fontify-rule))))
-
 ;; Vectors identify a substring of the token to be highlighted in some face.
 
 ;; Token types returned by xmltok-forward.
@@ -2582,13 +2594,7 @@
 	       (> (prefix-numeric-value arg) 0))))
     (when (not (eq new nxml-char-ref-extra-display))
       (setq nxml-char-ref-extra-display new)
-      (save-excursion
-	(save-restriction
-	  (widen)
-	  (if nxml-char-ref-extra-display
-	      (nxml-with-unmodifying-text-property-changes
-		(nxml-clear-fontified (point-min) (point-max)))
-	    (nxml-clear-char-ref-extra-display (point-min) (point-max))))))))
+      (font-lock-fontify-buffer))))
 
 (put 'nxml-char-ref 'evaporate t)
 
Index: nxml-util.el
===================================================================
--- nxml-util.el	(revision 44)
+++ nxml-util.el	(working copy)
@@ -24,6 +24,35 @@
 
 ;;; Code:
 
+(defconst nxml-debug nil
+  "enable nxml debugging. effective only at compile time")
+
+(eval-when-compile
+  (require 'cl))
+
+(defsubst nxml-debug (format &rest args)
+  (when nxml-debug
+    (apply #'message format args)))
+
+(defmacro nxml-debug-change (name start end)
+  (when nxml-debug
+    `(nxml-debug "%s: %S" ,name
+                (buffer-substring-no-properties ,start ,end))))
+
+(defmacro nxml-debug-set-inside (start end)
+  (when nxml-debug
+    `(let ((overlay (make-overlay ,start ,end)))
+       (overlay-put overlay 'face '(:background "red"))
+       (overlay-put overlay 'nxml-inside-debug t)
+       (nxml-debug-change "nxml-set-inside" ,start ,end))))
+
+(defmacro nxml-debug-clear-inside (start end)
+  (when nxml-debug
+    `(loop for overlay in (overlays-in ,start ,end)
+           if (overlay-get overlay 'nxml-inside-debug)
+           do (delete-overlay overlay)
+           finally (nxml-debug-change "nxml-clear-inside" ,start ,end))))
+
 (defun nxml-make-namespace (str)
   "Return a symbol for the namespace URI STR.
 STR must be a string. If STR is the empty string, return nil.
@@ -37,12 +66,21 @@
 This is the inverse of `nxml-make-namespace'."
   (and ns (substring (symbol-name ns) 1)))
 
-(defconst nxml-xml-namespace-uri 
+(defconst nxml-xml-namespace-uri
   (nxml-make-namespace "http://www.w3.org/XML/1998/namespace"))
 
 (defconst nxml-xmlns-namespace-uri
   (nxml-make-namespace "http://www.w3.org/2000/xmlns/"))
 
+(defmacro nxml-with-degradation-on-error (context &rest body)
+  (if (not nxml-debug)
+      (let ((error-symbol (gensym "err")))
+        `(condition-case ,error-symbol
+             (progn ,@body)
+           (error
+            (nxml-degrade ,context ,error-symbol))))
+    `(progn ,@body)))
+
 (defmacro nxml-with-unmodifying-text-property-changes (&rest body)
   "Evaluate BODY without any text property changes modifying the buffer.
 Any text properties changes happen as usual but the changes are not treated as
Index: nxml-rap.el
===================================================================
--- nxml-rap.el	(revision 44)
+++ nxml-rap.el	(working copy)
@@ -110,9 +110,11 @@
   (get-text-property pos 'nxml-inside))
 
 (defsubst nxml-clear-inside (start end)
+  (nxml-debug-clear-inside start end)
   (remove-text-properties start end '(nxml-inside nil)))
 
 (defsubst nxml-set-inside (start end type)
+  (nxml-debug-set-inside start end)
   (put-text-property start end 'nxml-inside type))
 
 (defun nxml-inside-end (pos)
@@ -137,12 +139,10 @@
   "Restore `nxml-scan-end' invariants after a change.
 The change happened between START and END.
 Return position after which lexical state is unchanged.
-END must be > nxml-prolog-end."
+END must be > nxml-prolog-end. START must be outside
+any 'inside' regions and at the beginning of a token."
   (if (>= start nxml-scan-end)
       nxml-scan-end
-    (goto-char start)
-    (nxml-move-outside-backwards)
-    (setq start (point))
     (let ((inside-remove-start start)
 	  xmltok-errors
 	  xmltok-dependent-regions)
@@ -211,7 +211,7 @@
 	      (setq adjusted-start ostart)))))
       (setq overlays (cdr overlays)))
     adjusted-start))
-		  
+
 (defun nxml-mark-parse-dependent-regions ()
   (while xmltok-dependent-regions
     (apply 'nxml-mark-parse-dependent-region
@@ -297,6 +297,20 @@
       (set-marker nxml-scan-end (point)))
     xmltok-type))
 
+(defun nxml-move-tag-backwards (bound)
+  "Move point backwards outside any 'inside' regions or tags, up
+to nxml-prolog-end. Point will either be at bound or a '<'
+character starting a tag outside any 'inside' regions. Ignores
+dependent regions. As a precondition, point must be >= bound."
+  (nxml-move-outside-backwards)
+  (when (not (equal (char-after) ?<))
+    (if (search-backward "<" bound t)
+        (progn
+          (nxml-move-outside-backwards)
+          (when (not (equal (char-after) ?<))
+            (search-backward "<" bound t)))
+      (goto-char bound))))
+
 (defun nxml-move-outside-backwards ()
   "Move point to first character of the containing special thing.
 Leave point unmoved if it is not inside anything special."
Index: rng-auto.el
===================================================================
--- rng-auto.el	(revision 44)
+++ rng-auto.el	(working copy)
@@ -106,12 +106,9 @@
 (autoload (quote nxml-mode) "nxml-mode" "\
 Major mode for editing XML.
 
-Syntax highlighting is performed unless the variable
-`nxml-syntax-highlight-flag' is nil.
-
 \\[nxml-finish-element] finishes the current element by inserting an end-tag.
 C-c C-i closes a start-tag with `>' and then inserts a balancing end-tag
-leaving point between the start-tag and end-tag. 
+leaving point between the start-tag and end-tag.
 \\[nxml-balanced-close-start-tag-block] is similar but for block rather than inline elements:
 the start-tag, point, and end-tag are all left on separate lines.
 If `nxml-slash-auto-complete-flag' is non-nil, then inserting a `</'

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-05-27 15:13       ` Daniel Colascione
@ 2008-05-27 15:37         ` Stefan Monnier
  2008-05-27 15:45           ` Daniel Colascione
  0 siblings, 1 reply; 24+ messages in thread
From: Stefan Monnier @ 2008-05-27 15:37 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

> Addressed. See new version of the patch.

OK, let's wait for the paperwork, then.

> +    (when (< (point) nxml-prolog-end)
> +      (assert (bobp))
> +      (nxml-fontify-prolog)
> +      (goto-char nxml-prolog-end))

This code needs a comment explaining why (bobp) should be true.


        Stefan




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-05-27 15:37         ` Stefan Monnier
@ 2008-05-27 15:45           ` Daniel Colascione
  2008-05-27 18:37             ` Stefan Monnier
       [not found]             ` <jwv8wxj8pf9.fsf-monnier+emacs@gnu.org>
  0 siblings, 2 replies; 24+ messages in thread
From: Daniel Colascione @ 2008-05-27 15:45 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

On Tuesday 27 May 2008, Stefan Monnier wrote:
> > Addressed. See new version of the patch.
>
> OK, let's wait for the paperwork, then.

Great. :-)

>
> > +    (when (< (point) nxml-prolog-end)
> > +      (assert (bobp))
> > +      (nxml-fontify-prolog)
> > +      (goto-char nxml-prolog-end))
>
> This code needs a comment explaining why (bobp) should be true.

Done. I thought the nxml-fontify-prolog next to (assert (bobp)) would tell the 
reader "Oh, nxml-fontify-prolog needs to start at the beginning of the 
buffer"

[-- Attachment #2: nxml-mode-4.patch --]
[-- Type: text/x-diff, Size: 19908 bytes --]

Index: nxml-mode.el
===================================================================
--- nxml-mode.el	(revision 44)
+++ nxml-mode.el	(working copy)
@@ -26,11 +26,6 @@
 
 ;; See nxml-rap.el for description of parsing strategy.
 
-;; The font locking here is independent of font-lock.el.  We want to
-;; do more sophisticated handling of changes and we want to use the
-;; same xmltok rather than regexps for parsing so that we parse
-;; consistently and correctly.
-
 ;;; Code:
 
 (when (featurep 'mucs)
@@ -55,11 +50,6 @@
   :group 'nxml
   :group 'font-lock-highlighting-faces)
 
-(defcustom nxml-syntax-highlight-flag t
-  "*Non-nil means nxml-mode should perform syntax highlighting."
-  :group 'nxml
-  :type 'boolean)
-
 (defcustom nxml-char-ref-display-glyph-flag t
   "*Non-nil means display glyph following character reference.
 The glyph is displayed in `nxml-glyph-face'.  The hook
@@ -99,8 +89,6 @@
   :group 'nxml
   :type 'integer)
 
-(defvar nxml-fontify-chunk-size 500)
-
 (defcustom nxml-bind-meta-tab-to-complete-flag (not window-system)
   "*Non-nil means bind M-TAB in `nxml-mode-map' to `nxml-complete'.
 C-return will be bound to `nxml-complete' in any case.
@@ -463,20 +451,14 @@
     map)
   "Keymap for nxml-mode.")
 
+(defvar nxml-font-lock-keywords
+  '(nxml-fontify-matcher)
+  "Default font lock keywords for nxml-mode")
+
 (defsubst nxml-set-face (start end face)
   (when (and face (< start end))
-    (put-text-property start end 'face face)))
+    (font-lock-append-text-property start end 'face face)))
 
-(defun nxml-clear-face (start end)
-  (remove-text-properties start end '(face nil))
-  (nxml-clear-char-ref-extra-display start end))
-
-(defsubst nxml-set-fontified (start end)
-  (put-text-property start end 'fontified t))
-
-(defsubst nxml-clear-fontified (start end)
-  (remove-text-properties start end '(fontified nil)))
-
 ;;;###autoload
 (defun nxml-mode ()
   ;; We use C-c C-i instead of \\[nxml-balanced-close-start-tag-inline]
@@ -484,9 +466,6 @@
   ;; not mnemonic.
   "Major mode for editing XML.
 
-Syntax highlighting is performed unless the variable
-`nxml-syntax-highlight-flag' is nil.
-
 \\[nxml-finish-element] finishes the current element by inserting an end-tag.
 C-c C-i closes a start-tag with `>' and then inserts a balancing end-tag
 leaving point between the start-tag and end-tag.
@@ -570,13 +549,9 @@
       (nxml-clear-dependent-regions (point-min) (point-max))
       (setq nxml-scan-end (copy-marker (point-min) nil))
       (nxml-with-unmodifying-text-property-changes
-	(when nxml-syntax-highlight-flag
-	  (nxml-clear-fontified (point-min) (point-max)))
-	(nxml-clear-inside (point-min) (point-max))
+        (nxml-clear-inside (point-min) (point-max))
 	(nxml-with-invisible-motion
 	  (nxml-scan-prolog)))))
-  (when nxml-syntax-highlight-flag
-    (add-hook 'fontification-functions 'nxml-fontify nil t))
   (add-hook 'after-change-functions 'nxml-after-change nil t)
   (add-hook 'write-contents-hooks 'nxml-prepare-to-save)
   (when (not (and (buffer-file-name) (file-exists-p (buffer-file-name))))
@@ -585,6 +560,19 @@
       (setq buffer-file-coding-system nxml-default-buffer-file-coding-system))
     (when nxml-auto-insert-xml-declaration-flag
       (nxml-insert-xml-declaration)))
+
+  (setq font-lock-defaults
+        '(nxml-font-lock-keywords
+          t    ; keywords-only; we highlight comments and strings here
+          nil  ; font-lock-keywords-case-fold-search. XML is case sensitive
+          nil  ; no special syntax table
+          nil  ; no automatic syntactic fontification
+          (font-lock-extend-after-change-region-function
+           . nxml-extend-after-change-region)
+          (font-lock-extend-region-functions . (nxml-extend-region))
+          (jit-lock-contextually . t)
+          (font-lock-unfontify-region-function . nxml-unfontify-region)))
+
   (run-hooks 'nxml-mode-hook))
 
 (defun nxml-degrade (context err)
@@ -598,85 +586,77 @@
     (save-restriction
       (widen)
       (nxml-with-unmodifying-text-property-changes
-	(nxml-clear-face (point-min) (point-max))
-	(nxml-set-fontified (point-min) (point-max))
-	(nxml-clear-inside (point-min) (point-max)))
+        (nxml-clear-inside (point-min) (point-max)))
       (setq mode-name "nXML/degraded"))))
 
 ;;; Change management
 
+(defun nxml-debug-region (start end)
+  (interactive "r")
+  (let ((font-lock-beg start)
+        (font-lock-end end))
+    (nxml-extend-region)
+    (goto-char font-lock-beg)
+    (set-mark font-lock-end)))
+
 (defun nxml-after-change (start end pre-change-length)
-  ;; Work around bug in insert-file-contents.
-  (when (> end (1+ (buffer-size)))
-    (setq start 1)
-    (setq end (1+ (buffer-size))))
-  (unless nxml-degraded
-    (condition-case err
-	(save-excursion
-	  (save-restriction
-	    (widen)
-	    (save-match-data
-	      (nxml-with-invisible-motion
-		(nxml-with-unmodifying-text-property-changes
-		  (nxml-after-change1 start end pre-change-length))))))
-      (error
-       (nxml-degrade 'nxml-after-change err)))))
+  ; in font-lock mode, nxml-after-change1 is called via
+  ; nxml-extend-after-change-region instead so that the updated
+  ; book-keeping information is available for fontification.
 
+  (unless (or font-lock-mode nxml-degraded)
+    (nxml-with-degradation-on-error 'nxml-after-change
+        (save-excursion
+          (save-restriction
+            (widen)
+            (save-match-data
+              (nxml-with-invisible-motion
+                (nxml-with-unmodifying-text-property-changes
+                  (nxml-after-change1
+                   start end pre-change-length)))))))))
+
 (defun nxml-after-change1 (start end pre-change-length)
-  (setq nxml-last-fontify-end nil)
+  "after-change book-keeping. returns a cons containing a
+possibly-enlarged change region. you must still call
+nxml-extend-region on this expanded region to obtain the full
+extent of the area needing refontification.
+
+For book-keeping, call this function even when fontification is
+disabled."
   (let ((pre-change-end (+ start pre-change-length)))
     (setq start
 	  (nxml-adjust-start-for-dependent-regions start
-						   end
-						   pre-change-length))
+                                                   end
+                                                   pre-change-length))
+
+    ;; If the prolog might have changed, rescan the prolog
     (when (<= start
-	      ;; Add 2 so as to include the < and following char
-	      ;; that start the instance, since changing these
-	      ;; can change where the prolog ends.
+	      ;; Add 2 so as to include the < and following char that
+	      ;; start the instance (document element), since changing
+	      ;; these can change where the prolog ends.
 	      (+ nxml-prolog-end 2))
-      ;; end must be extended to at least the end of the old prolog
+      ;; end must be extended to at least the end of the old prolog in
+      ;; case the new prolog is shorter
       (when (< pre-change-end nxml-prolog-end)
 	(setq end
 	      ;; don't let end get out of range even if pre-change-length
 	      ;; is bogus
 	      (min (point-max)
 		   (+ end (- nxml-prolog-end pre-change-end)))))
-      (nxml-scan-prolog)))
-  (cond ((<= end nxml-prolog-end)
-	 (setq end nxml-prolog-end)
-	 (goto-char start)
-	 ;; This is so that Emacs redisplay works
-	 (setq start (line-beginning-position)))
-	((and (<= start nxml-scan-end)
-	      (> start (point-min))
-	      (nxml-get-inside (1- start)))
-	 ;; The closing delimiter might have been removed.
-	 ;; So we may need to redisplay from the beginning
-	 ;; of the token.
-	 (goto-char (1- start))
-	 (nxml-move-outside-backwards)
-	 ;; This is so that Emacs redisplay works
-	 (setq start (line-beginning-position))
-	 (setq end (max (nxml-scan-after-change (point) end)
-			end)))
-	(t
-	 (goto-char start)
-	 ;; This is both for redisplay and to move back
-	 ;; past any incomplete opening delimiters
-	 (setq start (line-beginning-position))
-	 (setq end (max (nxml-scan-after-change start end)
-			end))))
-  (when nxml-syntax-highlight-flag
-    (when (>= start end)
-      ;; Must clear at least one char so as to trigger redisplay.
-      (cond ((< start (point-max))
-	     (setq end (1+ start)))
-	    (t
-	     (setq end (point-max))
-	     (goto-char end)
-	     (setq start (line-beginning-position)))))
-    (nxml-clear-fontified start end)))
 
+      (nxml-scan-prolog)
+      (setq start (point-min))))
+
+  (when (> end nxml-prolog-end)
+    (goto-char start)
+    (nxml-move-tag-backwards (point-min))
+    (setq start (point))
+    (setq end (max (nxml-scan-after-change start end)
+                   end)))
+
+  (nxml-debug-change "nxml-after-change1" start end)
+  (cons start end))
+
 ;;; Encodings
 
 (defun nxml-insert-xml-declaration ()
@@ -862,59 +842,104 @@
 
 ;;; Fontification
 
-(defun nxml-fontify (start)
-  (condition-case err
-      (save-excursion
-	(save-restriction
-	  (widen)
-	  (save-match-data
-	    (nxml-with-invisible-motion
-	      (nxml-with-unmodifying-text-property-changes
-		(if (or nxml-degraded
-			;; just in case we get called in the wrong buffer
-			(not nxml-prolog-end))
-		    (nxml-set-fontified start (point-max))
-		  (nxml-fontify1 start)))))))
-    (error
-     (nxml-degrade 'nxml-fontify err))))
+(defun nxml-unfontify-region (start end)
+  (font-lock-default-unfontify-region start end)
+  (nxml-clear-char-ref-extra-display start end))
 
-(defun nxml-fontify1 (start)
-  (cond ((< start nxml-prolog-end)
-	 (nxml-fontify-prolog)
-	 (nxml-set-fontified (point-min)
-			     nxml-prolog-end))
-	(t
-	 (goto-char start)
-	 (when (not (eq nxml-last-fontify-end start))
-	   (when (not (equal (char-after) ?\<))
-	     (search-backward "<" nxml-prolog-end t))
-	   (nxml-ensure-scan-up-to-date)
-	   (nxml-move-outside-backwards))
-	 (let ((start (point)))
-	   (nxml-do-fontify (min (point-max)
-				 (+ start nxml-fontify-chunk-size)))
-	   (setq nxml-last-fontify-end (point))
-	   (nxml-set-fontified start nxml-last-fontify-end)))))
+(defun nxml-extend-region ()
+  "Extend the region to hold the minimum area we can fontify with
+nXML. Called with font-lock-beg and font-lock-end dynamically bound."
+  (let ((start font-lock-beg)
+        (end font-lock-end))
 
-(defun nxml-fontify-buffer ()
-  (interactive)
-  (save-excursion
-    (save-restriction
-      (widen)
-      (nxml-with-invisible-motion
-	(goto-char (point-min))
-	(nxml-with-unmodifying-text-property-changes
-	  (nxml-fontify-prolog)
-	  (goto-char nxml-prolog-end)
-	  (nxml-do-fontify))))))
+    (nxml-debug-change "nxml-extend-region(input)" start end)
 
+    (when (< start nxml-prolog-end)
+      (setq start (point-min)))
+
+    (cond ((<= end nxml-prolog-end)
+           (setq end nxml-prolog-end))
+
+          (t
+           (goto-char start)
+           ;; some font-lock backends (like Emacs 22 jit-lock) snap
+           ;; the region to the beginning of the line no matter what
+           ;; we say here. To mitigate the resulting excess
+           ;; fontification, ignore leading whitespace.
+           (skip-syntax-forward " ")
+
+           ;; find the beginning of the previous tag
+           (when (not (equal (char-after) ?\<))
+             (search-backward "<" nxml-prolog-end t))
+           (nxml-ensure-scan-up-to-date)
+           (nxml-move-outside-backwards)
+           (setq start (point))
+
+           (while (< (point) end)
+             (nxml-tokenize-forward))
+
+           (setq end (point))))
+
+    (when (or (< start font-lock-beg)
+              (> end font-lock-end))
+      (setq font-lock-beg start
+            font-lock-end end)
+      (nxml-debug-change "nxml-extend-region" start end)
+      t)))
+
+(defun nxml-extend-after-change-region (start end pre-change-length)
+  (unless nxml-degraded
+    (setq nxml-last-fontify-end nil)
+
+    (nxml-with-degradation-on-error 'nxml-extend-after-change-region
+	(save-excursion
+	  (save-restriction
+	    (widen)
+	    (save-match-data
+	      (nxml-with-invisible-motion
+		(nxml-with-unmodifying-text-property-changes
+                  (nxml-extend-after-change-region1
+                   start end pre-change-length)))))))))
+
+(defun nxml-extend-after-change-region1 (start end pre-change-length)
+  (let* ((region (nxml-after-change1 start end pre-change-length))
+         (font-lock-beg (car region))
+         (font-lock-end (cdr region)))
+
+    (nxml-extend-region)
+    (cons font-lock-beg font-lock-end)))
+
+(defun nxml-fontify-matcher (bound)
+  "Called as font-lock keyword matcher."
+
+  (unless nxml-degraded
+    (nxml-debug-change "nxml-fontify-matcher" (point) bound)
+
+    (when (< (point) nxml-prolog-end)
+      ;; prolog needs to be fontified in one go, and
+      ;; nxml-extend-region makes sure we start at BOB.
+      (assert (bobp))
+      (nxml-fontify-prolog)
+      (goto-char nxml-prolog-end))
+
+    (let (xmltok-dependent-regions
+          xmltok-errors)
+      (while (and (nxml-tokenize-forward)
+                  (<= (point) bound)) ; intervals are open-ended
+        (nxml-apply-fontify-rule)))
+
+    (setq nxml-last-fontify-end (point)))
+
+  ;; Since we did the fontification internally, tell font-lock to not
+  ;; do anything itself.
+  nil)
+
 (defun nxml-fontify-prolog ()
   "Fontify the prolog.
 The buffer is assumed to be prepared for fontification.
 This does not set the fontified property, but it does clear
 faces appropriately."
   (let ((regions nxml-prolog-regions))
-    (nxml-clear-face (point-min) nxml-prolog-end)
     (while regions
       (let ((region (car regions)))
 	(nxml-apply-fontify-rule (aref region 0)
@@ -922,17 +947,6 @@
 				 (aref region 2)))
       (setq regions (cdr regions)))))
 
-(defun nxml-do-fontify (&optional bound)
-  "Fontify at least as far as bound.
-Leave point after last fontified position."
-  (unless bound (setq bound (point-max)))
-  (let (xmltok-dependent-regions
-	xmltok-errors)
-    (while (and (< (point) bound)
-		(nxml-tokenize-forward))
-      (nxml-clear-face xmltok-start (point))
-      (nxml-apply-fontify-rule))))
-
 ;; Vectors identify a substring of the token to be highlighted in some face.
 
 ;; Token types returned by xmltok-forward.
@@ -2582,13 +2596,7 @@
 	       (> (prefix-numeric-value arg) 0))))
     (when (not (eq new nxml-char-ref-extra-display))
       (setq nxml-char-ref-extra-display new)
-      (save-excursion
-	(save-restriction
-	  (widen)
-	  (if nxml-char-ref-extra-display
-	      (nxml-with-unmodifying-text-property-changes
-		(nxml-clear-fontified (point-min) (point-max)))
-	    (nxml-clear-char-ref-extra-display (point-min) (point-max))))))))
+      (font-lock-fontify-buffer))))
 
 (put 'nxml-char-ref 'evaporate t)
 
Index: nxml-util.el
===================================================================
--- nxml-util.el	(revision 44)
+++ nxml-util.el	(working copy)
@@ -24,6 +24,35 @@
 
 ;;; Code:
 
+(defconst nxml-debug nil
+  "enable nxml debugging. effective only at compile time")
+
+(eval-when-compile
+  (require 'cl))
+
+(defsubst nxml-debug (format &rest args)
+  (when nxml-debug
+    (apply #'message format args)))
+
+(defmacro nxml-debug-change (name start end)
+  (when nxml-debug
+    `(nxml-debug "%s: %S" ,name
+                (buffer-substring-no-properties ,start ,end))))
+
+(defmacro nxml-debug-set-inside (start end)
+  (when nxml-debug
+    `(let ((overlay (make-overlay ,start ,end)))
+       (overlay-put overlay 'face '(:background "red"))
+       (overlay-put overlay 'nxml-inside-debug t)
+       (nxml-debug-change "nxml-set-inside" ,start ,end))))
+
+(defmacro nxml-debug-clear-inside (start end)
+  (when nxml-debug
+    `(loop for overlay in (overlays-in ,start ,end)
+           if (overlay-get overlay 'nxml-inside-debug)
+           do (delete-overlay overlay)
+           finally (nxml-debug-change "nxml-clear-inside" ,start ,end))))
+
 (defun nxml-make-namespace (str)
   "Return a symbol for the namespace URI STR.
 STR must be a string. If STR is the empty string, return nil.
@@ -37,12 +66,21 @@
 This is the inverse of `nxml-make-namespace'."
   (and ns (substring (symbol-name ns) 1)))
 
-(defconst nxml-xml-namespace-uri 
+(defconst nxml-xml-namespace-uri
   (nxml-make-namespace "http://www.w3.org/XML/1998/namespace"))
 
 (defconst nxml-xmlns-namespace-uri
   (nxml-make-namespace "http://www.w3.org/2000/xmlns/"))
 
+(defmacro nxml-with-degradation-on-error (context &rest body)
+  (if (not nxml-debug)
+      (let ((error-symbol (gensym "err")))
+        `(condition-case ,error-symbol
+             (progn ,@body)
+           (error
+            (nxml-degrade ,context ,error-symbol))))
+    `(progn ,@body)))
+
 (defmacro nxml-with-unmodifying-text-property-changes (&rest body)
   "Evaluate BODY without any text property changes modifying the buffer.
 Any text properties changes happen as usual but the changes are not treated as
Index: nxml-rap.el
===================================================================
--- nxml-rap.el	(revision 44)
+++ nxml-rap.el	(working copy)
@@ -110,9 +110,11 @@
   (get-text-property pos 'nxml-inside))
 
 (defsubst nxml-clear-inside (start end)
+  (nxml-debug-clear-inside start end)
   (remove-text-properties start end '(nxml-inside nil)))
 
 (defsubst nxml-set-inside (start end type)
+  (nxml-debug-set-inside start end)
   (put-text-property start end 'nxml-inside type))
 
 (defun nxml-inside-end (pos)
@@ -137,12 +139,10 @@
   "Restore `nxml-scan-end' invariants after a change.
 The change happened between START and END.
 Return position after which lexical state is unchanged.
-END must be > nxml-prolog-end."
+END must be > nxml-prolog-end. START must be outside
+any 'inside' regions and at the beginning of a token."
   (if (>= start nxml-scan-end)
       nxml-scan-end
-    (goto-char start)
-    (nxml-move-outside-backwards)
-    (setq start (point))
     (let ((inside-remove-start start)
 	  xmltok-errors
 	  xmltok-dependent-regions)
@@ -211,7 +211,7 @@
 	      (setq adjusted-start ostart)))))
       (setq overlays (cdr overlays)))
     adjusted-start))
-		  
+
 (defun nxml-mark-parse-dependent-regions ()
   (while xmltok-dependent-regions
     (apply 'nxml-mark-parse-dependent-region
@@ -297,6 +297,20 @@
       (set-marker nxml-scan-end (point)))
     xmltok-type))
 
+(defun nxml-move-tag-backwards (bound)
+  "Move point backwards outside any 'inside' regions or tags, up
+to nxml-prolog-end. Point will either be at bound or a '<'
+character starting a tag outside any 'inside' regions. Ignores
+dependent regions. As a precondition, point must be >= bound."
+  (nxml-move-outside-backwards)
+  (when (not (equal (char-after) ?<))
+    (if (search-backward "<" bound t)
+        (progn
+          (nxml-move-outside-backwards)
+          (when (not (equal (char-after) ?<))
+            (search-backward "<" bound t)))
+      (goto-char bound))))
+
 (defun nxml-move-outside-backwards ()
   "Move point to first character of the containing special thing.
 Leave point unmoved if it is not inside anything special."
Index: rng-auto.el
===================================================================
--- rng-auto.el	(revision 44)
+++ rng-auto.el	(working copy)
@@ -106,12 +106,9 @@
 (autoload (quote nxml-mode) "nxml-mode" "\
 Major mode for editing XML.
 
-Syntax highlighting is performed unless the variable
-`nxml-syntax-highlight-flag' is nil.
-
 \\[nxml-finish-element] finishes the current element by inserting an end-tag.
 C-c C-i closes a start-tag with `>' and then inserts a balancing end-tag
-leaving point between the start-tag and end-tag. 
+leaving point between the start-tag and end-tag.
 \\[nxml-balanced-close-start-tag-block] is similar but for block rather than inline elements:
 the start-tag, point, and end-tag are all left on separate lines.
 If `nxml-slash-auto-complete-flag' is non-nil, then inserting a `</'

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-05-27 15:45           ` Daniel Colascione
@ 2008-05-27 18:37             ` Stefan Monnier
       [not found]             ` <jwv8wxj8pf9.fsf-monnier+emacs@gnu.org>
  1 sibling, 0 replies; 24+ messages in thread
From: Stefan Monnier @ 2008-05-27 18:37 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

> Done.  I thought the nxml-fontify-prolog next to (assert (bobp)) would
> tell the  reader "Oh, nxml-fontify-prolog needs to start at the
> beginning of the  buffer"

The call to nxml-fontify-prolog is the reason why (bobp) needs to be
true, but it doesn't tell us why we think it is true (after all, we
only checked (< (point) nxml-prolog-end), so it's not obvious that
we're at bobp).


        Stefan




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
       [not found]             ` <jwv8wxj8pf9.fsf-monnier+emacs@gnu.org>
@ 2008-06-05 23:07               ` Daniel Colascione
  2008-06-05 23:30                 ` Lennart Borgman (gmail)
  2008-06-06  7:01                 ` Stefan Monnier
  0 siblings, 2 replies; 24+ messages in thread
From: Daniel Colascione @ 2008-06-05 23:07 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On Thursday 05 June 2008, you wrote:
> >> > Addressed. See new version of the patch.
> >>
> >> OK, let's wait for the paperwork, then.
> >
> > Great. :-)
>
> The paperwork's in.
>
> Can you send a (hopefully) final version of your patch to emacs-devel so
> someone can install it?
>
>
>         Stefan

The last version I posted to the list is final. There are no issues I know 
about, and I've been using it myself.




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-06-05 23:07               ` Daniel Colascione
@ 2008-06-05 23:30                 ` Lennart Borgman (gmail)
  2008-06-06  7:01                 ` Stefan Monnier
  1 sibling, 0 replies; 24+ messages in thread
From: Lennart Borgman (gmail) @ 2008-06-05 23:30 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: Stefan Monnier, emacs-devel

Daniel Colascione wrote:
> On Thursday 05 June 2008, you wrote:
>>>>> Addressed. See new version of the patch.
>>>> OK, let's wait for the paperwork, then.
>>> Great. :-)
>> The paperwork's in.
>>
>> Can you send a (hopefully) final version of your patch to emacs-devel so
>> someone can install it?
>>
>>
>>         Stefan
> 
> The last version I posted to the list is final. There are no issues I know 
> about, and I've been using it myself.

Did you test this with mumamo?




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-06-05 23:07               ` Daniel Colascione
  2008-06-05 23:30                 ` Lennart Borgman (gmail)
@ 2008-06-06  7:01                 ` Stefan Monnier
  2008-06-06  7:24                   ` Lennart Borgman (gmail)
  2008-06-06 16:25                   ` Michael Olson
  1 sibling, 2 replies; 24+ messages in thread
From: Stefan Monnier @ 2008-06-06  7:01 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

>> >> > Addressed. See new version of the patch.
>> >>
>> >> OK, let's wait for the paperwork, then.
>> >
>> > Great. :-)
>> 
>> The paperwork's in.
>> 
>> Can you send a (hopefully) final version of your patch to emacs-devel so
>> someone can install it?

> The last version I posted to the list is final. There are no issues I know 
> about, and I've been using it myself.

Can someone install it please?


        Stefan




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-06-06  7:01                 ` Stefan Monnier
@ 2008-06-06  7:24                   ` Lennart Borgman (gmail)
  2008-06-06  7:59                     ` Stefan Monnier
  2008-06-06 16:25                   ` Michael Olson
  1 sibling, 1 reply; 24+ messages in thread
From: Lennart Borgman (gmail) @ 2008-06-06  7:24 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Steve Yegge, Daniel Colascione, Richard M. Stallman, emacs-devel

Stefan Monnier wrote:
>>>>>> Addressed. See new version of the patch.
>>>>> OK, let's wait for the paperwork, then.
>>>> Great. :-)
>>> The paperwork's in.
>>>
>>> Can you send a (hopefully) final version of your patch to emacs-devel so
>>> someone can install it?
> 
>> The last version I posted to the list is final. There are no issues I know 
>> about, and I've been using it myself.
> 
> Can someone install it please?


I do not think this can work with mumamo. Can you please wait with 
installing this?

I have asked two times now if it works with mumamo, but unfortunately I 
have got no response so far. Please test if it does and also explain why 
it will work with mumamo.

I have written an explanation of how fontifying parser could cooperate 
with font-lock, but it is not finished and I do not have time to send it 
right now. However my suggestions there seems to be in line with Steve 
Yegge's conclusion when he looked into this problem for js2.




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-06-06  7:24                   ` Lennart Borgman (gmail)
@ 2008-06-06  7:59                     ` Stefan Monnier
  2008-06-06  8:09                       ` Lennart Borgman (gmail)
  2008-06-06 19:04                       ` Richard M Stallman
  0 siblings, 2 replies; 24+ messages in thread
From: Stefan Monnier @ 2008-06-06  7:59 UTC (permalink / raw)
  To: Lennart Borgman (gmail)
  Cc: Steve Yegge, Daniel Colascione, Richard M. Stallman, emacs-devel

>> Can someone install it please?
> I do not think this can work with mumamo.

Can you explain why this change makes life more difficult for Mumamo?

I thought that the current nxml highlighting doesn't work with Mumamo
either anyway (and Mumamo uses xml-mode's highlighting instead for that
reason), so I got the impression that it doesn't make things worse.

> Can you please wait with installing this?

I think it's important for nxml to play nicely with other packages using
jit/font-lock, so I just want to make sure that the wait is not
too long.


        Stefan




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-06-06  7:59                     ` Stefan Monnier
@ 2008-06-06  8:09                       ` Lennart Borgman (gmail)
  2008-06-06 10:09                         ` Jason Rumney
  2008-06-06 14:23                         ` Chong Yidong
  2008-06-06 19:04                       ` Richard M Stallman
  1 sibling, 2 replies; 24+ messages in thread
From: Lennart Borgman (gmail) @ 2008-06-06  8:09 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Steve Yegge, Daniel Colascione, Richard M. Stallman, emacs-devel

Stefan Monnier wrote:
>>> Can someone install it please?
>> I do not think this can work with mumamo.
> 
> Can you explain why this change makes life more difficult for Mumamo?
> 
> I thought that the current nxml highlighting doesn't work with Mumamo
> either anyway (and Mumamo uses xml-mode's highlighting instead for that
> reason), so I got the impression that it doesn't make things worse.

I am in a hurry right now, so I can't write much at the moment.

It is true that nxml highlighting does not work, but I think I know how 
to make it work now. The parser from nxml is however already used with 
Mumamo.

>> Can you please wait with installing this?
> 
> I think it's important for nxml to play nicely with other packages using
> jit/font-lock, so I just want to make sure that the wait is not
> too long.

I will have time to respond more properly in a couple of days.




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-06-06  8:09                       ` Lennart Borgman (gmail)
@ 2008-06-06 10:09                         ` Jason Rumney
  2008-06-06 14:23                         ` Chong Yidong
  1 sibling, 0 replies; 24+ messages in thread
From: Jason Rumney @ 2008-06-06 10:09 UTC (permalink / raw)
  To: Lennart Borgman (gmail)
  Cc: Daniel Colascione, emacs-devel, Steve Yegge, Stefan Monnier,
	Richard M. Stallman

Lennart Borgman (gmail) wrote:
> It is true that nxml highlighting does not work, but I think I know 
> how to make it work now.

So you are asking to delay installing an improvement to nxml-mode's 
highlighting because you _think_ you can make your own lisp mode work 
with the old highlighting code, and noone, including yourself, has 
tested your lisp mode with the improved version? This seems like a 
strange request to make.





^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-06-06  8:09                       ` Lennart Borgman (gmail)
  2008-06-06 10:09                         ` Jason Rumney
@ 2008-06-06 14:23                         ` Chong Yidong
  1 sibling, 0 replies; 24+ messages in thread
From: Chong Yidong @ 2008-06-06 14:23 UTC (permalink / raw)
  To: Lennart Borgman (gmail)
  Cc: Daniel Colascione, emacs-devel, Steve Yegge, Stefan Monnier,
	Richard M. Stallman

"Lennart Borgman (gmail)" <lennart.borgman@gmail.com> writes:

> Stefan Monnier wrote:
>>>> Can someone install it please?
>>> I do not think this can work with mumamo.
>>
>> Can you explain why this change makes life more difficult for Mumamo?
>>
>> I thought that the current nxml highlighting doesn't work with Mumamo
>> either anyway (and Mumamo uses xml-mode's highlighting instead for that
>> reason), so I got the impression that it doesn't make things worse.
>
> I am in a hurry right now, so I can't write much at the moment.
>
> It is true that nxml highlighting does not work, but I think I know
> how to make it work now. The parser from nxml is however already used
> with Mumamo.

I don't think this is a good reason to hold the patch, unless you have
reason to believe that it is impossible to make mumamo work properly
with nxml when nxml uses font-lock.  I don't see why that would be the
case.




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-06-06  7:01                 ` Stefan Monnier
  2008-06-06  7:24                   ` Lennart Borgman (gmail)
@ 2008-06-06 16:25                   ` Michael Olson
  1 sibling, 0 replies; 24+ messages in thread
From: Michael Olson @ 2008-06-06 16:25 UTC (permalink / raw)
  To: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Can someone install it please?

Done, along with ChangeLog entry.

-- 
|       Michael Olson  |  FSF Associate Member #652     |
| http://mwolson.org/  |  Hobbies: Lisp, HCoop          |
| Projects: Emacs, Muse, ERC, EMMS, ErBot, DVC, Planner |
`-------------------------------------------------------'





^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [patch] use font-lock
  2008-06-06  7:59                     ` Stefan Monnier
  2008-06-06  8:09                       ` Lennart Borgman (gmail)
@ 2008-06-06 19:04                       ` Richard M Stallman
  1 sibling, 0 replies; 24+ messages in thread
From: Richard M Stallman @ 2008-06-06 19:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: danc, lennart.borgman, steve.yegge, emacs-devel

    > Can you please wait with installing this?

    I think it's important for nxml to play nicely with other packages using
    jit/font-lock, so I just want to make sure that the wait is not
    too long.

It can't hurt to wait a few days to give Lennart time to think and say
more.




^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2008-06-06 19:04 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <200805231711.30830.danc@merrillpress.com>
2008-05-23 21:52 ` [patch] use font-lock Lennart Borgman (gmail)
2008-05-23 22:24   ` [emacs-nxml-mode] " Daniel Colascione
     [not found]   ` <200805231824.18563.danc@merrillpress.com>
2008-05-23 22:50     ` Lennart Borgman (gmail)
2008-05-24 15:03       ` Daniel Colascione
2008-05-24 16:57         ` Lennart Borgman (gmail)
2008-05-24  2:39   ` [emacs-nxml-mode] " Stefan Monnier
2008-05-23 22:26 Daniel Colascione
2008-05-24 20:38 ` Stefan Monnier
2008-05-25 20:36   ` Daniel Colascione
2008-05-26 14:52     ` Stefan Monnier
2008-05-27 15:13       ` Daniel Colascione
2008-05-27 15:37         ` Stefan Monnier
2008-05-27 15:45           ` Daniel Colascione
2008-05-27 18:37             ` Stefan Monnier
     [not found]             ` <jwv8wxj8pf9.fsf-monnier+emacs@gnu.org>
2008-06-05 23:07               ` Daniel Colascione
2008-06-05 23:30                 ` Lennart Borgman (gmail)
2008-06-06  7:01                 ` Stefan Monnier
2008-06-06  7:24                   ` Lennart Borgman (gmail)
2008-06-06  7:59                     ` Stefan Monnier
2008-06-06  8:09                       ` Lennart Borgman (gmail)
2008-06-06 10:09                         ` Jason Rumney
2008-06-06 14:23                         ` Chong Yidong
2008-06-06 19:04                       ` Richard M Stallman
2008-06-06 16:25                   ` Michael Olson

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).