[PATCH] protocol: allow users to disable warning about old style links

[PATCH] protocol: allow users to disable warning about old style links

[Alex Branham]

[-- Attachment #1: Type: text/plain, Size: 1654 bytes --]

Hello,

This patch allows org mode users to disable the warning issued about old org protocol style links. This is important because sometimes (e.g. browser extensions), we don’t have easy control over the format specified. Issuing the warning is annoying because it pops up in a new window.

Thanks,
Alex

From 8bfc951308324de8e455128950193c046691f1f8 Mon Sep 17 00:00:00 2001
From: Alex Branham <branham@utexas.edu>
Date: Wed, 14 Jun 2017 14:06:18 +0200
Subject: [PATCH] protocol: allow user to disable warning about old style links

---
 lisp/org-protocol.el | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 825435674..2eb1094e9 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -271,6 +271,12 @@ This should be a single regexp string."
   :package-version '(Org . "8.0")
   :type 'string)

+(defcustom org-protocol-warn-about-old-links t
+  "If non-nil (the default), issue a warning when org protocol
+receives old style links."
+  :group 'org-protocol
+  :type 'boolean)
+
 ;;; Helper functions:

 (defun org-protocol-sanitize-uri (uri)
@@ -618,8 +624,9 @@ CLIENT is ignored."
 			     (if new-style
 				 (funcall func (org-protocol-parse-parameters
 						result new-style))
-			       (warn "Please update your Org Protocol handler \
-to deal with new-style links.")
+			       (when org-protocol-warn-about-old-links
+				 (warn "Please update your Org Protocol handler \
+to deal with new-style links."))
 			       (funcall func result))))
 		    ;; Greedy protocol handlers are responsible for
 		    ;; parsing their own filenames.
--
2.13.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-protocol-allow-user-to-disable-warning-about-old-sty.patch --]
[-- Type: text/x-diff, Size: 1348 bytes --]

From 8bfc951308324de8e455128950193c046691f1f8 Mon Sep 17 00:00:00 2001
From: Alex Branham <branham@utexas.edu>
Date: Wed, 14 Jun 2017 14:06:18 +0200
Subject: [PATCH] protocol: allow user to disable warning about old style links

---
 lisp/org-protocol.el | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 825435674..2eb1094e9 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -271,6 +271,12 @@ This should be a single regexp string."
   :package-version '(Org . "8.0")
   :type 'string)
 
+(defcustom org-protocol-warn-about-old-links t
+  "If non-nil (the default), issue a warning when org protocol
+receives old style links."
+  :group 'org-protocol
+  :type 'boolean)
+
 ;;; Helper functions:
 
 (defun org-protocol-sanitize-uri (uri)
@@ -618,8 +624,9 @@ CLIENT is ignored."
 			     (if new-style
 				 (funcall func (org-protocol-parse-parameters
 						result new-style))
-			       (warn "Please update your Org Protocol handler \
-to deal with new-style links.")
+			       (when org-protocol-warn-about-old-links
+				 (warn "Please update your Org Protocol handler \
+to deal with new-style links."))
 			       (funcall func result))))
 		    ;; Greedy protocol handlers are responsible for
 		    ;; parsing their own filenames.
-- 
2.13.1

Re: [PATCH] protocol: allow users to disable warning about old style links

[Nicolas Goaziou]

Hello,

Alex Branham <branham@utexas.edu> writes:

> This patch allows org mode users to disable the warning issued about
>old org protocol style links. This is important because sometimes (e.g.
>browser extensions), we don’t have easy control over the format
>specified.

Thank you.

Wouldn't it be better to report it upstream as a bug? I don't think
there are many Org protocols applications out there and the conversion
to the new syntax is easy.

> Issuing the warning is annoying because it pops up in a new window.

I think this variable should go in "org-compat.el", so that we can
remove it when we drop old syntax. It also need
proper :version, :package-version and :safe keywords.

Another option is to make it a simple `defvar' and have the warning
message suggesting to set it.

WDYT?

> From 8bfc951308324de8e455128950193c046691f1f8 Mon Sep 17 00:00:00 2001
> From: Alex Branham <branham@utexas.edu>
> Date: Wed, 14 Jun 2017 14:06:18 +0200
> Subject: [PATCH] protocol: allow user to disable warning about old style links
>
> ---
>  lisp/org-protocol.el | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
> index 825435674..2eb1094e9 100644
> --- a/lisp/org-protocol.el
> +++ b/lisp/org-protocol.el
> @@ -271,6 +271,12 @@ This should be a single regexp string."
>    :package-version '(Org . "8.0")
>    :type 'string)
>
> +(defcustom org-protocol-warn-about-old-links t
> +  "If non-nil (the default), issue a warning when org protocol
> +receives old style links."

  When non-nil, warn when Org protocol receives old style link.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH] protocol: allow users to disable warning about old style links

[Alex Branham]

[-- Attachment #1: Type: text/plain, Size: 4054 bytes --]

Hi, thanks for the reply.

On Fri 16 Jun 2017 at 06:13, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> Hello,
>
> Alex Branham <branham@utexas.edu> writes:
>
>> This patch allows org mode users to disable the warning issued about
>>old org protocol style links. This is important because sometimes (e.g.
>>browser extensions), we don’t have easy control over the format
>>specified.
>
> Thank you.
>
> Wouldn't it be better to report it upstream as a bug? I don't think
> there are many Org protocols applications out there and the conversion
> to the new syntax is easy.

I did report it upstream as a bug. The maintainer is (understandably, I think) hesitant to change since 1) old style links continue to be supported and 2) there’s no way for a browser extension to detect the version of org-mode to install. Since new style links aren’t supported in older org versions (like what comes shipped with Emacs), that could create issues.

>
>> Issuing the warning is annoying because it pops up in a new window.
>
> I think this variable should go in "org-compat.el", so that we can
> remove it when we drop old syntax. It also need
> proper :version, :package-version and :safe keywords.
>
> Another option is to make it a simple `defvar' and have the warning
> message suggesting to set it.

I changed it to a defvar and modified the warning. That seems simpler to me.

I wasn’t sure if defvars should go into org-compat.el (that file says it’s for compatibility issues with older Emacs versions?) or stay where it is. I’ve left it where it is for now but I can move it to org-compat.el if you want me to.

New patch attached and pasted below.

>
> WDYT?
>
>> From 8bfc951308324de8e455128950193c046691f1f8 Mon Sep 17 00:00:00 2001
>> From: Alex Branham <branham@utexas.edu>
>> Date: Wed, 14 Jun 2017 14:06:18 +0200
>> Subject: [PATCH] protocol: allow user to disable warning about old style links
>>
>> ---
>>  lisp/org-protocol.el | 11 +++++++++--
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
>> index 825435674..2eb1094e9 100644
>> --- a/lisp/org-protocol.el
>> +++ b/lisp/org-protocol.el
>> @@ -271,6 +271,12 @@ This should be a single regexp string."
>>    :package-version '(Org . "8.0")
>>    :type 'string)
>>
>> +(defcustom org-protocol-warn-about-old-links t
>> +  "If non-nil (the default), issue a warning when org protocol
>> +receives old style links."
>
>   When non-nil, warn when Org protocol receives old style link.
>
>
> Regards,

From 5d04e3a9404b2109751da6e43fba24a7cace00a8 Mon Sep 17 00:00:00 2001
From: Alex Branham <branham@utexas.edu>
Date: Fri, 16 Jun 2017 13:24:44 +0200
Subject: [PATCH] org-protocol: add option to surpress warning about old links

---
 lisp/org-protocol.el | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 825435674..531345b58 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -271,6 +271,10 @@ This should be a single regexp string."
   :package-version '(Org . "8.0")
   :type 'string)

+(defvar org-protocol-warn-about-old-links t
+  "If non-nil (the default), issue a warning when org protocol
+receives old style links.")
+
 ;;; Helper functions:

 (defun org-protocol-sanitize-uri (uri)
@@ -618,8 +622,9 @@ CLIENT is ignored."
 			     (if new-style
 				 (funcall func (org-protocol-parse-parameters
 						result new-style))
-			       (warn "Please update your Org Protocol handler \
-to deal with new-style links.")
+			       (when org-protocol-warn-about-old-links
+				 (warn "Please update your Org Protocol handler \
+to deal with new-style links.\n                 You may set `org-protocol-warn-about-old-links' to nil to surpress this message."))
 			       (funcall func result))))
 		    ;; Greedy protocol handlers are responsible for
 		    ;; parsing their own filenames.
--
2.13.1



--
J. Alexander Branham
PhD Candidate
Department of Government
University of Texas at Austin
https://www.jabranham.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-org-protocol-add-option-to-surpress-warning-about-ol.patch --]
[-- Type: text/x-diff, Size: 1398 bytes --]

From 5d04e3a9404b2109751da6e43fba24a7cace00a8 Mon Sep 17 00:00:00 2001
From: Alex Branham <branham@utexas.edu>
Date: Fri, 16 Jun 2017 13:24:44 +0200
Subject: [PATCH] org-protocol: add option to surpress warning about old links

---
 lisp/org-protocol.el | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 825435674..531345b58 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -271,6 +271,10 @@ This should be a single regexp string."
   :package-version '(Org . "8.0")
   :type 'string)
 
+(defvar org-protocol-warn-about-old-links t
+  "If non-nil (the default), issue a warning when org protocol
+receives old style links.")
+
 ;;; Helper functions:
 
 (defun org-protocol-sanitize-uri (uri)
@@ -618,8 +622,9 @@ CLIENT is ignored."
 			     (if new-style
 				 (funcall func (org-protocol-parse-parameters
 						result new-style))
-			       (warn "Please update your Org Protocol handler \
-to deal with new-style links.")
+			       (when org-protocol-warn-about-old-links
+				 (warn "Please update your Org Protocol handler \
+to deal with new-style links.\n                 You may set `org-protocol-warn-about-old-links' to nil to surpress this message."))
 			       (funcall func result))))
 		    ;; Greedy protocol handlers are responsible for
 		    ;; parsing their own filenames.
-- 
2.13.1

Re: [PATCH] protocol: allow users to disable warning about old style links

[Nicolas Goaziou]

Alex Branham <branham@utexas.edu> writes:

> I did report it upstream as a bug. The maintainer is (understandably,
> I think) hesitant to change since 1) old style links continue to be
> supported

But it will break at some point, hence the warning.

> and 2) there’s no way for a browser extension to detect the
> version of org-mode to install. Since new style links aren’t supported
> in older org versions (like what comes shipped with Emacs), that could
> create issues.

True. Note that Org shipped with Emacs is no longer supported.
Hopefully, Emacs 26.1 will ship with a brand new Org mode.

IIUC, one issue is that the developers of the extension wants to support
multiple Org versions. That's fair, but then, they could implement an
option to generate any of the two styles. It would also make the
transition easier when one of them is dropped.

> I changed it to a defvar and modified the warning. That seems simpler to me.
>
> I wasn’t sure if defvars should go into org-compat.el (that file says
> it’s for compatibility issues with older Emacs versions?) or stay
> where it is. I’ve left it where it is for now but I can move it to
> org-compat.el if you want me to.

"org-compat.el" is about compatibility in the broad sense. It should
contain any volatile code, i.e., code that can be removed in a future
major release.

This variable can go in "org-compat".

>>> +(defcustom org-protocol-warn-about-old-links t
>>> +  "If non-nil (the default), issue a warning when org protocol
>>> +receives old style links."

You forgot to update the docstring.

In any case, I think the extension could offer both styles instead of
adding this variable.

Thank you.

Regards,

Re: [PATCH] protocol: allow users to disable warning about old style links

[Tyler Smith]

On Fri, Jun 16, 2017, at 02:13 AM, Nicolas Goaziou wrote:
> 
> Wouldn't it be better to report it upstream as a bug? I don't think
> there are many Org protocols applications out there and the conversion
> to the new syntax is easy.

I'm just starting to use org-protocol, and I'm confused by this warning.
I can't find any description of the differences between the old and new
link styles in the documentation, or any pointers as to how and what I
should change to adopt the new syntax. The org manual links to
http://orgmode.org/worg/org-contrib/org-protocol.html, which seems to be
a year or more out of date.

I'm not sure who is 'upstream' in this context, and what, if anything, I
can do as an end-user to use the new style. Any suggestions?

Thanks,

Tyler

 

Re: [PATCH] protocol: allow users to disable warning about old style links

[Mario Martelli]

[-- Attachment #1.1: Type: text/plain, Size: 955 bytes --]


> I'm just starting to use org-protocol, and I'm confused by this warning.
> I can't find any description of the differences between the old and new
> link styles in the documentation, or any pointers as to how and what I
> should change to adopt the new syntax. The org manual links to
> http://orgmode.org/worg/org-contrib/org-protocol.html <http://orgmode.org/worg/org-contrib/org-protocol.html>, which seems to be
> a year or more out of date.

I have shared a proposal to move org-protocol documentation to core documentation.
It’s mainly copied together from the old one and 9.0 NEWS.

The important part regarding the new syntax could be found here:

https://github.com/mgmart/org-mode/blob/documentation/org-protocol/contrib/orgmanual.org#L7347-L7364 <https://github.com/mgmart/org-mode/blob/documentation/org-protocol/contrib/orgmanual.org#L7347-L7364>

Would that help or is there still something to add?

Kind regards
Mario

[-- Attachment #1.2: Type: text/html, Size: 5587 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3690 bytes --]

Re: [PATCH] protocol: allow users to disable warning about old style links

[Tyler Smith]

On Wed, Jun 21, 2017, at 12:01 PM, Mario Martelli wrote:
> 
> I have shared a proposal to move org-protocol documentation to core documentation.
> It’s mainly copied together from the old one and 9.0 NEWS.
> 
> The important part regarding the new syntax could be found here:
> 
> https://github.com/mgmart/org-mode/blob/documentation/org-protocol/contrib/orgmanual.org#L7347-L7364
> 
> Would that help or is there still something to add?

Thanks, Mario.

Your documentation refers to the new-style links, but the examples still
use the old style I think? My javascript is very basic, but from some
experimenting here, I think the old style links are what you've used:

    javascript:location.href='org-protocol://sub-protocol://'+
          encodeURIComponent(location.href)+'/'+
          encodeURIComponent(document.title)+'/'+
          encodeURIComponent(window.getSelection())

whereas the new-style key-value links would be:

    javascript:location.href='org-protocol://sub-protocol?url='+
          encodeURIComponent(location.href)+'&title='+
          encodeURIComponent(document.title)+'&selection='+
          encodeURIComponent(window.getSelection())

(I'm not actually sure what the name corresponding to `getSelection()`
is)

This is based on the following link style for store-link, which works
for me with Firefox 45.9 and Org 9.0 on Emacs 26.0.50:

    javascript:location.href='org-protocol://store-link?url='+encodeURIComponent(location.href)+'&title='+encodeURIComponent(document.title)

Also, I think your example for store-link should include the
document.title element?

    javascript:location.href='org-protocol://store-link://'+encodeURIComponent(location.href)

It's true that old style links are still supported, but if they produce
a warning error when you use them, it would make sense to suggest the
new style syntax as the default for use by us newbies.

Best,

Tyler

--
plantarum.ca

Re: [PATCH] protocol: allow users to disable warning about old style links

[Tyler Smith]

On Wed, Jun 21, 2017, at 12:24 PM, Tyler Smith wrote:
> 
> whereas the new-style key-value links would be:
> 
>     javascript:location.href='org-protocol://sub-protocol?url='+
>           encodeURIComponent(location.href)+'&title='+
>           encodeURIComponent(document.title)+'&selection='+
>           encodeURIComponent(window.getSelection())
> 

Aha! The new-style links are documented in the function docs. E.g., for
`org-protocol-capture`:

  javascript:location.href = 'org-protocol://capture?url='+ 
        encodeURIComponent(location.href) + '&title=' 
        encodeURIComponent(document.title) + '&body=' + 
        encodeURIComponent(window.getSelection())

It would be very helpful to have that visible in the new documentation.

Thanks,

Tyler

Re: [PATCH] protocol: allow users to disable warning about old style links

[Mario Martelli]

[-- Attachment #1.1: Type: text/plain, Size: 447 bytes --]


>  it would make sense to suggest the
> new style syntax as the default for use by us newbies.

yeah, thought about that too. At the moment it’s still a copy of the old doc. 

Got no feedback so far wether my approach would be acceptable or not. Therefore I did not put much effort into it. 

But you’re right. The examples should be new style URIs and the infobox should reference the old style URIs.

Kind regards
Mario
— 


[-- Attachment #1.2: Type: text/html, Size: 2283 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3690 bytes --]

Re: [PATCH] protocol: allow users to disable warning about old style links

[Tyler Smith]

On Wed, Jun 21, 2017, at 12:51 PM, Mario Martelli wrote:
> 
>>  it would make sense to suggest the
>> new style syntax as the default for use by us newbies.
> 
> yeah, thought about that too. At the moment it’s still a copy of the old doc. 
> 
> Got no feedback so far wether my approach would be acceptable or not. Therefore I did not put much effort into it. 
> 
> But you’re right. The examples should be new style URIs and the infobox should reference the old style URIs.

One last question/suggestion:

The default capture template didn't work for me:

    (*"w"*
         *"Default template"*
         entry
         (file+headline *"~/org/capture.org"* *"Notes"*)
         *"* %^{Title}\n\n  Source: %u, %c\n\n  %i"*
         *:empty-lines* 1)

The %u and %c bits got replaced by the actual link code (i.e.,
javascript:...). The following works as I wanted:

    ("w" "Default capture " entry
      (file "~/org/notes.org")
      "* %^{Title}
    Source: [[%:link][%:description]]
    %:initial" :empty-lines 1)

At this point I'm still a little confused, so perhaps I borked something
else up and the problem is local to my setup.

Best,

Tyler

Re: [PATCH] protocol: allow users to disable warning about old style links

[Mario Martelli]

[-- Attachment #1.1: Type: text/plain, Size: 805 bytes --]

> The default capture template didn't work for me:
> 
>    (*"w"*
>         *"Default template"*
>         entry
>         (file+headline *"~/org/capture.org <http://capture.org/>"* *"Notes"*)
>         *"* %^{Title}\n\n  Source: %u, %c\n\n  %i"*
>         *:empty-lines* 1)

There are some asterisks which do not belong there. Probably it helps to remove them.
Otherwise it’s similar to the template I’m using.

> The %u and %c bits got replaced by the actual link code (i.e.,
> javascript:...). The following works as I wanted:
> 
>    ("w" "Default capture " entry
>      (file "~/org/notes.org <http://notes.org/>")
>      "* %^{Title}
>    Source: [[%:link][%:description]]
>    %:initial" :empty-lines 1)

It it works then it’s fine :)

Kind regards
Mario
— 




[-- Attachment #1.2: Type: text/html, Size: 12231 bytes --]

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3690 bytes --]