From mboxrd@z Thu Jan 1 00:00:00 1970 From: Richard Kim Subject: Re: patch to add new link type "infoi" that leverages Info-index command Date: Sun, 9 Nov 2014 19:58:40 -0800 Message-ID: References: <87k338qii9.fsf@nicolasgoaziou.fr> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=001a11c34cf08ca8980507792dc3 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:58342) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xng7o-0006Ir-9R for Emacs-orgmode@gnu.org; Sun, 09 Nov 2014 22:58:46 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xng7m-0005jm-KN for Emacs-orgmode@gnu.org; Sun, 09 Nov 2014 22:58:44 -0500 Received: from mail-wi0-x22e.google.com ([2a00:1450:400c:c05::22e]:54460) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xng7m-0005jb-89 for Emacs-orgmode@gnu.org; Sun, 09 Nov 2014 22:58:42 -0500 Received: by mail-wi0-f174.google.com with SMTP id d1so9085876wiv.1 for ; Sun, 09 Nov 2014 19:58:40 -0800 (PST) In-Reply-To: <87k338qii9.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-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: Richard Kim , Emacs-orgmode@gnu.org --001a11c34cf08ca8980507792dc3 Content-Type: multipart/alternative; boundary=001a11c34cf08ca8940507792dc1 --001a11c34cf08ca8940507792dc1 Content-Type: text/plain; charset=UTF-8 Nicolas, Thanks for your feedback. I agree that using the same link type is better. Hence I took an alternate approach as detailed in the attached patch. Enhanced org-info-follow-link to attempt index lookup if node lookup fails. Following is my check in message found in the attached patch: Enhanced org-info-follow-link to attempt index lookup if node lookup fails. Info index is almost always finer grain than info nodes. For example with this change, [[info:libc#close]] brings up not only "(libc)Opening and Closing Files" info node, but also place the cursor on the line that documents "close" function within the node. This is done by looking up "close"in the index upon failing to find a node named "close". Hence one can now link function, variable and other names that are in the index rather than being limited to info node names. Typically there are far more index items than there are node names. For example libc manual has about 700 nodes, but over 4000 concept, type, function, and variables index items. On 6 November 2014 10:41, Nicolas Goaziou wrote: > Hello, > > Richard Kim writes: > > > A patch is provided below which implements a new link type called "infoi" > > as a complement to "info" type that exist already. > > Thanks for your patch. > > > Why new link type? Because info index is almost always finer grain > > than info nodes. For example [[infoi:libc#close]] brings up not only > > "(libc)Opening and Closing Files" info node, but also place the cursor > > on the line that documents "open" function within the node. Hence it > > is more useful to link function, variable and other names that are in > > the index. Most info documents have very rich indexes. > > OK. I would have preferred to merge both features into the same link > type, but I see no elegant syntax to distinguish between a node and an > index search. > > > I am not familiar with org coding style, so I share this patch to > > present the idea rather than as a finished patch that can be checked > > in. I assume that name changes and other changes will be needed prior > > to being acceptable for check in assuming that the idea is ok. > > The idea is OK. Some comments follow. > > > org-info.el: Add new link type "infoi" that looks up info file index. > > No full stop at the end of the summary. > > > * lisp/org-info.el (org-info-index-open): New function to implement > > new link type named "infoi". > > "New function" is enough here. > > You can describe the motivation behind the patch farther in the commit > message, after another blank line. > > > +;;; info-index > > + > > +(org-add-link-type "infoi" 'org-info-index-open) > > + > > +(declare-function Info-index "info" (topic)) > > + > > +(defun org-info-index-open (name) > > + "Follow an Info file and look up index item specified by NAME." > > + (if (or (string-match "\\(.*\\)[#:]:?\\(.*\\)" name) > > + (string-match "\\(.*\\)" name)) > > + (let ((nodename (match-string 2 name))) > > + (require 'info) > > + (Info-find-node (match-string 1 name) "Top") > > + (if nodename ; If there isn't a node, choose "Top" > > + (Info-index nodename))) > > Prefer `when' over one-armed `if'. > > > + (message "Could not open: %s" name))) > > This introduce some code duplication in "org-info.el". Could you factor > it out? > > > Regards, > > -- > Nicolas Goaziou > --001a11c34cf08ca8940507792dc1 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Nicolas,

Thanks for your feedback.=C2=A0 = I agree that using the same link type is better.
Hence I took an a= lternate approach as detailed in the attached patch.
=C2=A0Enhanced org-= info-follow-link to attempt index lookup if node lookup fails.
Fol= lowing is my check in message found in the attached patch:
=C2=A0= =C2=A0
=C2=A0=C2=A0=C2=A0 Enhanced org-info-follow-link to attempt inde= x lookup if node lookup fails.
=C2=A0=C2=A0=C2=A0
=C2=A0=C2=A0=C2=A0= Info index is almost always finer grain than info nodes.=C2=A0 For example=
=C2=A0=C2=A0=C2=A0 with this change, [[info:libc#close]] brings up not = only
=C2=A0=C2=A0=C2=A0 "(libc)Opening and Closing Files" info= node, but also place the cursor
=C2=A0=C2=A0=C2=A0 on the line that doc= uments "close" function within the node.=C2=A0 This is
=C2=A0= =C2=A0=C2=A0 done by looking up "close"in the index upon failing = to find a node
=C2=A0=C2=A0=C2=A0 named "close".=C2=A0 Hence o= ne can now link function, variable and other
=C2=A0=C2=A0=C2=A0 names th= at are in the index rather than being limited to info node
=C2=A0=C2=A0= =C2=A0 names.=C2=A0 Typically there are far more index items than there are= node
=C2=A0=C2=A0=C2=A0 names.=C2=A0 For example libc manual has about = 700 nodes, but over
=C2=A0=C2=A0=C2=A0 4000 concept, type, function, and= variables index items.


<= br>
On 6 November 2014 10:41, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:
Hello,

Richard Kim <emacs18@gmail.com&= gt; writes:

> A patch is provided below which implements a new link type called &quo= t;infoi"
> as a complement to "info" type that exist already.

Thanks for your patch.

> Why new link type?=C2=A0 Because info index is almost always finer gra= in
> than info nodes.=C2=A0 For example [[infoi:libc#close]] brings up not = only
> "(libc)Opening and Closing Files" info node, but also place = the cursor
> on the line that documents "open" function within the node.= =C2=A0 Hence it
> is more useful to link function, variable and other names that are in<= br> > the index.=C2=A0 Most info documents have very rich indexes.

OK. I would have preferred to merge both features into the same link=
type, but I see no elegant syntax to distinguish between a node and an
index search.

> I am not familiar with org coding style, so I share this patch to
> present the idea rather than as a finished patch that can be checked > in.=C2=A0 I assume that name changes and other changes will be needed = prior
> to being acceptable for check in assuming that the idea is ok.

The idea is OK. Some comments follow.

> org-info.el: Add new link type "infoi" that looks up info fi= le index.

No full stop at the end of the summary.

> * lisp/org-info.el (org-info-index-open): New function to implement > new link type named "infoi".

"New function" is enough here.

You can describe the motivation behind the patch farther in the commit
message, after another blank line.

> +;;; info-index
> +
> +(org-add-link-type "infoi" 'org-info-index-open)
> +
> +(declare-function Info-index "info" (topic))
> +
> +(defun org-info-index-open (name)
> +=C2=A0 "Follow an Info file and look up index item specified by = NAME."
> +=C2=A0 (if (or (string-match "\\(.*\\)[#:]:?\\(.*\\)" name)=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (string-match "\\(.*\\)"= name))
> +=C2=A0 =C2=A0 =C2=A0 (let ((nodename (match-string 2 name)))
> +=C2=A0 =C2=A0 (require 'info)
> +=C2=A0 =C2=A0 (Info-find-node (match-string 1 name) "Top")<= br> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 (if nodename ; If there isn't a node,= choose "Top"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (Info-index nodename)))

Prefer `when' over one-armed `if'.

> +=C2=A0 =C2=A0 (message "Could not open: %s" name)))

This introduce some code duplication in "org-info.el". Cou= ld you factor
it out?


Regards,

--
Nicolas Goaziou

--001a11c34cf08ca8940507792dc1-- --001a11c34cf08ca8980507792dc3 Content-Type: text/x-patch; charset=US-ASCII; name="0001-Enhanced-org-info-follow-link-to-attempt-index-looku.patch" Content-Disposition: attachment; filename="0001-Enhanced-org-info-follow-link-to-attempt-index-looku.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_i2bas1f20 RnJvbSBjODUwODA0MjY3ZjM0M2QwMjBmOTE0OTllMThjYmRlODRhM2ZiODk3IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBLaW0gPGVtYWNzMThAZ21haWwuY29tPgpEYXRlOiBTdW4sIDkg Tm92IDIwMTQgMTk6NDM6MTggLTA4MDAKU3ViamVjdDogW1BBVENIXSBFbmhhbmNlZCBvcmctaW5m by1mb2xsb3ctbGluayB0byBhdHRlbXB0IGluZGV4IGxvb2t1cCBpZiBub2RlCiBsb29rdXAgZmFp bHMuCgpJbmZvIGluZGV4IGlzIGFsbW9zdCBhbHdheXMgZmluZXIgZ3JhaW4gdGhhbiBpbmZvIG5v ZGVzLiAgRm9yIGV4YW1wbGUKd2l0aCB0aGlzIGNoYW5nZSwgW1tpbmZvOmxpYmMjY2xvc2VdXSBi cmluZ3MgdXAgbm90IG9ubHkKIihsaWJjKU9wZW5pbmcgYW5kIENsb3NpbmcgRmlsZXMiIGluZm8g bm9kZSwgYnV0IGFsc28gcGxhY2UgdGhlIGN1cnNvcgpvbiB0aGUgbGluZSB0aGF0IGRvY3VtZW50 cyAiY2xvc2UiIGZ1bmN0aW9uIHdpdGhpbiB0aGUgbm9kZS4gIFRoaXMgaXMKZG9uZSBieSBsb29r aW5nIHVwICJjbG9zZSJpbiB0aGUgaW5kZXggdXBvbiBmYWlsaW5nIHRvIGZpbmQgYSBub2RlCm5h bWVkICJjbG9zZSIuICBIZW5jZSBvbmUgY2FuIG5vdyBsaW5rIGZ1bmN0aW9uLCB2YXJpYWJsZSBh bmQgb3RoZXIKbmFtZXMgdGhhdCBhcmUgaW4gdGhlIGluZGV4IHJhdGhlciB0aGFuIGJlaW5nIGxp bWl0ZWQgdG8gaW5mbyBub2RlCm5hbWVzLiAgVHlwaWNhbGx5IHRoZXJlIGFyZSBmYXIgbW9yZSBp bmRleCBpdGVtcyB0aGFuIHRoZXJlIGFyZSBub2RlCm5hbWVzLiAgRm9yIGV4YW1wbGUgbGliYyBt YW51YWwgaGFzIGFib3V0IDcwMCBub2RlcywgYnV0IG92ZXIKNDAwMCBjb25jZXB0LCB0eXBlLCBm dW5jdGlvbiwgYW5kIHZhcmlhYmxlcyBpbmRleCBpdGVtcy4KLS0tCiBkb2Mvb3JnLnRleGkgICAg IHwgIDIgKy0KIGxpc3Avb3JnLWluZm8uZWwgfCAxNSArKysrKysrKysrKy0tLS0KIDIgZmlsZXMg Y2hhbmdlZCwgMTIgaW5zZXJ0aW9ucygrKSwgNSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9k b2Mvb3JnLnRleGkgYi9kb2Mvb3JnLnRleGkKaW5kZXggZGIxNDkwYS4uMDhlMDcxZCAxMDA2NDQK LS0tIGEvZG9jL29yZy50ZXhpCisrKyBiL2RvYy9vcmcudGV4aQpAQCAtMzU3NSw3ICszNTc1LDcg QEAgZ251czpncm91cCAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgQHJ7R251cyBncm91 cCBsaW5rfQogZ251czpncm91cCNpZCAgICAgICAgICAgICAgICAgICAgICAgICAgICAgQHJ7R251 cyBhcnRpY2xlIGxpbmt9CiBiYmRiOlIuKlN0YWxsbWFuICAgICAgICAgICAgICAgICAgICAgICAg ICBAcntCQkRCIGxpbmsgKHdpdGggcmVnZXhwKX0KIGlyYzovaXJjLmNvbS8jZW1hY3MvYm9iICAg ICAgICAgICAgICAgICAgIEBye0lSQyBsaW5rfQotaW5mbzpvcmcjRXh0ZXJuYWwgbGlua3MgICAg ICAgICAgICAgICAgICAgQHJ7SW5mbyBub2RlIGxpbmt9CitpbmZvOm9yZyNFeHRlcm5hbCBsaW5r cyAgICAgICAgICAgICAgICAgICBAcntJbmZvIG5vZGUgb3IgaW5kZXggbGlua30KIHNoZWxsOmxz ICoub3JnICAgICAgICAgICAgICAgICAgICAgICAgICAgIEBye0Egc2hlbGwgY29tbWFuZH0KIGVs aXNwOm9yZy1hZ2VuZGEgICAgICAgICAgICAgICAgICAgICAgICAgIEBye0ludGVyYWN0aXZlIEVs aXNwIGNvbW1hbmR9CiBlbGlzcDooZmluZC1maWxlLW90aGVyLWZyYW1lICJFbGlzcC5vcmciKSBA cntFbGlzcCBmb3JtIHRvIGV2YWx1YXRlfQpkaWZmIC0tZ2l0IGEvbGlzcC9vcmctaW5mby5lbCBi L2xpc3Avb3JnLWluZm8uZWwKaW5kZXggOGEyZDcxNy4uN2RlYWEzMyAxMDA2NDQKLS0tIGEvbGlz cC9vcmctaW5mby5lbAorKysgYi9saXNwL29yZy1pbmZvLmVsCkBAIC02NywxMSArNjcsMTggQEAK ICAgIkZvbGxvdyBhbiBJbmZvIGZpbGUgYW5kIG5vZGUgbGluayBzcGVjaWZpZWQgYnkgTkFNRS4i CiAgIChpZiAob3IgKHN0cmluZy1tYXRjaCAiXFwoLipcXClbIzpdOj9cXCguKlxcKSIgbmFtZSkK ICAgICAgICAgICAoc3RyaW5nLW1hdGNoICJcXCguKlxcKSIgbmFtZSkpCi0gICAgICAocHJvZ24K KyAgICAgIChsZXQgKChmaWxlbmFtZSAobWF0Y2gtc3RyaW5nIDEgbmFtZSkpCisJICAgIChub2Rl bmFtZS1vci1pbmRleCAob3IgKG1hdGNoLXN0cmluZyAyIG5hbWUpICJUb3AiKSkpCiAJKHJlcXVp cmUgJ2luZm8pCi0gICAgICAgIChpZiAobWF0Y2gtc3RyaW5nIDIgbmFtZSkgOyBJZiB0aGVyZSBp c24ndCBhIG5vZGUsIGNob29zZSAiVG9wIgotICAgICAgICAgICAgKEluZm8tZmluZC1ub2RlICht YXRjaC1zdHJpbmcgMSBuYW1lKSAobWF0Y2gtc3RyaW5nIDIgbmFtZSkpCi0gICAgICAgICAgKElu Zm8tZmluZC1ub2RlIChtYXRjaC1zdHJpbmcgMSBuYW1lKSAiVG9wIikpKQorCTs7IElmIG5vZGVu YW1lLW9yLWluZGV4IGlzIGludmFsaWQgbm9kZSBuYW1lLCB0aGVuIGxvb2sgaXQKKwk7OyB1cCBp biB0aGUgaW5kZXguCisJKGNvbmRpdGlvbi1jYXNlIG5pbAorCSAgICAoSW5mby1maW5kLW5vZGUg ZmlsZW5hbWUgbm9kZW5hbWUtb3ItaW5kZXgpCisJICAodXNlci1lcnJvciAoSW5mby1maW5kLW5v ZGUgZmlsZW5hbWUgIlRvcCIpCisJCSAgICAgIChjb25kaXRpb24tY2FzZSBuaWwKKwkJCSAgKElu Zm8taW5kZXggbm9kZW5hbWUtb3ItaW5kZXgpCisJCQkodXNlci1lcnJvciAobWVzc2FnZSAiQ291 bGQgbm90IGZpbmQgJyVzJyBub2RlIG9yIGluZGV4IGVudHJ5LiIgCisJCQkJCSAgICAgbm9kZW5h bWUtb3ItaW5kZXgpKSkpKSkKICAgICAobWVzc2FnZSAiQ291bGQgbm90IG9wZW46ICVzIiBuYW1l KSkpCiAKIChwcm92aWRlICdvcmctaW5mbykKLS0gCjEuOS4xCgo= --001a11c34cf08ca8980507792dc3--