Hi, Is there any reason why org-protocol tests are not run by default? Is dependency on server.el considered as too heavy for routine testing? I know, the tests are broken, but I suspect, it is a consequence that additional efforts are required to run them. Almost unrelated note: either I missed something or org tests depend on a couple of things unavailable in emacs-25, e.g. temporary-file-directory function.
* testing/lisp/test-org-protocol.el (test-org-protocol/org-protocol-parse-parameters): Specify that the case simulating real life capture uses new style parameters string to prevent test failure. It looks like a typo survived since addition of this case in 2216f4d2c7. --- testing/lisp/test-org-protocol.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/lisp/test-org-protocol.el b/testing/lisp/test-org-protocol.el index b5df7c132..5ab96fdc2 100644 --- a/testing/lisp/test-org-protocol.el +++ b/testing/lisp/test-org-protocol.el @@ -40,7 +40,7 @@ "url=https%3A%2F%2Forgmode.org%2Forg.html%23capture-protocol&" "title=The%20Org%20Manual&" "body=9.4.2%20capture%20protocol")) - (data (org-protocol-parse-parameters url))) + (data (org-protocol-parse-parameters url t))) (should (string= (plist-get data :template) "p")) (should (string= (plist-get data :url) "https://orgmode.org/org.html#capture-protocol")) (should (string= (plist-get data :title) "The Org Manual")) -- 2.25.1
* lisp/org-protocol.el (org-protocol-check-filename-for-protocol): Avoid incorrect regexp in check whether command line argument uses new syntax. Fix failures of org-protocol tests. Question mark was not escaped in the previous version 928e67df7e, so any string was matched by lazy "*". Match in never used, thus `string-match-p` would be better, but actually regexp is redundant here. It is not documented what browser or desktop environment adds extra slash before "?". Accordingly to mid:A2B0655F-BF28-4943-BC05-99021BFDA1B3@robewald.de, Windows may be involved. Likely it happens with double slash after schema as in org-protocol://capture?url=URL&title=TITLE due to subprotocol is considered as host name and URI is normalized by adding a slash as mandatory path part before "?" query. So just reverting the original commit will likely cause a regression. Another guess is that with single or triple slash (org-protocol:/capture?url=URL) subprotocol is a part of path thus no "smart" actions are necessary. --- lisp/org-protocol.el | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el index 731f51e19..86a8dc3bc 100644 --- a/lisp/org-protocol.el +++ b/lisp/org-protocol.el @@ -631,7 +631,7 @@ CLIENT is ignored." (greedy (plist-get (cdr prolist) :greedy)) (split (split-string fname proto)) (result (if greedy restoffiles (cadr split))) - (new-style (string-match "/*?" (match-string 1 fname)))) + (new-style (not (= ?: (aref (match-string 1 fname) 0))))) (when (plist-get (cdr prolist) :kill-client) (message "Greedy org-protocol handler. Killing client.") (server-edit)) -- 2.25.1
* testing/lisp/test-org-protocol.el (test-org-protocol/org-protocol-store-link-file, test-org-protocol/org-protocol-capture-file): Add tests to document that existing calls to `org-protocol-sanitize-uri' could make passed URLs invalid by changing number of slashes after scheme. Till a fix of the problem the new tests are marked as expected failures. Unless a relatively recent commit 5748615c48, I would believe that `org-protocol-sanitize-uri' was added by mistake to the initial implementation. Capture URLs are anyway escaped with percent encoding. --- testing/lisp/test-org-protocol.el | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/testing/lisp/test-org-protocol.el b/testing/lisp/test-org-protocol.el index 5ab96fdc2..d33052b30 100644 --- a/testing/lisp/test-org-protocol.el +++ b/testing/lisp/test-org-protocol.el @@ -78,6 +78,16 @@ (should (null (org-protocol-check-filename-for-protocol uri (list uri) nil))) (should (equal (car org-stored-links) '("URL3" "TITLE3"))))) +(ert-deftest test-org-protocol/org-protocol-store-link-file () + "store-link: `org-protocol-sanitize-uri' could distort URL." + :expected-result :failed + (let ((uri "/org-protocol:/store-link:/file%3A%2F%2F%2Fetc%2Fmailcap/Triple%20Slash")) + (should (null (org-protocol-check-filename-for-protocol uri (list uri) nil))) + (should (equal (car org-stored-links) '("file:///etc/mailcap" "Triple Slash")))) + (let ((uri "/org-protocol:/store-link?url=file%3A%2F%2F%2Fetc%2Fmailcap&title=Triple%20Slash")) + (should (null (org-protocol-check-filename-for-protocol uri (list uri) nil))) + (should (equal (car org-stored-links) '("file:///etc/mailcap" "Triple Slash"))))) + (ert-deftest test-org-protocol/org-protocol-capture () "Test `org-protocol-capture' specifications." (let* ((org-protocol-default-template-key "t") @@ -134,6 +144,20 @@ test-urls) (delete-file temp-file-name))) +(ert-deftest test-org-protocol/org-protocol-capture-file () + "capture: `org-protocol-sanitize-uri' could distort URL." + :expected-result :failed + (let* ((org-protocol-default-template-key "t") + (temp-file-name (make-temp-file "org-protocol-test")) + (org-capture-templates + `(("t" "Test" plain (file ,temp-file-name) "%a\n%i\n" :kill-buffer t)))) + (let ((uri "/org-protocol:/capture:/t/file%3A%2F%2F%2Fetc%2Fmailcap/Triple%20Slash/Body")) + (should (null (org-protocol-check-filename-for-protocol uri (list uri) nil))) + (should (string= (buffer-string) "[[file:///etc/mailcap][Triple Slash]]\nBody"))) + (let ((uri "/org-protocol:/capture?template=t&url=file%3A%2F%2F%2Fetc%2Fmailcap&title=Triple%20Slash&body=Body")) + (should (null (org-protocol-check-filename-for-protocol uri (list uri) nil))) + (should (string= (buffer-string) "[[file:///etc/mailcap][Triple Slash]]\nBody"))))) + (ert-deftest test-org-protocol/org-protocol-open-source () "Test org-protocol://open-source links." (let* ((temp-file-name1 (make-temp-file "org-protocol-test1")) -- 2.25.1
Applied in maint, thanks and sorry for the slow reply.
Maxim Nikulin <manikulin@gmail.com> writes:
> * testing/lisp/test-org-protocol.el
> (test-org-protocol/org-protocol-parse-parameters): Specify that the case
> simulating real life capture uses new style parameters string
> to prevent test failure.
>
> It looks like a typo survived since addition of this case in
> 2216f4d2c7.
--
Bastien
Applied as 3dbeb677e in master, thanks.
Maxim Nikulin <manikulin@gmail.com> writes:
> * testing/lisp/test-org-protocol.el
> (test-org-protocol/org-protocol-store-link-file,
> test-org-protocol/org-protocol-capture-file): Add tests to document
> that existing calls to `org-protocol-sanitize-uri' could make passed
> URLs invalid by changing number of slashes after scheme.
--
Bastien
Applied in maint, thanks! -- Bastien