Re: [O] [PATCH] org-protocol: Allow key=val=value2-style URLs

[Nicolas Goaziou]

Hello,

> Sure! Revised patch attached updates docstrings and adds ORG-NEWS.
>
> I also added more test cases and fixed a few small edge cases (ex:
> org-capture with just a template, no links). I reworded the
> documentation strings to pass M-x checkdoc as well. =)

Great ! 

Applied. Thank you.


Regards,

-- 
Nicolas Goaziou

Re: [O] [PATCH] org-protocol: Allow key=val=value2-style URLs

[Sacha Chua]

Nicolas Goaziou  writes:

Hello, Nicolas!

> Apparently, there is no more feedback, so I think it's good to go. 
> One minor issue, however: could you check that sentences (in comments
> and docstrings) are always separated by two spaces?
> Also, would you mind providing an entry for ORG-NEWS?

Sure! Revised patch attached updates docstrings and adds ORG-NEWS.

I also added more test cases and fixed a few small edge cases (ex:
org-capture with just a template, no links). I reworded the
documentation strings to pass M-x checkdoc as well. =)

>From 6045c5856c9f411ae4c337cef1a30848b8e7975e 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=val=val2-style URLs

* lisp/org-protocol.el: Update documentation.
  (org-protocol-store-link, org-protocol-capture,
  org-protocol-open-source): Accept new-style links.
  (org-protocol-check-filename-for-protocol): Update documentation.
  (org-protocol-parse-parameters, org-protocol-assign-parameters):
  New functions.

  This allows the use of org-protocol on KDE 5 and makes org-protocol
  links more URI-like.  New-style links are of the form:
  org-protocol://store-link?title=TITLE=URL

* testing/lisp/test-org-protocol.el: New file.

* etc/NEWS: Add org-protocol new-style links.
---
 etc/ORG-NEWS  |  19 +++
 lisp/org-protocol.el  | 283 +-
 testing/lisp/test-org-protocol.el | 191 +
 3 files changed, 396 insertions(+), 97 deletions(-)
 create mode 100644 testing/lisp/test-org-protocol.el

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 0422ff1..9d7178e 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -11,6 +11,25 @@ Please send Org bug reports to mailto:emacs-orgmode@gnu.org.
 * Version 9.0
 
 ** New features
+*** New org-protocol key=value syntax
+
+Org-protocol can now handle query-style parameters such as:
+
+#+begin_example
+org-protocol://store-link?url=http:%2F%2Flocalhost%2Findex.html=The%20title
+org-protocol://capture?template=x=Hello=World=http:%2F%2Fexample.com
+#+end_example
+
+Old-style links such as
+=org-protocol://store-link:/http:%2F%2Flocalhost%2Findex.html/The%20title=
+continue to be supported.
+
+If you have defined your own handler functions for
+~org-protocol-protocol-alist~, change them to accept either a property
+list (for new-style links) or a string (for old-style links).  Use
+~org-protocol-parse-parameters~ to convert old-style links into
+property lists.
+
 *** Org linter
 ~org-lint~ can check syntax and report common issues in Org documents.
 *** New option ~date-tree-last~ for ~org-agenda-insert-diary-strategy~
diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 339f2b7..6ee9163 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -49,7 +49,7 @@
 ;;   4.) Try this from the command line (adjust the URL as needed):
 ;;
 ;;   $ emacsclient \
-;; org-protocol://store-link://http:%2F%2Flocalhost%2Findex.html/The%20title
+;; org-protocol://store-link?url=http:%2F%2Flocalhost%2Findex.html=The%20title
 ;;
 ;;   5.) Optionally add custom sub-protocols and handlers:
 ;;
@@ -60,7 +60,7 @@
 ;;
 ;;   A "sub-protocol" will be found in URLs like this:
 ;;
-;;   org-protocol://sub-protocol://data
+;;   org-protocol://sub-protocol?key=val=val2
 ;;
 ;; If it works, you can now setup other applications for using this feature.
 ;;
@@ -94,20 +94,20 @@
 ;; You may use the same bookmark URL for all those standard handlers and just
 ;; adjust the sub-protocol used:
 ;;
-;; location.href='org-protocol://sub-protocol://'+
-;;   encodeURIComponent(location.href)+'/'+
-;;   encodeURIComponent(document.title)+'/'+
+;; location.href='org-protocol://sub-protocol?url='+
+;;   encodeURIComponent(location.href)+'='+
+;;   encodeURIComponent(document.title)+'='+
 ;;   encodeURIComponent(window.getSelection())
 ;;
 ;; The handler for the sub-protocol \"capture\" detects an optional template
 ;; char that, if present, triggers the use of a special template.
 ;; Example:
 ;;
-;; location.href='org-protocol://sub-protocol://x/'+ ...
+;; location.href='org-protocol://capture?template=x'+ ...
 ;;
-;;  use template ?x.
+;;  uses template ?x.
 ;;
-;; Note, that using double slashes is optional from org-protocol.el's point of
+;; Note that using double slashes is optional from org-protocol.el's point of
 ;; view because emacsclient squashes the slashes to one.
 ;;
 ;;
@@ -225,27 +225,36 @@ Each element of this list must be of the form:
 
   (module-name :protocol protocol :function func :kill-client nil)
 
-protocol - protocol to detect in a filename without trailing colon and slashes.
-   See rfc1738 section 2.1 for more on this.
-   If you define a protocol \"my-protocol\", `org-protocol-check-filename-for-protocol'
-   will search 

Re: [O] [PATCH] org-protocol: Allow key=val=value2-style URLs

[Nicolas Goaziou]

Hello,

Sacha Chua  writes:

> Following up on this patch. Do you need anything else from me before you
> merge this? What happens next? =)

Apparently, there is no more feedback, so I think it's good to go. 

One minor issue, however: could you check that sentences (in comments
and docstrings) are always separated by two spaces?

Also, would you mind providing an entry for ORG-NEWS?

Thank you.


Regards,

-- 
Nicolas Goaziou

Re: [O] [PATCH] org-protocol: Allow key=val=value2-style URLs

[Sacha Chua]

Sacha Chua  writes:

Hello, all!

> On second thought, your suggestion for always unhexifying makes the
> calls from the other handlers simpler, so I've attached a corrected patch.

Following up on this patch. Do you need anything else from me before you
merge this? What happens next? =)

Sacha

Re: [O] [PATCH] org-protocol: Allow key=val=value2-style URLs

[Sacha Chua]

Sacha Chua  writes:

Hello, Aaron, all!

> Beats me, but the previous implementation made it a separate parameter
> for org-protocol-split-data, so I figured I'd carry it upwards into this
...
> Same notes as for hexify. Probably okay either way, but I don't know
> enough about who else uses this code to say. =) I can change it if you'd
> like to make that decision.

On second thought, your suggestion for always unhexifying makes the
calls from the other handlers simpler, so I've attached a corrected patch.
(In fact, my previous implementation forgot to unhexify something!
)

>From 080642a2452995f67709becc37e76ee5dd1dc8d3 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=val=val2-style URLs

* lisp/org-protocol.el: Update documentation.
  (org-protocol-store-link, org-protocol-capture,
  org-protocol-open-source): Accept new-style links.
  (org-protocol-check-filename-for-protocol): Updated documentation.
  (org-protocol-parse-parameters, org-protocol-assign-parameters):
  New functions.

  This allows the use of org-protocol on KDE 5 and makes org-protocol
  links more URI-like. New-style links are of the form:
  org-protocol://store-link?title=TITLE=URL

* testing/lisp/test-org-protocol.el: New file.
---
 lisp/org-protocol.el  | 191 ++
 testing/lisp/test-org-protocol.el | 181 
 2 files changed, 315 insertions(+), 57 deletions(-)
 create mode 100644 testing/lisp/test-org-protocol.el

diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 339f2b7..ce7cb36 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -49,7 +49,7 @@
 ;;   4.) Try this from the command line (adjust the URL as needed):
 ;;
 ;;   $ emacsclient \
-;; org-protocol://store-link://http:%2F%2Flocalhost%2Findex.html/The%20title
+;; org-protocol://store-link?url=http:%2F%2Flocalhost%2Findex.html=The%20title
 ;;
 ;;   5.) Optionally add custom sub-protocols and handlers:
 ;;
@@ -60,7 +60,7 @@
 ;;
 ;;   A "sub-protocol" will be found in URLs like this:
 ;;
-;;   org-protocol://sub-protocol://data
+;;   org-protocol://sub-protocol?key=val=val2
 ;;
 ;; If it works, you can now setup other applications for using this feature.
 ;;
@@ -94,20 +94,20 @@
 ;; You may use the same bookmark URL for all those standard handlers and just
 ;; adjust the sub-protocol used:
 ;;
-;; location.href='org-protocol://sub-protocol://'+
-;;   encodeURIComponent(location.href)+'/'+
-;;   encodeURIComponent(document.title)+'/'+
+;; location.href='org-protocol://sub-protocol?url='+
+;;   encodeURIComponent(location.href)+'='+
+;;   encodeURIComponent(document.title)+'='+
 ;;   encodeURIComponent(window.getSelection())
 ;;
 ;; The handler for the sub-protocol \"capture\" detects an optional template
 ;; char that, if present, triggers the use of a special template.
 ;; Example:
 ;;
-;; location.href='org-protocol://sub-protocol://x/'+ ...
+;; location.href='org-protocol://capture?template=x'+ ...
 ;;
-;;  use template ?x.
+;;  uses template ?x.
 ;;
-;; Note, that using double slashes is optional from org-protocol.el's point of
+;; Note that using double slashes is optional from org-protocol.el's point of
 ;; view because emacsclient squashes the slashes to one.
 ;;
 ;;
@@ -233,19 +233,27 @@ protocol - protocol to detect in a filename without trailing colon and slashes.
`org-protocol-the-protocol'.  Double and triple slashes are compressed
to one by emacsclient.
 
-function - function that handles requests with protocol and takes exactly one
-   argument: the filename with all protocols stripped.  If the function
-   returns nil, emacsclient and -server do nothing.  Any non-nil return
-   value is considered a valid filename and thus passed to the server.
+function - function that handles requests with protocol and takes
+   one argument. If a new-style link (key=val=val2)
+   is given, the argument will be a property list with
+   the values from the link. If an old-style link is
+   given (val1/val2), the argument will be the filename
+   with all protocols stripped.
 
-   `org-protocol.el provides some support for handling those filenames,
-   if you stay with the conventions used for the standard handlers in
-   `org-protocol-protocol-alist-default'.  See `org-protocol-split-data'.
+   If the function returns nil, emacsclient and -server
+   do nothing.  Any non-nil return value is considered a
+   valid filename and thus passed to the server.
+
+   `org-protocol.el' provides some support for handling
+   old-style filenames, if you stay with the conventions
+   used for the standard handlers in
+   

Re: [O] [PATCH] org-protocol: Allow key=val=value2-style URLs

[Sacha Chua]

Aaron Ecay  writes:

Hello, Aaron, all!

> API change to remove the ‘new-style’ arguments from these functions.
> I don’t have any clever ideas to solve this, and it’s not an objection

How about this approach? If it's new-style, we pass the new-style
property list as the only parameter to the org-protocol function. Since
the standard org-protocol handlers have been updated to deal with
property lists, they should cover maybe 95% of use cases. I've changed
the condition-case to a more general error handler, so anyone with
custom functions (who presumably uses old-style links too) will get the
old-style behaviour and a warning.

> (org-protocol-parse-parameters, org-protocol-assign-parameters): New
> functions.

Done.

> I also think the convention in Changelogs is not to put in details, but
> just to say “New function” or “Accept new-style links”.  A narrative
> explanation can be put in the git commit message below the changelog

I have no idea either. I've simplified the changelog message, though.

>> +(defun org-protocol-parse-parameters (info new-style  
>> default-order 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

Beats me, but the previous implementation made it a separate parameter
for org-protocol-split-data, so I figured I'd carry it upwards into this
new wrapper function. The previous implementation of
org-protocol-open-source unhexified after parsing the parameters instead
of passing the unhexify parameter to org-protocol-split-data directly.
I'm not sure if anyone's relying on the nuances of that, so I left it
roughly the same.

> about it.  Handler functions get access to the raw string if they need it
> for some reason, I don’t think our helper functions need to bother

Now that I've changed handler functions to use only one parameter, they
don't get the raw strings unless they're greedy handler functions or
they use old-style links... Hmm.

> unhexify != t case. Similarly, I would not have a separator argument,
> but use the value of ‘org-protocol-data-separator’ directly. In 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?

Same notes as for hexify. Probably okay either way, but I don't know
enough about who else uses this code to say. =) I can change it if you'd
like to make that decision.

>From dbe56d80028da9d335d91673c7871b6ebac0e6e7 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=val=val2-style URLs

* lisp/org-protocol.el: Update documentation.
  (org-protocol-store-link, org-protocol-capture,
  org-protocol-open-source): Accept new-style links.
  (org-protocol-check-filename-for-protocol): Updated documentation.
  (org-protocol-parse-parameters, org-protocol-assign-parameters):
  New functions.

  This allows the use of org-protocol on KDE 5 and makes org-protocol
  links more URI-like. New-style links are of the form:
  org-protocol://store-link?title=TITLE=URL

* testing/lisp/test-org-protocol.el: New file.
---
 lisp/org-protocol.el  | 204 +++---
 testing/lisp/test-org-protocol.el | 181 +
 2 files changed, 328 insertions(+), 57 deletions(-)
 create mode 100644 testing/lisp/test-org-protocol.el

diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 339f2b7..b350d40 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -49,7 +49,7 @@
 ;;   4.) Try this from the command line (adjust the URL as needed):
 ;;
 ;;   $ emacsclient \
-;; org-protocol://store-link://http:%2F%2Flocalhost%2Findex.html/The%20title
+;; org-protocol://store-link?url=http:%2F%2Flocalhost%2Findex.html=The%20title
 ;;
 ;;   5.) Optionally add custom sub-protocols and handlers:
 ;;
@@ -60,7 +60,7 @@
 ;;
 ;;   A "sub-protocol" will be found in URLs like this:
 ;;
-;;   org-protocol://sub-protocol://data
+;;   org-protocol://sub-protocol?key=val=val2
 ;;
 ;; If it works, you can now setup other applications for using this feature.
 ;;
@@ -94,20 +94,20 @@
 ;; You may use the same bookmark URL for all those standard handlers and just
 ;; adjust the sub-protocol used:
 ;;
-;; location.href='org-protocol://sub-protocol://'+
-;;   encodeURIComponent(location.href)+'/'+
-;;   encodeURIComponent(document.title)+'/'+
+;; location.href='org-protocol://sub-protocol?url='+
+;;   encodeURIComponent(location.href)+'='+
+;;   encodeURIComponent(document.title)+'='+
 ;;   encodeURIComponent(window.getSelection())
 ;;
 ;; The handler for the sub-protocol \"capture\" detects an optional template
 ;; char that, if present, triggers the use of a special template.
 ;; Example:
 ;;
-;; 

Re: [O] [PATCH] org-protocol: Allow key=val=value2-style URLs

[Aaron Ecay]

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 ‘new-style’ arguments from these functions.
I don’t have any clever ideas to solve this, and it’s not an objection
to your patch – just something I thought I’d mention in case 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=val=val2-style URLs
> 
> * 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 “New function” or “Accept new-style links”.  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’ll admit I don’t understand Changelog conventions and
think they are a pointless relic, so YMMV.

[...]

>  
> +(defun org-protocol-parse-parameters (info new-style  default-order 
> 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’t think our helper functions need to bother with the
unhexify != t case.  Similarly, I would not have a separator argument, but
use the value of ‘org-protocol-data-separator’ directly.  In 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?

[...]
  
>  (defun org-protocol-check-filename-for-protocol (fname restoffiles client)
>[...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= (match-string 1 fname) "?")))

As a way to encourage users to move to new-style links, should we add a
warning if new-style = nil?

Thanks again for the patch,

-- 
Aaron Ecay

[O] [PATCH] org-protocol: Allow key=val=value2-style URLs

[Sacha Chua]

Aaron Ecay  writes:

Hello, Aaron, Rasmus, all!

> better solution IMO would be to make org-protocol links valid urls in
> another way, using the query string format:
> org-protocol://store-link?url=[...]=[...]

Aaron: Great point! I've changed my code to support this style of
org-protocol link, and I think the tests I've added to
test-org-protocol.el double-check that old links are still supported.
I've added an extra argument to the functions defined in
org-protocol-protocol-alist and org-protocol-protocol-alist-default, but
I have a condition-case around the funcall so that old functions should
continue to work. I've updated the documentation to encourage new-style
links. What do you think of this patch? I've changed the subject to
reflect the new focus.

Rasmus: that means fiddling with ports is no longer needed, yay. I've
also added the test dependency and lexical binding cookie to
test-org-protocol, as you suggested. Since the missing-test-dependency
signal means that the test isn't run as part of make test, is there
anything I should do to get it to be included in automated tests, or is
it fine leaving it as is?

Sacha

>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=val=val2-style URLs

* 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.
  (org-protocol-store-link): Accept new-style links like
  org-protocol://store-link?title=TITLE=URL
  (org-protocol-capture): Accept new-style links like
  org-protocol://capture?title=TITLE=URL=x=BODY
  (org-protocol-do-capture): Update to accept new-style links.
  (org-protocol-open-source): Accept new-style links like
  org-protocol://open-source?url=URL
  (org-protocol-check-filename-for-protocol): Updated documentation.

  This allows the use of org-protocol on KDE 5 and makes org-protocol
  links more URI-like.

* testing/lisp/test-org-protocol.el: New file.
---
 lisp/org-protocol.el  | 194 +++---
 testing/lisp/test-org-protocol.el | 170 +
 2 files changed, 307 insertions(+), 57 deletions(-)
 create mode 100644 testing/lisp/test-org-protocol.el

diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 339f2b7..7f301e4 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -49,7 +49,7 @@
 ;;   4.) Try this from the command line (adjust the URL as needed):
 ;;
 ;;   $ emacsclient \
-;; org-protocol://store-link://http:%2F%2Flocalhost%2Findex.html/The%20title
+;; org-protocol://store-link?url=http:%2F%2Flocalhost%2Findex.html=The%20title
 ;;
 ;;   5.) Optionally add custom sub-protocols and handlers:
 ;;
@@ -60,7 +60,7 @@
 ;;
 ;;   A "sub-protocol" will be found in URLs like this:
 ;;
-;;   org-protocol://sub-protocol://data
+;;   org-protocol://sub-protocol?key=val=val2
 ;;
 ;; If it works, you can now setup other applications for using this feature.
 ;;
@@ -94,20 +94,20 @@
 ;; You may use the same bookmark URL for all those standard handlers and just
 ;; adjust the sub-protocol used:
 ;;
-;; location.href='org-protocol://sub-protocol://'+
-;;   encodeURIComponent(location.href)+'/'+
-;;   encodeURIComponent(document.title)+'/'+
+;; location.href='org-protocol://sub-protocol?url='+
+;;   encodeURIComponent(location.href)+'='+
+;;   encodeURIComponent(document.title)+'='+
 ;;   encodeURIComponent(window.getSelection())
 ;;
 ;; The handler for the sub-protocol \"capture\" detects an optional template
 ;; char that, if present, triggers the use of a special template.
 ;; Example:
 ;;
-;; location.href='org-protocol://sub-protocol://x/'+ ...
+;; location.href='org-protocol://capture?template=x'+ ...
 ;;
-;;  use template ?x.
+;;  uses template ?x.
 ;;
-;; Note, that using double slashes is optional from org-protocol.el's point of
+;; Note that using double slashes is optional from org-protocol.el's point of
 ;; view because emacsclient squashes the slashes to one.
 ;;
 ;;
@@ -233,19 +233,21 @@ protocol - protocol to detect in a filename without trailing colon and slashes.
`org-protocol-the-protocol'.  Double and triple slashes are compressed
to one by emacsclient.
 
-function - function that handles requests with protocol and takes exactly one
-   argument: the filename with all protocols stripped.  If the function
+function - function that handles requests with protocol and takes two
+   arguments: the filename with all protocols stripped, and a new-style
+   argument that indicates whether new-style arguments (key=val=val2)
+   or old-style arguments (val/val2) were used.