From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takaaki Ishikawa Subject: Re: [PATCH] Add a custom list in org-mac-link.el Date: Fri, 16 Jun 2017 03:38:55 +0900 Message-ID: References: <87h8ziikze.fsf@nicolasgoaziou.fr> <87wp8dgqjv.fsf@nicolasgoaziou.fr> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="f403043efc782626b7055203fbfe" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:48148) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLZfW-0001D0-9N for emacs-orgmode@gnu.org; Thu, 15 Jun 2017 14:38:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dLZfV-0007Bs-4X for emacs-orgmode@gnu.org; Thu, 15 Jun 2017 14:38:58 -0400 Received: from mail-ua0-x229.google.com ([2607:f8b0:400c:c08::229]:34421) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dLZfU-0007BE-R7 for emacs-orgmode@gnu.org; Thu, 15 Jun 2017 14:38:57 -0400 Received: by mail-ua0-x229.google.com with SMTP id m31so13421433uam.1 for ; Thu, 15 Jun 2017 11:38:56 -0700 (PDT) In-Reply-To: <87wp8dgqjv.fsf@nicolasgoaziou.fr> 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" To: Nicolas Goaziou , orgmode list --f403043efc782626b7055203fbfe Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Dear Nicolas, Thank you for your kind feedback. Please find my comments below. > +(defcustom org-mac-link-descriptors > > + `(("F" "inder" org-mac-finder-insert-selected > ,org-mac-grab-Finder-app-p) > > + ("m" "ail" org-mac-message-insert-selected > ,org-mac-grab-Mail-app-p) > > + ("d" "EVONthink Pro Office" org-mac-devonthink-item-insert-selecte= d > > + ,org-mac-grab-devonthink-app-p) > > + ("o" "utlook" org-mac-outlook-message-insert-selected > ,org-mac-grab-Outlook-app-p) > > + ("a" "ddressbook" org-mac-addressbook-insert-selected > ,org-mac-grab-Addressbook-app-p) > > + ("s" "afari" org-mac-safari-insert-frontmost-url > ,org-mac-grab-Safari-app-p) > > + ("f" "irefox" org-mac-firefox-insert-frontmost-url > ,org-mac-grab-Firefox-app-p) > > + ("v" "imperator" org-mac-vimperator-insert-frontmost-url > ,org-mac-grab-Firefox+Vimperator-p) > > + ("c" "hrome" org-mac-chrome-insert-frontmost-url > ,org-mac-grab-Chrome-app-p) > > + ("e" "evernote" org-mac-evernote-note-insert-selected > ,org-mac-grab-Evernote-app-p) > > + ("t" "ogether" org-mac-together-insert-selected > ,org-mac-grab-Together-app-p) > > + ("S" "kim" org-mac-skim-insert-page ,org-mac-grab-Skim-app-p) > > + ("A" "crobat" org-mac-acrobat-insert-page > ,org-mac-grab-Acrobat-app-p)) > > + "Descriptors to select an application." > > Could you expand the docstring a bit? What is a descriptor, what type of > data structure is it? > Updated docstring is "Alist of descriptors. Each descriptor consists of four elements to build the link grabber menu in the minibuffer. A single character shall be used for the first element to select an application, and the pair with the secon= d element represents the name of the application. The third element is a function to insert information grabbed from selected application. And the last element is a flag to indicate whether the descriptor appears in the link grabber menu." Do we need more detail? > + :tag "A list of descriptors" > > + :group 'org-mac-link' > > + :type 'symbol) > > The :type value is wrong. You need a composite type here. > What do you feel about the following composite type. :type '(alist :value-type (string string function boolean) Is it useful to let the =C2=B0descriptors' binding? We could simply replace > `descriptors' with `org-mac-link-descriptors' in the `let' body. WDYT? Yes, agree with you. I'll replace two `descriptors' in the function with `org-mac-link-descriptors' when I produce an updated patch after receiving your comments. Best, Takaaki --f403043efc782626b7055203fbfe Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Dear Nicolas,

Thank you for your kind f= eedback. Please find my comments below.

> +(defcustom org-mac-link-descriptors
> +=C2=A0 `(("F" "inder" org-mac-finder-insert-selec= ted ,org-mac-grab-Finder-app-p)
> +=C2=A0 =C2=A0 ("m" "ail" org-mac-message-insert-<= wbr>selected ,org-mac-grab-Mail-app-p)
> +=C2=A0 =C2=A0 ("d" "EVONthink Pro Office" org-mac= -devonthink-item-insert-selected
> +=C2=A0 =C2=A0 =C2=A0,org-mac-grab-devonthink-app-p)
> +=C2=A0 =C2=A0 ("o" "utlook" org-mac-outlook-messa= ge-insert-selected ,org-mac-grab-Outlook-app-p)
> +=C2=A0 =C2=A0 ("a" "ddressbook" org-mac-addressbo= ok-insert-selected ,org-mac-grab-Addressbook-app-p)
> +=C2=A0 =C2=A0 ("s" "afari" org-mac-safari-insert-= frontmost-url ,org-mac-grab-Safari-app-p)
> +=C2=A0 =C2=A0 ("f" "irefox" org-mac-firefox-inser= t-frontmost-url ,org-mac-grab-Firefox-app-p)
> +=C2=A0 =C2=A0 ("v" "imperator" org-mac-vimperator= -insert-frontmost-url ,org-mac-grab-Firefox+Vimperator-p)
> +=C2=A0 =C2=A0 ("c" "hrome" org-mac-chrome-insert-= frontmost-url ,org-mac-grab-Chrome-app-p)
> +=C2=A0 =C2=A0 ("e" "evernote" org-mac-evernote-no= te-insert-selected ,org-mac-grab-Evernote-app-p)
> +=C2=A0 =C2=A0 ("t" "ogether" org-mac-together-ins= ert-selected ,org-mac-grab-Together-app-p)
> +=C2=A0 =C2=A0 ("S" "kim" org-mac-skim-insert-page= ,org-mac-grab-Skim-app-p)
> +=C2=A0 =C2=A0 ("A" "crobat" org-mac-acrobat-inser= t-page ,org-mac-grab-Acrobat-app-p))
> +=C2=A0 "Descriptors to select an application."

Could you expand the docstring a bit? What is a descriptor, what type of data structure is it?

Updated docstring= is

=C2=A0 "Alist of descriptors. Each d= escriptor consists of four elements to build
the link grabber men= u in the minibuffer. A single character shall be used
for the fir= st element to select an application, and the pair with the second
element represents the name of the application. The third element is a=C2= =A0
function to insert information grabbed from selected applicat= ion. And the last
element is a flag to indicate whether the descr= iptor appears in the link
grabber menu."
Do we need more detail?

> +=C2=A0 :tag "A list of descriptors"
> +=C2=A0 :group 'org-mac-link'
> +=C2=A0 :type 'symbol)

The :type value is wrong. You need a composite type here.
<= div>
What do you feel about the following composite type.

:type '(alist :value-type (string string function= boolean)


Is it useful to let the =C2=B0descriptors' binding? We could simply rep= lace
`descriptors' with `org-mac-link-descriptors' in the `let' body= . WDYT?

Yes, agree with you. I'll repla= ce two `descriptors' in the function with `org-mac-link-descriptors'= ; when I produce an updated patch after receiving your comments.
=
Best,
Takaaki=C2=A0

--f403043efc782626b7055203fbfe--