From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Drew Adams" Newsgroups: gmane.emacs.bugs Subject: bug#14029: 24.2.50; [PATCH] imenu problems with special elements Date: Thu, 21 Mar 2013 22:12:34 -0700 Message-ID: <5ABDF35DFCF64DEEAA37AFAE5048578D@us.oracle.com> References: <87hak4gram.fsf@fh-trier.de> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Trace: ger.gmane.org 1363929194 28714 80.91.229.3 (22 Mar 2013 05:13:14 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 22 Mar 2013 05:13:14 +0000 (UTC) To: "'Andreas Politz'" , <14029@debbugs.gnu.org> Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Mar 22 06:13:40 2013 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1UIuIM-0001DE-Qp for geb-bug-gnu-emacs@m.gmane.org; Fri, 22 Mar 2013 06:13:39 +0100 Original-Received: from localhost ([::1]:48994 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIuHz-0006ZP-8o for geb-bug-gnu-emacs@m.gmane.org; Fri, 22 Mar 2013 01:13:15 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:33095) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIuHu-0006Y3-NT for bug-gnu-emacs@gnu.org; Fri, 22 Mar 2013 01:13:12 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UIuHs-0001Bh-KP for bug-gnu-emacs@gnu.org; Fri, 22 Mar 2013 01:13:10 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:35744) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIuHs-0001BW-IC for bug-gnu-emacs@gnu.org; Fri, 22 Mar 2013 01:13:08 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1UIuJi-0002cA-Jr for bug-gnu-emacs@gnu.org; Fri, 22 Mar 2013 01:15:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "Drew Adams" Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 22 Mar 2013 05:15:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 14029 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 14029-submit@debbugs.gnu.org id=B14029.136392928010003 (code B ref 14029); Fri, 22 Mar 2013 05:15:02 +0000 Original-Received: (at 14029) by debbugs.gnu.org; 22 Mar 2013 05:14:40 +0000 Original-Received: from localhost ([127.0.0.1]:39853 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1UIuJL-0002bI-EH for submit@debbugs.gnu.org; Fri, 22 Mar 2013 01:14:39 -0400 Original-Received: from userp1040.oracle.com ([156.151.31.81]:23821) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1UIuJI-0002b8-0t for 14029@debbugs.gnu.org; Fri, 22 Mar 2013 01:14:37 -0400 Original-Received: from ucsinet22.oracle.com (ucsinet22.oracle.com [156.151.31.94]) by userp1040.oracle.com (Sentrion-MTA-4.3.1/Sentrion-MTA-4.3.1) with ESMTP id r2M5Cd3R031070 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 22 Mar 2013 05:12:40 GMT Original-Received: from acsmt356.oracle.com (acsmt356.oracle.com [141.146.40.156]) by ucsinet22.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id r2M5CcNp020204 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 22 Mar 2013 05:12:39 GMT Original-Received: from abhmt107.oracle.com (abhmt107.oracle.com [141.146.116.59]) by acsmt356.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id r2M5Cce3008393; Fri, 22 Mar 2013 00:12:38 -0500 Original-Received: from dradamslap1 (/10.159.248.227) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 21 Mar 2013 22:12:37 -0700 X-Mailer: Microsoft Office Outlook 11 In-Reply-To: <87hak4gram.fsf@fh-trier.de> Thread-Index: Ac4mnB1hVV/+YU70SkCJcJk1Vz3ahwAGQBng X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.6157 X-Source-IP: ucsinet22.oracle.com [156.151.31.94] X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:72783 Archived-At: Thanks for this report and fix. Neither the original code nor your patch is super clear to me, so I have some (non-rhetorical) questions below. But if someone else understands this better, feel free to ignore. > | Special elements look like this: > | (INDEX-NAME INDEX-POSITION FUNCTION ARGUMENTS...) > | Selecting a special element performs: > | (funcall FUNCTION > | INDEX-NAME INDEX-POSITION ARGUMENTS...) > > 1. At least one function does not recognize these elements, resulting > in an error. Can you be more specific? Which function? What error? Recipe to reproduce? (Maybe you are referring to the last change in your patch, for `imenu'?) > 2. The advertised calling convention is different from the > actual one in `imenu'. How so? What difference do you see, where? > 3. The `imenu--subalist-p' predicate is unsafe and incomplete. How so? > (defun imenu--subalist-p (item) > ! (and (consp (cdr item)) (listp (cadr item)) (defun imenu--subalist-p (item) > ! (and (consp item) > ! (consp (cdr item)) > ! (listp (cadr item)) (consp (cdr item)) is equivalent to (and (consp item) (consp (cdr item))), assuming ITEM has the proper form (so that cdr does not raise an error). (consp (cdr-safe item)) should do the same thing. > ! (not (eq (car (cadr item)) 'lambda)))) > ! (not (functionp (cadr item))))) Is it possible for (cadr item) to be a list and also be `functionp' and yet not have its car be `lambda'? Dunno. I was under the impression that it was impossible, but I could be wrong. If it is possible, is it better to test `functionp' here? Dunno. > ! (if (setq res (imenu--in-alist str tail)) > ! (if (and (imenu--subalist-p elt) > ! (setq res (imenu--in-alist str tail))) Why is (imenu--subalist-p elt) needed here? What error case does it prevent? Can't the code assume a properly constructed menu here, so that if TAIL is a list then it is a bottom-level element, and so it is proper to just set ALIST to nil? > ! (rest (if is-special-item (cddr index-item)))) > ! (rest (if is-special-item (cdddr index-item)))) This change looks good, but `cdddr' is in cl.el, so perhaps it is better to use (nthcdr 3 index-item). I'm only a little bit surprised that this one hasn't already been reported and fixed - there have been other bugs (e.g. #12717) related to special items. I use special items myself, but so far I have not used non-nil ARGS, so I have not encountered this one (your last change).