[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] Encoding Problem in export?
From: |
Nicolas Goaziou |
Subject: |
Re: [O] Encoding Problem in export? |
Date: |
Sat, 16 Nov 2013 21:43:02 +0100 |
Hello,
Michael Brand <address@hidden> writes:
> I would like to ask you to review the attached patch so I can change
> it when necessary before I git push it.
Sure.
> - (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)))))
IMO, the following is nicer:
(funcall (if (fboundp 'url-encode-url) #'url-encode-url
#'org-link-escape-browser)
(concat type ":" path))
Also, it's better to document this in the source code rather than in the
test suite. Also, you could add, as a reminder, that we can remove
`org-link-escape-browser' altogether once we drop support for Emacs 23.
> - (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)))))
Ditto.
> - (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))))))
No need to change this. Moreover, I tend to prefer `should' outside the
sexp because it is easier to debug, when needed (`should' is quite
opaque when stepping through the function).
> + ;; 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)))))
You are testing `url-encode-url' here, not an Org function. Is it really
required?
Regards,
--
Nicolas Goaziou