From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rasmus Subject: Re: [PATCH] org-protocol: Allow optional port specification Date: Wed, 02 Dec 2015 20:32:32 +0100 Message-ID: <87zixswsrz.fsf@pank.eu> References: <87wpswrg7q.fsf@sachachua.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:46663) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4D8z-0007ST-1g for emacs-orgmode@gnu.org; Wed, 02 Dec 2015 14:32:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4D8v-0004QE-Nh for emacs-orgmode@gnu.org; Wed, 02 Dec 2015 14:32:48 -0500 Received: from plane.gmane.org ([80.91.229.3]:58964) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4D8v-0004Q1-HU for emacs-orgmode@gnu.org; Wed, 02 Dec 2015 14:32:45 -0500 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1a4D8s-0000tO-Rt for emacs-orgmode@gnu.org; Wed, 02 Dec 2015 20:32:43 +0100 Received: from 46.166.188.236 ([46.166.188.236]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 02 Dec 2015 20:32:42 +0100 Received: from rasmus by 46.166.188.236 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 02 Dec 2015 20:32:42 +0100 List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: emacs-orgmode@gnu.org Hi Sacha, Thanks for your patch. Sacha Chua writes: > I was trying to get org-protocol to work on KDE Plasma 5.4.2. I set up > my ~/.kde/share/kde4/services/org.protocol, but the standard > org-protocol sample syntax: > > org-protocol://store-link://URL/TITLE > > resulted in the error: > > Malformed URL > Port field was empty; source was "..."; scheme = "org-protocol", > host = "store-link", path = "// ..." > > Modifying my Javascript to create links of the form: > > org-protocol://store-link:0//URL/TITLE > > made org-protocol correctly pass the link to emacsclient KDE 5.4.2. This > patch allows the optional specification of a port in the URI. What do > you think? First, I’m not familiar with org-protocol or messaging systems in general so I cannot offer input on this. > @@ -532,7 +532,7 @@ as filename." > (when (string-match the-protocol fname) > (dolist (prolist sub-protocols) > (let ((proto (concat the-protocol > - (regexp-quote (plist-get (cdr prolist) :protocol)) ":/+"))) > + (regexp-quote (plist-get (cdr prolist) :protocol)) ":[^/]*/+"))) This seems pretty general. Are there any dangerous of accepting everything but /? Is this only meant to capture a port? If so, is there any disadvantage in only allowing numbers? As said, I don’t know anything about these topics. > +;;; test-org-protocol.el --- tests for org-protocol.el Tests Also, I guess you should add -*- lexical-binding: t; -*- these days. > +;;; Code: > + > +(ert-deftest test-org-protocol/org-protocol-check-filename-for-protocol () > + "Test `org-protocol-check-filename-for-protocol' specifications." > + ;; Store link > + (let ((uri "/some/directory/org-protocol:/store-link:/URL/TITLE")) > + (should (null (org-protocol-check-filename-for-protocol uri (list uri) nil)))) > + (should (equal (car org-stored-links) '("URL" "TITLE"))) > + ;; Handle multiple slashes > + (let ((uri "/some/directory/org-protocol://store-link://URL2//TITLE2")) > + (should (null (org-protocol-check-filename-for-protocol uri (list uri) nil)))) > + (should (equal (car org-stored-links) '("URL2" "TITLE2"))) > + ;; Ignore port - useful for KDE > + (let ((uri "/some/directory/org-protocol:/store-link:0//URL3//TITLE3")) > + (should (null (org-protocol-check-filename-for-protocol uri (list uri) nil)))) > + (should (equal (car org-stored-links) '("URL3" "TITLE3")))) I don't know org-protocol well enough to comment on your tests. But I guess you should add something like this, extrapolating from other test files, (unless (featurep 'org-protocol) (signal 'missing-test-dependency "org-protocol")) Cheers, Rasmus -- ⠠⠵