unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#11916: 24.1.50; Making url-dav work
@ 2012-07-11 21:00 David Engster
  2012-07-18 12:25 ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: David Engster @ 2012-07-11 21:00 UTC (permalink / raw)
  To: 11916

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

I'm trying to simply get a listing from a directory on a WebDAV share
using Emacs. I was overjoyed to see that Emacs ships with url-dav.el,
which seemed to do exactly what I needed. However, it seems that url-dav
depends on a specially patched xml.el for namespace expansion which
apparently never made it into Emacs proper. If you look into
`url-dav-supported-p', it looks for a function `xml-expand-namespace'
which I cannot find anywhere in xml.el's history. At least one other
lone soul faced the same problem and came to the same conclusion (see
http://lists.gnu.org/archive/html/help-gnu-emacs/2011-11/msg00158.html).

Now, xml.el can properly deal with namespaces since 2004 or so, but it
returns the QNames through a cons ("DAV:" . "property"), which is kind
of... cumbersome. The url-dav package naturally expects a plain symbol
'DAV:property.

I started rewriting url-dav to work with the cons's returned by xml.el,
but it's tedious work and, more importantly, makes the code really
ugly. Instead, I now just use a small function to replace the cons's in
xml.el's output with the plain symbols the package expects. Please find
the patch attached.

-David


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: url-dav-diff.patch --]
[-- Type: text/x-patch, Size: 1592 bytes --]

--- url-dav.el	2012-07-11 22:34:49.401721367 +0200
+++ url-dav.el_patched	2012-07-11 22:33:34.504867729 +0200
@@ -395,10 +395,15 @@
 		 url-http-content-type
 		 (string-match "\\`\\(text\\|application\\)/xml"
 			       url-http-content-type))
-		(setq tree (xml-parse-region (point) (point-max)))))
+		(setq tree (xml-parse-region (point) (point-max) nil nil t))))
 	;; Clean up after ourselves.
 	(kill-buffer buffer)))
 
+    ;; This package was initially written for a different kind of
+    ;; QNAMES expansion, hence we have now to rewrite those so that
+    ;; for example ("DAV:" . "foo") becomes the symbol 'DAV:foo.
+    (url-dav-changexml (car tree))
+
     ;; We should now be
     (if (eq (xml-node-name (car tree)) 'DAV:multistatus)
 	(url-dav-dispatch-node (car tree))
@@ -411,6 +416,21 @@
 	;; nobody but us needs to know the difference.
 	(list (cons url properties))))))
 
+(defun url-dav-changexml (node)
+  "Rewrite all expanded names in NODE to plain symbols.
+That means, all cons produced from `xml-parse-region'
+like (\"DAV:\" .  \"foo\") become plain symbols DAV:foo.
+This replacement happens in-place."
+  (when (consp node)
+    (let ((name (xml-node-name node))
+	  (children (xml-node-children node)))
+      (when (and (consp name)
+		 (equal (car name) "DAV:"))
+	(setcar node (intern (concat "DAV:" (cdr name)))))
+      (when children
+	(dolist (cur children)
+	  (url-dav-changexml cur))))))
+
 (defun url-dav-request (url method tag body
 				 &optional depth headers namespaces)
   "Perform WebDAV operation METHOD on URL.  Return the parsed responses.

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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-11 21:00 bug#11916: 24.1.50; Making url-dav work David Engster
@ 2012-07-18 12:25 ` Stefan Monnier
  2012-07-18 17:45   ` David Engster
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-07-18 12:25 UTC (permalink / raw)
  To: David Engster; +Cc: 11916

> I started rewriting url-dav to work with the cons's returned by xml.el,
> but it's tedious work and, more importantly, makes the code really
> ugly.

Could you show us some example of the ugliness to get a feeling for the
tradeoffs?  I see you'd need to change url-dav-find-parser, but that
doesn't sound so terrible, so the ugliness seems to come from parts that
aren't as immediately apparent.

> Instead, I now just use a small function to replace the cons's in
> xml.el's output with the plain symbols the package expects. Please find
> the patch attached.

Rather than provide a "changexml" function which traverses the XML tree
and changes the cons to symbols, how 'bout changing xml-parse-region?

I have the vague impression that the representation was changed from
symbols to cons cells on the premise that it was more Lispy (would
avoid (re)parsing the symbol names), but if it turns out to be a pain
to use, maybe that was a bad call (basically, you're saying that you
never need/want to parse those QNAMES).

Another issue is: what does libxml-parse-xml-region do (it would be better
to try and reduce the difference between the two)?


        Stefan


> --- url-dav.el	2012-07-11 22:34:49.401721367 +0200
> +++ url-dav.el_patched	2012-07-11 22:33:34.504867729 +0200
> @@ -395,10 +395,15 @@
>  		 url-http-content-type
>  		 (string-match "\\`\\(text\\|application\\)/xml"
>  			       url-http-content-type))
> -		(setq tree (xml-parse-region (point) (point-max)))))
> +		(setq tree (xml-parse-region (point) (point-max) nil nil t))))
>  	;; Clean up after ourselves.
>  	(kill-buffer buffer)))
 
> +    ;; This package was initially written for a different kind of
> +    ;; QNAMES expansion, hence we have now to rewrite those so that
> +    ;; for example ("DAV:" . "foo") becomes the symbol 'DAV:foo.
> +    (url-dav-changexml (car tree))
> +
>      ;; We should now be
>      (if (eq (xml-node-name (car tree)) 'DAV:multistatus)
>  	(url-dav-dispatch-node (car tree))
> @@ -411,6 +416,21 @@
>  	;; nobody but us needs to know the difference.
>  	(list (cons url properties))))))
 
> +(defun url-dav-changexml (node)
> +  "Rewrite all expanded names in NODE to plain symbols.
> +That means, all cons produced from `xml-parse-region'
> +like (\"DAV:\" .  \"foo\") become plain symbols DAV:foo.
> +This replacement happens in-place."
> +  (when (consp node)
> +    (let ((name (xml-node-name node))
> +	  (children (xml-node-children node)))
> +      (when (and (consp name)
> +		 (equal (car name) "DAV:"))
> +	(setcar node (intern (concat "DAV:" (cdr name)))))
> +      (when children
> +	(dolist (cur children)
> +	  (url-dav-changexml cur))))))
> +
>  (defun url-dav-request (url method tag body
>  				 &optional depth headers namespaces)
>    "Perform WebDAV operation METHOD on URL.  Return the parsed responses.





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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-18 12:25 ` Stefan Monnier
@ 2012-07-18 17:45   ` David Engster
  2012-07-19  7:15     ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: David Engster @ 2012-07-18 17:45 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11916

Stefan Monnier writes:
>> I started rewriting url-dav to work with the cons's returned by xml.el,
>> but it's tedious work and, more importantly, makes the code really
>> ugly.
>
> Could you show us some example of the ugliness to get a feeling for the
> tradeoffs?  I see you'd need to change url-dav-find-parser, but that
> doesn't sound so terrible, so the ugliness seems to come from parts that
> aren't as immediately apparent.

Just do an M-x occur on "'DAV:"; pretty much every test you see against
some symbol 'DAV:foo (about 30) has to be changed to test against
'("DAV" . "foo") instead. This also implies that every 'eq' has to be
changed to 'equal', 'assq' to 'assoc', 'plist-get' to 'lax-plist-get',
and so on. I started doing that and it just felt wrong; the code gets
larger, less readable and possibly slower without any real
benefit. However, I don't feel terribly strong about this, so if this
in-place XML change is considered too hack-ish, I can still finish that
rewrite.

>> Instead, I now just use a small function to replace the cons's in
>> xml.el's output with the plain symbols the package expects. Please find
>> the patch attached.
>
> Rather than provide a "changexml" function which traverses the XML tree
> and changes the cons to symbols, how 'bout changing xml-parse-region?
>
> I have the vague impression that the representation was changed from
> symbols to cons cells on the premise that it was more Lispy (would
> avoid (re)parsing the symbol names), but if it turns out to be a pain
> to use, maybe that was a bad call (basically, you're saying that you
> never need/want to parse those QNAMES).

I think namespace-aware parsing was added in 2004 or so, so changing it
now would surely break some code out there. Also, I could imagine that
under other circumstances you can profit from such a representation; but
url-dav simply wants to make sure that everything is in the 'DAV'
namespace.

> Another issue is: what does libxml-parse-xml-region do (it would be better
> to try and reduce the difference between the two)?

libxml simply does not do namespace-aware parsing; you just get the
plain symbols without any prefix.

-David





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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-18 17:45   ` David Engster
@ 2012-07-19  7:15     ` Stefan Monnier
  2012-07-19 15:28       ` David Engster
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-07-19  7:15 UTC (permalink / raw)
  To: David Engster; +Cc: 11916

> Just do an M-x occur on "'DAV:"; pretty much every test you see against
> some symbol 'DAV:foo (about 30) has to be changed to test against
> '("DAV" . "foo") instead. This also implies that every 'eq' has to be
> changed to 'equal', 'assq' to 'assoc', 'plist-get' to 'lax-plist-get',
> and so on. I started doing that and it just felt wrong; the code gets
> larger, less readable and possibly slower without any real
> benefit. However, I don't feel terribly strong about this, so if this
> in-place XML change is considered too hack-ish, I can still finish that
> rewrite.

What would happen if url-dav.el didn't use namespaces (and just removed
"DAV:" from all those symbols)?

> I think namespace-aware parsing was added in 2004 or so, so changing it
> now would surely break some code out there. Also, I could imagine that
> under other circumstances you can profit from such a representation; but
> url-dav simply wants to make sure that everything is in the 'DAV'
> namespace.

Of course, I was thinking of changing it in a backward compatible way,
by letting the `parse-ns' argument specify which kind of result
you want.
The changes should be mostly limited to xml-maybe-do-ns.


        Stefan





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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-19  7:15     ` Stefan Monnier
@ 2012-07-19 15:28       ` David Engster
  2012-07-19 22:12         ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: David Engster @ 2012-07-19 15:28 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11916

Stefan Monnier writes:
>> Just do an M-x occur on "'DAV:"; pretty much every test you see against
>> some symbol 'DAV:foo (about 30) has to be changed to test against
>> '("DAV" . "foo") instead. This also implies that every 'eq' has to be
>> changed to 'equal', 'assq' to 'assoc', 'plist-get' to 'lax-plist-get',
>> and so on. I started doing that and it just felt wrong; the code gets
>> larger, less readable and possibly slower without any real
>> benefit. However, I don't feel terribly strong about this, so if this
>> in-place XML change is considered too hack-ish, I can still finish that
>> rewrite.
>
> What would happen if url-dav.el didn't use namespaces (and just removed
> "DAV:" from all those symbols)?

You might get name clashes; for example, the code might parse a
'collection' although it is actually not a "DAV:collection" but a
"FOOBAR:collection". Granted, it's not very likely, and if this would be
used in a read-only fashion (like parsing atom feeds) I'd drop the
namespaces in a heartbeat. But since url-dav will usually be used to
manipulate actual files on remote servers, I'd rather not risk it.

>> I think namespace-aware parsing was added in 2004 or so, so changing it
>> now would surely break some code out there. Also, I could imagine that
>> under other circumstances you can profit from such a representation; but
>> url-dav simply wants to make sure that everything is in the 'DAV'
>> namespace.
>
> Of course, I was thinking of changing it in a backward compatible way,
> by letting the `parse-ns' argument specify which kind of result you
> want.  The changes should be mostly limited to xml-maybe-do-ns.

I could live with that.

-David





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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-19 15:28       ` David Engster
@ 2012-07-19 22:12         ` Stefan Monnier
  2012-07-21 12:11           ` David Engster
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-07-19 22:12 UTC (permalink / raw)
  To: David Engster; +Cc: 11916

> You might get name clashes; for example, the code might parse a
> 'collection' although it is actually not a "DAV:collection" but a
> "FOOBAR:collection". Granted, it's not very likely, and if this would be
> used in a read-only fashion (like parsing atom feeds) I'd drop the
> namespaces in a heartbeat. But since url-dav will usually be used to
> manipulate actual files on remote servers, I'd rather not risk it.

I see.  So using libxml wouldn't be an option (or maybe libxml can also
do it, but we'd need to change libxml-parse-xml-region for that?).

>> Of course, I was thinking of changing it in a backward compatible way,
>> by letting the `parse-ns' argument specify which kind of result you
>> want.  The changes should be mostly limited to xml-maybe-do-ns.
> I could live with that.

Could you prepare a patch for that?


        Stefan





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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-19 22:12         ` Stefan Monnier
@ 2012-07-21 12:11           ` David Engster
  2012-07-22 10:11             ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: David Engster @ 2012-07-21 12:11 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11916

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

Stefan Monnier writes:
>> You might get name clashes; for example, the code might parse a
>> 'collection' although it is actually not a "DAV:collection" but a
>> "FOOBAR:collection". Granted, it's not very likely, and if this would be
>> used in a read-only fashion (like parsing atom feeds) I'd drop the
>> namespaces in a heartbeat. But since url-dav will usually be used to
>> manipulate actual files on remote servers, I'd rather not risk it.
>
> I see.  So using libxml wouldn't be an option (or maybe libxml can also
> do it, but we'd need to change libxml-parse-xml-region for that?).

Yes, libxml can do namespace parsing.

>>> Of course, I was thinking of changing it in a backward compatible way,
>>> by letting the `parse-ns' argument specify which kind of result you
>>> want.  The changes should be mostly limited to xml-maybe-do-ns.
>> I could live with that.
>
> Could you prepare a patch for that?

Attached. I had to go another route, though; turns out the `parse-ns'
argument is already overloaded in `xml-parse-tag' (it can be used to
provide a namespace->URI mapping), but that wasn't mentioned in the
other parse functions. So I had to introduce an additional argument.

I also attached my current changes in url-dav.el, which next to
supporting the new `simple-qnames' argument contain a few other
fixes. Here's the complete ChangeLog:

xml.el:

(xml-node-name): Mention `simple-qnames' in doc-string.
(xml-parse-file, xml-parse-region, xml--parse-buffer)
(xml-parse-tag, xml-parse-tag-1, xml-parse-attlist): Add argument
`simple-qnames'.  Adapt all calls to parse functions to hand over this
new argument.  Adapt doc-strings to mention `simple-qnames' and also
mention that `parse-ns' can be used to provide namespace mappings.
(xml-maybe-do-ns): Return symbol instead of cons depending on
`simple-qnames' argument.


url-dav.el:

(url-dav-supported-p): Added doc-string and remove check for feature
`xml' and function `xml-expand-namespace' which never existed in Emacs
proper.
(url-dav-process-response): Remove all indentation from XML
before parsing.  Change call to `xml-parse-region' to do namespace
expansion with simple qualified names.
(url-dav-request): Add autoload.
(url-dav-directory-files): Properly deal with empty directories.  Call
hexify before generating relative URLs.
(url-dav-file-directory-p): Fix bug when checking for 'DAV:collection
(resources are returned as a list).

-David


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xml-diff.patch --]
[-- Type: text/x-patch, Size: 8633 bytes --]

=== modified file 'lisp/xml.el'
--- lisp/xml.el	2012-07-04 16:14:05 +0000
+++ lisp/xml.el	2012-07-21 10:47:53 +0000
@@ -118,16 +118,18 @@
   "Return the tag associated with NODE.
 Without namespace-aware parsing, the tag is a symbol.
 
-With namespace-aware parsing, the tag is a cons of a string
-representing the uri of the namespace with the local name of the
-tag.  For example,
+With namespace-aware parsing, by default the tag is a cons of a
+string representing the uri of the namespace with the local name
+of the tag.  For example,
 
     <foo>
 
 would be represented by
 
-    '(\"\" . \"foo\")."
+    '(\"\" . \"foo\").
 
+If you would rather like a plain symbol instead, provide a
+non-nil SIMPLE-QNAMES argument to the parser functions."
   (car node))
 
 (defsubst xml-node-attributes (node)
@@ -309,17 +311,24 @@
 ;;; Entry points:
 
 ;;;###autoload
-(defun xml-parse-file (file &optional parse-dtd parse-ns)
+(defun xml-parse-file (file &optional parse-dtd parse-ns simple-qnames)
   "Parse the well-formed XML file FILE.
 Return the top node with all its children.
 If PARSE-DTD is non-nil, the DTD is parsed rather than skipped.
-If PARSE-NS is non-nil, then QNAMES are expanded."
+If PARSE-NS is non-nil, expand QNAMES; if the value of PARSE-NS
+is a list, use it as an alist mapping namespaces to URIs.
+Expanded names will by default be returned as a cons
+
+  (\"foo:\" . \"bar\").
+
+If you would like to get a plain symbol 'foo:bar instead, set
+SIMPLE-QNAMES to a non-nil value."
   (with-temp-buffer
     (insert-file-contents file)
-    (xml--parse-buffer parse-dtd parse-ns)))
+    (xml--parse-buffer parse-dtd parse-ns simple-qnames)))
 
 ;;;###autoload
-(defun xml-parse-region (&optional beg end buffer parse-dtd parse-ns)
+(defun xml-parse-region (&optional beg end buffer parse-dtd parse-ns simple-qnames)
   "Parse the region from BEG to END in BUFFER.
 Return the XML parse tree, or raise an error if the region does
 not contain well-formed XML.
@@ -329,14 +338,21 @@
 If BUFFER is nil, it defaults to the current buffer.
 If PARSE-DTD is non-nil, parse the DTD and return it as the first
 element of the list.
-If PARSE-NS is non-nil, expand QNAMES."
+If PARSE-NS is non-nil, expand QNAMES; if the value of PARSE-NS
+is a list, use it as an alist mapping namespaces to URIs.
+Expanded names will by default be returned as a cons
+
+  (\"foo:\" . \"bar\").
+
+If you would like to get a plain symbol 'foo:bar instead, set
+SIMPLE-QNAMES to a non-nil value."
   ;; Use fixed syntax table to ensure regexp char classes and syntax
   ;; specs DTRT.
   (unless buffer
     (setq buffer (current-buffer)))
   (with-temp-buffer
     (insert-buffer-substring-no-properties buffer beg end)
-    (xml--parse-buffer parse-dtd parse-ns)))
+    (xml--parse-buffer parse-dtd parse-ns simple-qnames)))
 
 ;; XML [5]
 
@@ -344,7 +360,7 @@
 ;;   document  ::=  prolog element Misc*
 ;;   prolog    ::=  XMLDecl? Misc* (doctypedecl Misc*)?
 
-(defun xml--parse-buffer (parse-dtd parse-ns)
+(defun xml--parse-buffer (parse-dtd parse-ns simple-qnames)
   (with-syntax-table xml-syntax-table
     (let ((case-fold-search nil)	; XML is case-sensitive.
 	  ;; Prevent entity definitions from changing the defaults
@@ -356,7 +372,7 @@
 	(if (search-forward "<" nil t)
 	    (progn
 	      (forward-char -1)
-	      (setq result (xml-parse-tag-1 parse-dtd parse-ns))
+	      (setq result (xml-parse-tag-1 parse-dtd parse-ns simple-qnames))
 	      (cond
 	       ((null result)
 		;; Not looking at an xml start tag.
@@ -377,7 +393,7 @@
 	  (cons dtd (nreverse xml))
 	(nreverse xml)))))
 
-(defun xml-maybe-do-ns (name default xml-ns)
+(defun xml-maybe-do-ns (name default xml-ns simple-qnames)
   "Perform any namespace expansion.
 NAME is the name to perform the expansion on.
 DEFAULT is the default namespace.  XML-NS is a cons of namespace
@@ -386,7 +402,10 @@
 
 During namespace-aware parsing, any name without a namespace is
 put into the namespace identified by DEFAULT.  nil is used to
-specify that the name shouldn't be given a namespace."
+specify that the name shouldn't be given a namespace.
+Expanded names will by default be returned as a cons.  If you
+would like to get plain symbols, set SIMPLE-QNAMES to a non-nil
+value."
   (if (consp xml-ns)
       (let* ((nsp (string-match ":" name))
 	     (lname (if nsp (substring name (match-end 0)) name))
@@ -397,15 +416,24 @@
 	     (ns (or (cdr (assoc (if special "xmlns" prefix)
                                  xml-ns))
                      "")))
-        (cons ns (if special "" lname)))
+	(if (and simple-qnames
+		 (not (string= prefix "xmlns")))
+	    (intern (concat ns lname))
+	  (cons ns (if special "" lname))))
     (intern name)))
 
-(defun xml-parse-tag (&optional parse-dtd parse-ns)
+(defun xml-parse-tag (&optional parse-dtd parse-ns simple-qnames)
   "Parse the tag at point.
 If PARSE-DTD is non-nil, the DTD of the document, if any, is parsed and
 returned as the first element in the list.
 If PARSE-NS is non-nil, expand QNAMES; if the value of PARSE-NS
 is a list, use it as an alist mapping namespaces to URIs.
+Expanded names will by default be returned as a cons
+
+  (\"foo:\" . \"bar\").
+
+If you would like to get a plain symbol 'foo:bar instead, set
+SIMPLE-QNAMES to a non-nil value.
 
 Return one of:
  - a list : the matching node
@@ -421,9 +449,9 @@
       (with-syntax-table xml-syntax-table
 	(insert-buffer-substring-no-properties buf pos)
 	(goto-char (point-min))
-	(xml-parse-tag-1 parse-dtd parse-ns)))))
+	(xml-parse-tag-1 parse-dtd parse-ns simple-qnames)))))
 
-(defun xml-parse-tag-1 (&optional parse-dtd parse-ns)
+(defun xml-parse-tag-1 (&optional parse-dtd parse-ns simple-qnames)
   "Like `xml-parse-tag', but possibly modify the buffer while working."
   (let ((xml-validating-parser (or parse-dtd xml-validating-parser))
 	(xml-ns (cond ((consp parse-ns) parse-ns)
@@ -433,7 +461,7 @@
      ((looking-at "<\\?")
       (search-forward "?>")
       (skip-syntax-forward " ")
-      (xml-parse-tag-1 parse-dtd xml-ns))
+      (xml-parse-tag-1 parse-dtd xml-ns simple-qnames))
      ;; Character data (CDATA) sections, in which no tag should be interpreted
      ((looking-at "<!\\[CDATA\\[")
       (let ((pos (match-end 0)))
@@ -447,8 +475,8 @@
       (let ((dtd (xml-parse-dtd parse-ns)))
 	(skip-syntax-forward " ")
 	(if xml-validating-parser
-	    (cons dtd (xml-parse-tag-1 nil xml-ns))
-	  (xml-parse-tag-1 nil xml-ns))))
+	    (cons dtd (xml-parse-tag-1 nil xml-ns simple-qnames))
+	  (xml-parse-tag-1 nil xml-ns simple-qnames))))
      ;; skip comments
      ((looking-at "<!--")
       (search-forward "-->")
@@ -456,7 +484,7 @@
       (skip-syntax-forward " ")
       (unless (eobp)
 	(let ((xml-sub-parser t))
-	  (xml-parse-tag-1 parse-dtd xml-ns))))
+	  (xml-parse-tag-1 parse-dtd xml-ns simple-qnames))))
      ;; end tag
      ((looking-at "</")
       '())
@@ -466,7 +494,7 @@
       ;; Parse this node
       (let* ((node-name (match-string-no-properties 1))
 	     ;; Parse the attribute list.
-	     (attrs (xml-parse-attlist xml-ns))
+	     (attrs (xml-parse-attlist xml-ns simple-qnames))
 	     children)
 	;; add the xmlns:* attrs to our cache
 	(when (consp xml-ns)
@@ -476,7 +504,8 @@
 			      (caar attr)))
 	      (push (cons (cdar attr) (cdr attr))
 		    xml-ns))))
-	(setq children (list attrs (xml-maybe-do-ns node-name "" xml-ns)))
+	(setq children (list attrs (xml-maybe-do-ns node-name ""
+						    xml-ns simple-qnames)))
 	(cond
 	 ;; is this an empty element ?
 	 ((looking-at "/>")
@@ -502,7 +531,7 @@
 		       node-name))
 	       ;; Read a sub-element and push it onto CHILDREN.
 	       ((= (char-after) ?<)
-		(let ((tag (xml-parse-tag-1 nil xml-ns)))
+		(let ((tag (xml-parse-tag-1 nil xml-ns simple-qnames)))
 		  (when tag
 		    (push tag children))))
 	       ;; Read some character data.
@@ -585,7 +614,7 @@
       (goto-char end-marker)
       (buffer-substring start (point)))))
 
-(defun xml-parse-attlist (&optional xml-ns)
+(defun xml-parse-attlist (&optional xml-ns simple-qnames)
   "Return the attribute-list after point.
 Leave point at the first non-blank character after the tag."
   (let ((attlist ())
@@ -594,7 +623,8 @@
     (while (looking-at (eval-when-compile
 			 (concat "\\(" xml-name-re "\\)\\s-*=\\s-*")))
       (setq end-pos (match-end 0))
-      (setq name (xml-maybe-do-ns (match-string-no-properties 1) nil xml-ns))
+      (setq name (xml-maybe-do-ns (match-string-no-properties 1)
+				  nil xml-ns simple-qnames))
       (goto-char end-pos)
 
       ;; See also: http://www.w3.org/TR/2000/REC-xml-20001006#AVNormalize


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: url-dav-diff.patch --]
[-- Type: text/x-patch, Size: 3842 bytes --]

=== modified file 'lisp/url/url-dav.el'
--- lisp/url/url-dav.el	2012-07-11 23:13:41 +0000
+++ lisp/url/url-dav.el	2012-07-21 11:45:23 +0000
@@ -53,10 +53,10 @@
 
 ;;;###autoload
 (defun url-dav-supported-p (url)
-  (and (featurep 'xml)
-       (fboundp 'xml-expand-namespace)
-       (url-intersection url-dav-supported-protocols
-			 (plist-get (url-http-options url) 'dav))))
+  "Return WebDAV protocol version supported by URL.
+Returns nil if WebDAV is not supported."
+  (url-intersection url-dav-supported-protocols
+		    (plist-get (url-http-options url) 'dav)))
 
 (defun url-dav-node-text (node)
   "Return the text data from the XML node NODE."
@@ -385,7 +385,12 @@
     (when buffer
       (unwind-protect
 	  (with-current-buffer buffer
+	    ;; First remove all indentation and line endings
 	    (goto-char url-http-end-of-headers)
+	    (indent-rigidly (point) (point-max) -1000)
+	    (save-excursion
+	      (while (re-search-forward "\r?\n" nil t)
+		(replace-match "")))
 	    (setq overall-status url-http-response-status)
 
 	    ;; XML documents can be transferred as either text/xml or
@@ -395,7 +400,7 @@
 		 url-http-content-type
 		 (string-match "\\`\\(text\\|application\\)/xml"
 			       url-http-content-type))
-		(setq tree (xml-parse-region (point) (point-max)))))
+		(setq tree (xml-parse-region (point) (point-max) nil nil t t))))
 	;; Clean up after ourselves.
 	(kill-buffer buffer)))
 
@@ -411,6 +416,7 @@
 	;; nobody but us needs to know the difference.
 	(list (cons url properties))))))
 
+;;;###autoload
 (defun url-dav-request (url method tag body
 				 &optional depth headers namespaces)
   "Perform WebDAV operation METHOD on URL.  Return the parsed responses.
@@ -768,8 +774,8 @@
 (defun url-dav-directory-files (url &optional full match nosort files-only)
   "Return a list of names of files in URL.
 There are three optional arguments:
-If FULL is non-nil, return absolute file names.  Otherwise return names
- that are relative to the specified directory.
+If FULL is non-nil, return absolute URLs.  Otherwise return names
+ that are relative to the specified URL.
 If MATCH is non-nil, mention only file names that match the regexp MATCH.
 If NOSORT is non-nil, the list is not sorted--its order is unpredictable.
  NOSORT is useful if you plan to sort the result yourself."
@@ -779,8 +785,9 @@
 	(files nil)
 	(parsed-url (url-generic-parse-url url)))
 
-    (if (= (length properties) 1)
-	(signal 'file-error (list "Opening directory" "not a directory" url)))
+    (when (and (= (length properties) 1)
+	       (not (url-dav-file-directory-p url)))
+      (signal 'file-error (list "Opening directory" "not a directory" url)))
 
     (while properties
       (setq child-props (pop properties)
@@ -791,10 +798,13 @@
 	  nil
 
 	;; Fully expand the URL and then rip off the beginning if we
-	;; are not supposed to return fully-qualified names.
+	;; are not supposed to return fully-qualified names.  
 	(setq child-url (url-expand-file-name child-url parsed-url))
 	(if (not full)
-	    (setq child-url (substring child-url (length url))))
+	    ;; Parts of the URL might be hex'ed.
+	    (setq child-url (url-unhex-string
+			     (substring (url-hexify-string child-url)
+					(length (url-hexify-string url))))))
 
 	;; We don't want '/' as the last character in filenames...
 	(if (string-match "/$" child-url)
@@ -814,7 +824,8 @@
 (defun url-dav-file-directory-p (url)
   "Return t if URL names an existing DAV collection."
   (let ((properties (cdar (url-dav-get-properties url '(DAV:resourcetype)))))
-    (eq (plist-get properties 'DAV:resourcetype) 'DAV:collection)))
+    (when (member 'DAV:collection (plist-get properties 'DAV:resourcetype))
+      t)))
 
 (defun url-dav-make-directory (url &optional parents)
   "Create the directory DIR and any nonexistent parent dirs."


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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-21 12:11           ` David Engster
@ 2012-07-22 10:11             ` Stefan Monnier
  2012-07-25 21:04               ` David Engster
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-07-22 10:11 UTC (permalink / raw)
  To: David Engster; +Cc: 11916

> Attached. I had to go another route, though; turns out the `parse-ns'
> argument is already overloaded in `xml-parse-tag' (it can be used to
> provide a namespace->URI mapping), but that wasn't mentioned in the
> other parse functions. So I had to introduce an additional argument.

I'd seen that, indeed, but I think that since this parse-ns arg is
mostly passed around and only finally used in one place, I'd rather not
add an argument but instead pass both values via the single
parse-ns argument.  parse-ns could then be:
- nil as before.
- an alist of namespace->URI.
- a cons cell (symbol-qnames . ALIST) which does the same as the
  previous one but uses symbols instead of cons cells for qnames.
- the symbol `symbol-qnames' to mean (symbol-qnames . STANDARD-ALIST).
- t to mean STANDARD-ALIST.
The last two are only allowed when entering xml-parse-region but not in
recursive calls (and not in calls to xml-maybe-do-ns).


        Stefan





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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-22 10:11             ` Stefan Monnier
@ 2012-07-25 21:04               ` David Engster
  2012-07-26  0:04                 ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: David Engster @ 2012-07-25 21:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11916

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

Stefan Monnier writes:
>> Attached. I had to go another route, though; turns out the `parse-ns'
>> argument is already overloaded in `xml-parse-tag' (it can be used to
>> provide a namespace->URI mapping), but that wasn't mentioned in the
>> other parse functions. So I had to introduce an additional argument.
>
> I'd seen that, indeed, but I think that since this parse-ns arg is
> mostly passed around and only finally used in one place, I'd rather not
> add an argument but instead pass both values via the single
> parse-ns argument.  parse-ns could then be:
> - nil as before.
> - an alist of namespace->URI.
> - a cons cell (symbol-qnames . ALIST) which does the same as the
>   previous one but uses symbols instead of cons cells for qnames.
> - the symbol `symbol-qnames' to mean (symbol-qnames . STANDARD-ALIST).
> - t to mean STANDARD-ALIST.
> The last two are only allowed when entering xml-parse-region but not in
> recursive calls (and not in calls to xml-maybe-do-ns).

That's... uh... creative. ;-) 

Anyway, it's a very seldom used feature, so I just implemented what you
suggested. Updated ChangeLog:

lisp/xml.el:

(xml-node-name): Mention `symbol-qnames' in doc-string.
(xml-parse-file, xml-parse-region): Explain PARSE-NS argument in the
doc-string.
(xml-maybe-do-ns): Return expanded name as symbol instead of cons
depending on new `simple-qnames' argument.
(xml-parse-tag-1): Deal with new PARSE-NS argument definition.  Add
`symbol-qnames' to other function calls that need it.
(xml-parse-attlist): Add `symbol-qnames' argument.

url-dav.el:

(url-dav-supported-p): Added doc-string and remove check for feature
`xml' and function `xml-expand-namespace' which never existed in Emacs
proper.
(url-dav-process-response): Remove all indentation from XML
before parsing.  Change call to `xml-parse-region' to do namespace
expansion with simple qualified names.
(url-dav-request): Add autoload.
(url-dav-directory-files): Properly deal with empty directories.  Call
hexify before generating relative URLs.
(url-dav-file-directory-p): Fix bug when checking for 'DAV:collection
(resources are returned as a list).


-David


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xml-diff.patch --]
[-- Type: text/x-patch, Size: 7155 bytes --]

=== modified file 'lisp/xml.el'
--- lisp/xml.el	2012-07-04 16:14:05 +0000
+++ lisp/xml.el	2012-07-25 20:54:52 +0000
@@ -126,7 +126,10 @@
 
 would be represented by
 
-    '(\"\" . \"foo\")."
+    '(\"\" . \"foo\").
+
+If you'd just like a plain symbol instead, use 'symbol-qnames in
+the PARSE-NS argument."
 
   (car node))
 
@@ -313,7 +316,22 @@
   "Parse the well-formed XML file FILE.
 Return the top node with all its children.
 If PARSE-DTD is non-nil, the DTD is parsed rather than skipped.
-If PARSE-NS is non-nil, then QNAMES are expanded."
+
+If PARSE-NS is non-nil, then QNAMES are expanded.  By default,
+the variable `xml-default-ns' is the mapping from namespaces to
+URIs, and expanded names will be returned as a cons
+
+  (\"namespace:\" . \"foo\").
+
+If PARSE-NS is an alist, it will be used as the mapping from
+namespace to URIs instead.
+
+If it is the symbol 'symbol-qnames, expanded names will be
+returned as a plain symbol 'namespace:foo instead of a cons.
+
+Both features can be combined by providing a cons cell
+
+  (symbol-qnames . ALIST)."
   (with-temp-buffer
     (insert-file-contents file)
     (xml--parse-buffer parse-dtd parse-ns)))
@@ -329,7 +347,21 @@
 If BUFFER is nil, it defaults to the current buffer.
 If PARSE-DTD is non-nil, parse the DTD and return it as the first
 element of the list.
-If PARSE-NS is non-nil, expand QNAMES."
+If PARSE-NS is non-nil, then QNAMES are expanded.  By default,
+the variable `xml-default-ns' is the mapping from namespaces to
+URIs, and expanded names will be returned as a cons
+
+  (\"namespace:\" . \"foo\").
+
+If PARSE-NS is an alist, it will be used as the mapping from
+namespace to URIs instead.
+
+If it is the symbol 'symbol-qnames, expanded names will be
+returned as a plain symbol 'namespace:foo instead of a cons.
+
+Both features can be combined by providing a cons cell
+
+  (symbol-qnames . ALIST)."
   ;; Use fixed syntax table to ensure regexp char classes and syntax
   ;; specs DTRT.
   (unless buffer
@@ -377,7 +409,7 @@
 	  (cons dtd (nreverse xml))
 	(nreverse xml)))))
 
-(defun xml-maybe-do-ns (name default xml-ns)
+(defun xml-maybe-do-ns (name default xml-ns symbol-qnames)
   "Perform any namespace expansion.
 NAME is the name to perform the expansion on.
 DEFAULT is the default namespace.  XML-NS is a cons of namespace
@@ -386,7 +418,10 @@
 
 During namespace-aware parsing, any name without a namespace is
 put into the namespace identified by DEFAULT.  nil is used to
-specify that the name shouldn't be given a namespace."
+specify that the name shouldn't be given a namespace.
+Expanded names will by default be returned as a cons.  If you
+would like to get plain symbols, set SYMBOL-QNAMES to a non-nil
+value."
   (if (consp xml-ns)
       (let* ((nsp (string-match ":" name))
 	     (lname (if nsp (substring name (match-end 0)) name))
@@ -397,15 +432,18 @@
 	     (ns (or (cdr (assoc (if special "xmlns" prefix)
                                  xml-ns))
                      "")))
-        (cons ns (if special "" lname)))
+	(if (and symbol-qnames
+		 (not (string= prefix "xmlns")))
+	    (intern (concat ns lname))
+	  (cons ns (if special "" lname))))
     (intern name)))
 
 (defun xml-parse-tag (&optional parse-dtd parse-ns)
   "Parse the tag at point.
 If PARSE-DTD is non-nil, the DTD of the document, if any, is parsed and
 returned as the first element in the list.
-If PARSE-NS is non-nil, expand QNAMES; if the value of PARSE-NS
-is a list, use it as an alist mapping namespaces to URIs.
+If PARSE-NS is non-nil, expand QNAMES; for further details, see
+`xml-parse-region'.
 
 Return one of:
  - a list : the matching node
@@ -425,15 +463,23 @@
 
 (defun xml-parse-tag-1 (&optional parse-dtd parse-ns)
   "Like `xml-parse-tag', but possibly modify the buffer while working."
-  (let ((xml-validating-parser (or parse-dtd xml-validating-parser))
-	(xml-ns (cond ((consp parse-ns) parse-ns)
-		      (parse-ns xml-default-ns))))
+  (let* ((xml-validating-parser (or parse-dtd xml-validating-parser))
+	 (symbol-qnames
+	  (when (or (eq parse-ns 'symbol-qnames)
+		    (eq (car-safe parse-ns) 'symbol-qnames))
+	    'symbol-qnames))
+	 (xml-ns
+	  (cond ((symbolp (car-safe parse-ns))
+		 (or (cdr-safe parse-ns)
+		     xml-default-ns))
+		((consp parse-ns) parse-ns)
+		(parse-ns xml-default-ns))))
     (cond
      ;; Processing instructions, like <?xml version="1.0"?>.
      ((looking-at "<\\?")
       (search-forward "?>")
       (skip-syntax-forward " ")
-      (xml-parse-tag-1 parse-dtd xml-ns))
+      (xml-parse-tag-1 parse-dtd (cons symbol-qnames xml-ns)))
      ;; Character data (CDATA) sections, in which no tag should be interpreted
      ((looking-at "<!\\[CDATA\\[")
       (let ((pos (match-end 0)))
@@ -447,8 +493,8 @@
       (let ((dtd (xml-parse-dtd parse-ns)))
 	(skip-syntax-forward " ")
 	(if xml-validating-parser
-	    (cons dtd (xml-parse-tag-1 nil xml-ns))
-	  (xml-parse-tag-1 nil xml-ns))))
+	    (cons dtd (xml-parse-tag-1 nil (cons symbol-qnames xml-ns)))
+	  (xml-parse-tag-1 nil (cons symbol-qnames xml-ns)))))
      ;; skip comments
      ((looking-at "<!--")
       (search-forward "-->")
@@ -456,7 +502,7 @@
       (skip-syntax-forward " ")
       (unless (eobp)
 	(let ((xml-sub-parser t))
-	  (xml-parse-tag-1 parse-dtd xml-ns))))
+	  (xml-parse-tag-1 parse-dtd (cons symbol-qnames xml-ns)))))
      ;; end tag
      ((looking-at "</")
       '())
@@ -466,7 +512,7 @@
       ;; Parse this node
       (let* ((node-name (match-string-no-properties 1))
 	     ;; Parse the attribute list.
-	     (attrs (xml-parse-attlist xml-ns))
+	     (attrs (xml-parse-attlist xml-ns symbol-qnames))
 	     children)
 	;; add the xmlns:* attrs to our cache
 	(when (consp xml-ns)
@@ -476,7 +522,8 @@
 			      (caar attr)))
 	      (push (cons (cdar attr) (cdr attr))
 		    xml-ns))))
-	(setq children (list attrs (xml-maybe-do-ns node-name "" xml-ns)))
+	(setq children (list attrs (xml-maybe-do-ns node-name ""
+						    xml-ns symbol-qnames)))
 	(cond
 	 ;; is this an empty element ?
 	 ((looking-at "/>")
@@ -502,7 +549,7 @@
 		       node-name))
 	       ;; Read a sub-element and push it onto CHILDREN.
 	       ((= (char-after) ?<)
-		(let ((tag (xml-parse-tag-1 nil xml-ns)))
+		(let ((tag (xml-parse-tag-1 nil (cons symbol-qnames xml-ns))))
 		  (when tag
 		    (push tag children))))
 	       ;; Read some character data.
@@ -585,7 +632,7 @@
       (goto-char end-marker)
       (buffer-substring start (point)))))
 
-(defun xml-parse-attlist (&optional xml-ns)
+(defun xml-parse-attlist (&optional xml-ns symbol-qnames)
   "Return the attribute-list after point.
 Leave point at the first non-blank character after the tag."
   (let ((attlist ())
@@ -594,7 +641,8 @@
     (while (looking-at (eval-when-compile
 			 (concat "\\(" xml-name-re "\\)\\s-*=\\s-*")))
       (setq end-pos (match-end 0))
-      (setq name (xml-maybe-do-ns (match-string-no-properties 1) nil xml-ns))
+      (setq name (xml-maybe-do-ns (match-string-no-properties 1)
+				  nil xml-ns symbol-qnames))
       (goto-char end-pos)
 
       ;; See also: http://www.w3.org/TR/2000/REC-xml-20001006#AVNormalize


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: url-dav-diff.patch --]
[-- Type: text/x-patch, Size: 3678 bytes --]

=== modified file 'lisp/url/url-dav.el'
--- lisp/url/url-dav.el	2012-07-11 23:13:41 +0000
+++ lisp/url/url-dav.el	2012-07-25 20:44:16 +0000
@@ -53,10 +53,10 @@
 
 ;;;###autoload
 (defun url-dav-supported-p (url)
-  (and (featurep 'xml)
-       (fboundp 'xml-expand-namespace)
-       (url-intersection url-dav-supported-protocols
-			 (plist-get (url-http-options url) 'dav))))
+  "Return WebDAV protocol version supported by URL.
+Returns nil if WebDAV is not supported."
+  (url-intersection url-dav-supported-protocols
+		    (plist-get (url-http-options url) 'dav)))
 
 (defun url-dav-node-text (node)
   "Return the text data from the XML node NODE."
@@ -385,7 +385,12 @@
     (when buffer
       (unwind-protect
 	  (with-current-buffer buffer
+	    ;; First remove all indentation and line endings
 	    (goto-char url-http-end-of-headers)
+	    (indent-rigidly (point) (point-max) -1000)
+	    (save-excursion
+	      (while (re-search-forward "\r?\n" nil t)
+		(replace-match "")))
 	    (setq overall-status url-http-response-status)
 
 	    ;; XML documents can be transferred as either text/xml or
@@ -395,7 +400,7 @@
 		 url-http-content-type
 		 (string-match "\\`\\(text\\|application\\)/xml"
 			       url-http-content-type))
-		(setq tree (xml-parse-region (point) (point-max)))))
+		(setq tree (xml-parse-region (point) (point-max) nil nil 'symbol-qnames))))
 	;; Clean up after ourselves.
 	(kill-buffer buffer)))
 
@@ -411,6 +416,7 @@
 	;; nobody but us needs to know the difference.
 	(list (cons url properties))))))
 
+;;;###autoload
 (defun url-dav-request (url method tag body
 				 &optional depth headers namespaces)
   "Perform WebDAV operation METHOD on URL.  Return the parsed responses.
@@ -768,8 +774,8 @@
 (defun url-dav-directory-files (url &optional full match nosort files-only)
   "Return a list of names of files in URL.
 There are three optional arguments:
-If FULL is non-nil, return absolute file names.  Otherwise return names
- that are relative to the specified directory.
+If FULL is non-nil, return absolute URLs.  Otherwise return names
+ that are relative to the specified URL.
 If MATCH is non-nil, mention only file names that match the regexp MATCH.
 If NOSORT is non-nil, the list is not sorted--its order is unpredictable.
  NOSORT is useful if you plan to sort the result yourself."
@@ -779,8 +785,9 @@
 	(files nil)
 	(parsed-url (url-generic-parse-url url)))
 
-    (if (= (length properties) 1)
-	(signal 'file-error (list "Opening directory" "not a directory" url)))
+    (when (and (= (length properties) 1)
+	       (not (url-dav-file-directory-p url)))
+      (signal 'file-error (list "Opening directory" "not a directory" url)))
 
     (while properties
       (setq child-props (pop properties)
@@ -794,7 +801,9 @@
 	;; are not supposed to return fully-qualified names.
 	(setq child-url (url-expand-file-name child-url parsed-url))
 	(if (not full)
-	    (setq child-url (substring child-url (length url))))
+	    ;; Parts of the URL might be hex'ed.
+	    (setq child-url (substring (url-unhex-string child-url)
+				       (length url))))
 
 	;; We don't want '/' as the last character in filenames...
 	(if (string-match "/$" child-url)
@@ -814,7 +823,8 @@
 (defun url-dav-file-directory-p (url)
   "Return t if URL names an existing DAV collection."
   (let ((properties (cdar (url-dav-get-properties url '(DAV:resourcetype)))))
-    (eq (plist-get properties 'DAV:resourcetype) 'DAV:collection)))
+    (when (member 'DAV:collection (plist-get properties 'DAV:resourcetype))
+      t)))
 
 (defun url-dav-make-directory (url &optional parents)
   "Create the directory DIR and any nonexistent parent dirs."


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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-25 21:04               ` David Engster
@ 2012-07-26  0:04                 ` Stefan Monnier
  2012-07-26 16:01                   ` David Engster
  0 siblings, 1 reply; 12+ messages in thread
From: Stefan Monnier @ 2012-07-26  0:04 UTC (permalink / raw)
  To: David Engster; +Cc: 11916

> Anyway, it's a very seldom used feature, so I just implemented what you
> suggested.

Not quite.  See below.

>  (defun xml-parse-tag-1 (&optional parse-dtd parse-ns)
>    "Like `xml-parse-tag', but possibly modify the buffer while working."
> -  (let ((xml-validating-parser (or parse-dtd xml-validating-parser))
> -	(xml-ns (cond ((consp parse-ns) parse-ns)
> -		      (parse-ns xml-default-ns))))
> +  (let* ((xml-validating-parser (or parse-dtd xml-validating-parser))
> +	 (symbol-qnames
> +	  (when (or (eq parse-ns 'symbol-qnames)
> +		    (eq (car-safe parse-ns) 'symbol-qnames))
> +	    'symbol-qnames))
> +	 (xml-ns
> +	  (cond ((symbolp (car-safe parse-ns))
> +		 (or (cdr-safe parse-ns)
> +		     xml-default-ns))
> +		((consp parse-ns) parse-ns)
> +		(parse-ns xml-default-ns))))

No, no, don't add a symbol-qnames variable here, use the same
representation I proposed for xml-ns instead.  And similarly don't add
any argument to xml-maybe-do-ns.  This should result in a patch that
touches very little code (just the computation of xml-ns above and its
use in xml-maybe-do-ns, and not much else).


        Stefan





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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-26  0:04                 ` Stefan Monnier
@ 2012-07-26 16:01                   ` David Engster
  2012-07-26 23:32                     ` Stefan Monnier
  0 siblings, 1 reply; 12+ messages in thread
From: David Engster @ 2012-07-26 16:01 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 11916

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

Stefan Monnier writes:
> No, no, don't add a symbol-qnames variable here, use the same
> representation I proposed for xml-ns instead.  And similarly don't add
> any argument to xml-maybe-do-ns.  This should result in a patch that
> touches very little code (just the computation of xml-ns above and its
> use in xml-maybe-do-ns, and not much else).

OK, next try (only attaching diff to xml.el, since url-dav is unchanged).

-David


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xml-diff.patch --]
[-- Type: text/x-patch, Size: 4876 bytes --]

=== modified file 'lisp/xml.el'
--- lisp/xml.el	2012-07-04 16:14:05 +0000
+++ lisp/xml.el	2012-07-26 14:55:34 +0000
@@ -126,7 +126,10 @@
 
 would be represented by
 
-    '(\"\" . \"foo\")."
+    '(\"\" . \"foo\").
+
+If you'd just like a plain symbol instead, use 'symbol-qnames in
+the PARSE-NS argument."
 
   (car node))
 
@@ -313,7 +316,22 @@
   "Parse the well-formed XML file FILE.
 Return the top node with all its children.
 If PARSE-DTD is non-nil, the DTD is parsed rather than skipped.
-If PARSE-NS is non-nil, then QNAMES are expanded."
+
+If PARSE-NS is non-nil, then QNAMES are expanded.  By default,
+the variable `xml-default-ns' is the mapping from namespaces to
+URIs, and expanded names will be returned as a cons
+
+  (\"namespace:\" . \"foo\").
+
+If PARSE-NS is an alist, it will be used as the mapping from
+namespace to URIs instead.
+
+If it is the symbol 'symbol-qnames, expanded names will be
+returned as a plain symbol 'namespace:foo instead of a cons.
+
+Both features can be combined by providing a cons cell
+
+  (symbol-qnames . ALIST)."
   (with-temp-buffer
     (insert-file-contents file)
     (xml--parse-buffer parse-dtd parse-ns)))
@@ -329,7 +347,21 @@
 If BUFFER is nil, it defaults to the current buffer.
 If PARSE-DTD is non-nil, parse the DTD and return it as the first
 element of the list.
-If PARSE-NS is non-nil, expand QNAMES."
+If PARSE-NS is non-nil, then QNAMES are expanded.  By default,
+the variable `xml-default-ns' is the mapping from namespaces to
+URIs, and expanded names will be returned as a cons
+
+  (\"namespace:\" . \"foo\").
+
+If PARSE-NS is an alist, it will be used as the mapping from
+namespace to URIs instead.
+
+If it is the symbol 'symbol-qnames, expanded names will be
+returned as a plain symbol 'namespace:foo instead of a cons.
+
+Both features can be combined by providing a cons cell
+
+  (symbol-qnames . ALIST)."
   ;; Use fixed syntax table to ensure regexp char classes and syntax
   ;; specs DTRT.
   (unless buffer
@@ -386,26 +418,36 @@
 
 During namespace-aware parsing, any name without a namespace is
 put into the namespace identified by DEFAULT.  nil is used to
-specify that the name shouldn't be given a namespace."
+specify that the name shouldn't be given a namespace.
+Expanded names will by default be returned as a cons.  If you
+would like to get plain symbols instead, provide a cons cell
+
+  (symbol-qnames . ALIST)
+
+in the XML-NS argument."
   (if (consp xml-ns)
-      (let* ((nsp (string-match ":" name))
+      (let* ((symbol-qnames (eq (car-safe xml-ns) 'symbol-qnames))
+	     (nsp (string-match ":" name))
 	     (lname (if nsp (substring name (match-end 0)) name))
 	     (prefix (if nsp (substring name 0 (match-beginning 0)) default))
 	     (special (and (string-equal lname "xmlns") (not prefix)))
              ;; Setting default to nil will insure that there is not
              ;; matching cons in xml-ns.  In which case we
 	     (ns (or (cdr (assoc (if special "xmlns" prefix)
-                                 xml-ns))
+                                 (if symbol-qnames (cdr xml-ns) xml-ns)))
                      "")))
-        (cons ns (if special "" lname)))
+	(if (and symbol-qnames
+		 (not (string= prefix "xmlns")))
+	    (intern (concat ns lname))
+	  (cons ns (if special "" lname))))
     (intern name)))
 
 (defun xml-parse-tag (&optional parse-dtd parse-ns)
   "Parse the tag at point.
 If PARSE-DTD is non-nil, the DTD of the document, if any, is parsed and
 returned as the first element in the list.
-If PARSE-NS is non-nil, expand QNAMES; if the value of PARSE-NS
-is a list, use it as an alist mapping namespaces to URIs.
+If PARSE-NS is non-nil, expand QNAMES; for further details, see
+`xml-parse-region'.
 
 Return one of:
  - a list : the matching node
@@ -425,9 +467,16 @@
 
 (defun xml-parse-tag-1 (&optional parse-dtd parse-ns)
   "Like `xml-parse-tag', but possibly modify the buffer while working."
-  (let ((xml-validating-parser (or parse-dtd xml-validating-parser))
-	(xml-ns (cond ((consp parse-ns) parse-ns)
-		      (parse-ns xml-default-ns))))
+  (let* ((xml-validating-parser (or parse-dtd xml-validating-parser))
+	 (xml-ns
+	  (cond ((eq parse-ns 'symbol-qnames)
+		 (cons 'symbol-qnames xml-default-ns))
+		((or (consp (car-safe parse-ns))
+		     (and (eq (car-safe parse-ns) 'symbol-qnames)
+			  (listp (cdr parse-ns))))
+		 parse-ns)
+		(parse-ns
+		 xml-default-ns))))
     (cond
      ;; Processing instructions, like <?xml version="1.0"?>.
      ((looking-at "<\\?")
@@ -475,7 +524,9 @@
 		       (equal "http://www.w3.org/2000/xmlns/"
 			      (caar attr)))
 	      (push (cons (cdar attr) (cdr attr))
-		    xml-ns))))
+		    (if (symbolp (car xml-ns))
+			(cdr xml-ns)
+		      xml-ns)))))
 	(setq children (list attrs (xml-maybe-do-ns node-name "" xml-ns)))
 	(cond
 	 ;; is this an empty element ?


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

* bug#11916: 24.1.50; Making url-dav work
  2012-07-26 16:01                   ` David Engster
@ 2012-07-26 23:32                     ` Stefan Monnier
  0 siblings, 0 replies; 12+ messages in thread
From: Stefan Monnier @ 2012-07-26 23:32 UTC (permalink / raw)
  To: David Engster; +Cc: 11916

> OK, next try (only attaching diff to xml.el, since url-dav is unchanged).

That looks perfect (it even uses my newly introduced "if"
place ;-), thanks, feel free to install,


        Stefan





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

end of thread, other threads:[~2012-07-26 23:32 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-11 21:00 bug#11916: 24.1.50; Making url-dav work David Engster
2012-07-18 12:25 ` Stefan Monnier
2012-07-18 17:45   ` David Engster
2012-07-19  7:15     ` Stefan Monnier
2012-07-19 15:28       ` David Engster
2012-07-19 22:12         ` Stefan Monnier
2012-07-21 12:11           ` David Engster
2012-07-22 10:11             ` Stefan Monnier
2012-07-25 21:04               ` David Engster
2012-07-26  0:04                 ` Stefan Monnier
2012-07-26 16:01                   ` David Engster
2012-07-26 23:32                     ` Stefan Monnier

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