Hi Nicolas
I would like to ask you to review the attached patch so I can change
it when necessary before I git push it.
> it probably means that `org-link-escape' is a bit too zealous (BTW
> why don't this function rely on `url-encode-url'?)
url-encode-url is a very good hint to solve a different issue that
I try to deal with:
(from http://lists.gnu.org/archive/html/emacs-orgmode/2013-10/msg00204.html)
On Sat, Oct 5, 2013 at 3:04 PM, Michael Brand
<michael.ch.br...@gmail.com> wrote:
> [...] related change that I will suggest with an ERT in a later
> patch: Just add "+" to org-link-escape-chars-browser.
"+" has not been added to org-link-escape-chars-browser yet and in the
meantime I realized that it should not be added in order to not break
existing Org links like:
[[http://lists.gnu.org/archive/cgi-bin/namazu.cgi?idxname=emacs-orgmode&query="Release+8.2";]]
that work the same as
[[http://lists.gnu.org/archive/cgi-bin/namazu.cgi?idxname=emacs-orgmode&query="Release
8.2"]]
Better is to change the function org-link-escape-browser to use
url-encode-url when available (since Emacs 24.3). With this I can
write my use case to open a browser with +subject:"Release 8.2" in the
query field now as an Org link written manually with %2B for the "+"
at the beginning and %25 for its "%" like this
[[http://lists.gnu.org/archive/cgi-bin/namazu.cgi?idxname=emacs-orgmode&query=%252Bsubject:"Release+8.2";]]
or
[[http://lists.gnu.org/archive/cgi-bin/namazu.cgi?idxname=emacs-orgmode&query=%252Bsubject:"Release
8.2"]]
Link escaping in org-store-link and link unescaping in the first part
of org-open-at-point are not changed, this is important to keep
backward compatibility with old Org links. I have used this patch for
several weeks on Emacs 24.3.2 without any problem.
Michael
commit 686bd788243ef38951c3c529d4c67c3ad766f417
Author: Michael Brand <michael.ch.br...@gmail.com>
Date: Sat Nov 16 16:13:57 2013 +0100
Hyperlink: Use url-encode-url for browse-url
* lisp/org.el (org-open-at-point): When available (Emacs 24.3) use
`url-encode-url' instead of `org-link-escape-browser'.
* testing/lisp/test-org.el
(test-org/org-link-escape-url-with-escaped-char): Substitute repeated
literal string with constant.
(test-org/org-link-escape-chars-browser): Extend test coverage with
`url-encode-url' and with "query="-space as plus sign or space.
diff --git a/lisp/org.el b/lisp/org.el
index a3c1958..aa91ffc 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -10468,11 +10468,20 @@ application the system uses for this file type."
(apply cmd (nreverse args1))))
((member type '("http" "https" "ftp" "news"))
- (browse-url (concat type ":" (org-link-escape-browser path))))
+ ;; see `ert-deftest'
+ ;; `test-org/org-link-escape-chars-browser'
+ (browse-url
+ (if (fboundp 'url-encode-url)
+ (url-encode-url (concat type ":" path))
+ (org-link-escape-browser (concat type ":" path)))))
((string= type "doi")
- (browse-url (concat org-doi-server-url
- (org-link-escape-browser path))))
+ ;; see `ert-deftest'
+ ;; `test-org/org-link-escape-chars-browser'
+ (browse-url
+ (if (fboundp 'url-encode-url)
+ (url-encode-url (concat org-doi-server-url path))
+ (org-link-escape-browser (concat org-doi-server-url path)))))
((member type '("message"))
(browse-url (concat type ":" path)))
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index f4672eb..084e95d 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -552,21 +552,53 @@
(ert-deftest test-org/org-link-escape-url-with-escaped-char ()
"Escape and unescape a URL that includes an escaped char.
http://article.gmane.org/gmane.emacs.orgmode/21459/";
- (should
- (string=
- "http://some.host.com/form?&id=blah%2Bblah25";
- (org-link-unescape
- (org-link-escape "http://some.host.com/form?&id=blah%2Bblah25";)))))
+ (let ((a "http://some.host.com/form?&id=blah%2Bblah25";))
+ (should (string= a (org-link-unescape (org-link-escape a))))))
(ert-deftest test-org/org-link-escape-chars-browser ()
- "Escape a URL to pass to `browse-url'."
- (should
- (string=
- (concat "http://lists.gnu.org/archive/cgi-bin/namazu.cgi?query=";
- "%22Release%208.2%22&idxname=emacs-orgmode")
- (org-link-escape-browser
- (concat "http://lists.gnu.org/archive/cgi-bin/namazu.cgi?query=";
- "\"Release 8.2\"&idxname=emacs-orgmode")))))
+ "Escape a URL before passing it to `browse-url'.
+
+This test is to ensure that `org-open-at-point' on the Org links
+
+
[[http://lists.gnu.org/archive/cgi-bin/namazu.cgi?idxname=emacs-orgmode&query=%252Bsubject:\"Release+8.2\";]]
+
[[http://lists.gnu.org/archive/cgi-bin/namazu.cgi?idxname=emacs-orgmode&query=%252Bsubject:\"Release
8.2\"]]
+
+will open a browser with +subject:\"Release 8.2\" in the query
+field."
+
+ ;; Each string argument passed to `url-encode-url' or
+ ;; `org-link-escape-browser' in the tests below (or when
+ ;; `org-open-at-point' is used in an Org buffer) looks like after
+ ;; the Org link from the docstring has been unescaped by
+ ;; `org-link-unescape' in `org-open-at-point'
+ (let ((query (concat "http://lists.gnu.org/archive/cgi-bin/namazu.cgi?";
+ "idxname=emacs-orgmode&query="))
+ (plus "%2Bsubject:\"Release+8.2\"") ; "query="-space as plus sign
+ (space "%2Bsubject:\"Release 8.2\"")) ; "query="-space as space
+
+ ;; This is the behavior of `org-open-at-point' when used together
+ ;; with an Emacs 24.3 or later where `url-encode-url' is available
+ (when (fboundp 'url-encode-url)
+ ;; "query="-space as plus sign
+ (should (string= (concat query "%2Bsubject:%22Release+8.2%22")
+ (url-encode-url (concat query plus))))
+ ;; "query="-space as space
+ (should (string= (concat query "%2Bsubject:%22Release%208.2%22")
+ (url-encode-url (concat query space)))))
+
+ ;; The %252B below returned from `org-link-escape-browser' is not
+ ;; desired and not working with some browser/OS but tested here to
+ ;; document what happens when the fallback to
+ ;; `org-link-escape-browser' in `org-open-at-point' is in use,
+ ;; which is the legacy behavior of `org-open-at-point' when used
+ ;; together with an Emacs before version 24.3
+ ;;
+ ;; "query="-space as plus sign
+ (should (string= (concat query "%252Bsubject:%22Release+8.2%22")
+ (org-link-escape-browser (concat query plus))))
+ ;; "query="-space as space
+ (should (string= (concat query "%252Bsubject:%22Release%208.2%22")
+ (org-link-escape-browser (concat query space))))))
