From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: patch to add new link type "infoi" that leverages Info-index command Date: Thu, 06 Nov 2014 19:41:18 +0100 Message-ID: <87k338qii9.fsf@nicolasgoaziou.fr> References: Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:53425) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmRz8-00037v-Mh for Emacs-orgmode@gnu.org; Thu, 06 Nov 2014 13:40:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XmRyx-0005wX-Tf for Emacs-orgmode@gnu.org; Thu, 06 Nov 2014 13:40:42 -0500 Received: from relay6-d.mail.gandi.net ([2001:4b98:c:538::198]:50677) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XmRyx-0005wK-F2 for Emacs-orgmode@gnu.org; Thu, 06 Nov 2014 13:40:31 -0500 In-Reply-To: (Richard Kim's message of "Sat, 1 Nov 2014 11:23:06 -0700") 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 Cc: Emacs-orgmode@gnu.org 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