From mboxrd@z Thu Jan 1 00:00:00 1970 From: Aaron Ecay Subject: Re: [PATCH] org-protocol: Allow key=val&key2=value2-style URLs Date: Sat, 05 Dec 2015 13:35:42 +0000 Message-ID: <87k2ot2f2p.fsf@gmail.com> References: <87wpswrg7q.fsf@sachachua.com> <87bna7bbl5.fsf@gmail.com> <8737vh3jtq.fsf_-_@sachachua.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:56998) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a5D08-0004gp-C8 for emacs-orgmode@gnu.org; Sat, 05 Dec 2015 08:35:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a5D05-0005BO-5K for emacs-orgmode@gnu.org; Sat, 05 Dec 2015 08:35:48 -0500 Received: from mail-wm0-x230.google.com ([2a00:1450:400c:c09::230]:38484) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a5D04-0005BH-SE for emacs-orgmode@gnu.org; Sat, 05 Dec 2015 08:35:45 -0500 Received: by wmec201 with SMTP id c201so98222368wme.1 for ; Sat, 05 Dec 2015 05:35:44 -0800 (PST) In-Reply-To: <8737vh3jtq.fsf_-_@sachachua.com> 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: Sacha Chua , emacs-orgmode@gnu.org Hi Sacha, Thanks for the patch. It looks great! I assume eventually we will want to deprecate the old-style links, and make new-style the only style. Unfortunately, this would mean another API change to remove the =E2=80=98new-style=E2=80=99 arguments from these f= unctions. I don=E2=80=99t have any clever ideas to solve this, and it=E2=80=99s not a= n objection to your patch =E2=80=93 just something I thought I=E2=80=99d mention in cas= e someone can think of a way to deal with the eventual deprecation without an API change. A few comments below: 2015ko abenudak 4an, Sacha Chua-ek idatzi zuen: > From aff151930a73c22bb3fdf3ae9b442cecc08aaa67 Mon Sep 17 00:00:00 2001 > From: Sacha Chua > Date: Wed, 2 Dec 2015 10:53:07 -0500 > Subject: [PATCH] org-protocol: Allow key=3Dval&key2=3Dval2-style URLs >=20 > * lisp/org-protocol.el: Update documentation. > (org-protocol-parse-parameters): New function to simplify handling of > old- or new-style links. > (org-protocol-assign-parameters): New function to simplify handling of > old- or new-style links. You can combine these like: (org-protocol-parse-parameters, org-protocol-assign-parameters): New functions. I also think the convention in Changelogs is not to put in details, but just to say =E2=80=9CNew function=E2=80=9D or =E2=80=9CAccept new-style lin= ks=E2=80=9D. A narrative explanation can be put in the git commit message below the changelog section (and will not be included in the Changelog file distributed with Emacs). But I=E2=80=99ll admit I don=E2=80=99t understand Changelog conven= tions and think they are a pointless relic, so YMMV. [...] >=20=20 > +(defun org-protocol-parse-parameters (info new-style &optional default-o= rder unhexify separator) Is there ever a case where we would want unhexify to be something other than t? Hexification is imposed by the URL format, there is no optionality about it. Handler functions get access to the raw string if they need it for some reason, I don=E2=80=99t think our helper functions need to bother = with the unhexify !=3D t case. Similarly, I would not have a separator argument, but use the value of =E2=80=98org-protocol-data-separator=E2=80=99 directly. I= n the rare case that a caller needs to influence the separator, they can let-bind that variable. TLDR: can we get rid of unhexify and separator arguments? [...] =20=20 > (defun org-protocol-check-filename-for-protocol (fname restoffiles clien= t) > [...docstring omitted...] > (let ((sub-protocols (append org-protocol-protocol-alist > org-protocol-protocol-alist-default))) > (catch 'fname > @@ -532,19 +604,27 @@ 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)) "\\(:/+\\|\\?\\)= "))) > (when (string-match proto fname) > (let* ((func (plist-get (cdr prolist) :function)) > (greedy (plist-get (cdr prolist) :greedy)) > (split (split-string fname proto)) > - (result (if greedy restoffiles (cadr split)))) > + (result (if greedy restoffiles (cadr split))) > + (new-style (string=3D (match-string 1 fname) "?"))) As a way to encourage users to move to new-style links, should we add a warning if new-style =3D nil? Thanks again for the patch, --=20 Aaron Ecay