From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Andreas Politz Newsgroups: gmane.emacs.bugs Subject: bug#14029: 24.2.50; [PATCH] imenu problems with special elements Date: Sat, 23 Mar 2013 16:54:58 +0100 Message-ID: <878v5ew1pp.fsf@fh-trier.de> References: <87hak4gram.fsf@fh-trier.de> <5ABDF35DFCF64DEEAA37AFAE5048578D@us.oracle.com> <87ip4iwfoe.fsf@fh-trier.de> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1364054172 29846 80.91.229.3 (23 Mar 2013 15:56:12 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 23 Mar 2013 15:56:12 +0000 (UTC) To: 14029@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Mar 23 16:56:34 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 1UJQo6-00057x-DP for geb-bug-gnu-emacs@m.gmane.org; Sat, 23 Mar 2013 16:56:34 +0100 Original-Received: from localhost ([::1]:51244 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UJQni-0006Og-Vh for geb-bug-gnu-emacs@m.gmane.org; Sat, 23 Mar 2013 11:56:10 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:48831) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UJQnd-0006NU-Fm for bug-gnu-emacs@gnu.org; Sat, 23 Mar 2013 11:56:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UJQnX-0001cU-Es for bug-gnu-emacs@gnu.org; Sat, 23 Mar 2013 11:56:05 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:38268) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UJQnX-0001cQ-AW for bug-gnu-emacs@gnu.org; Sat, 23 Mar 2013 11:55:59 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1UJQpV-0006sj-Kk for bug-gnu-emacs@gnu.org; Sat, 23 Mar 2013 11:58:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Andreas Politz Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 23 Mar 2013 15:58:01 +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 X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Original-Received: via spool by submit@debbugs.gnu.org id=B.136405425126394 (code B ref -1); Sat, 23 Mar 2013 15:58:01 +0000 Original-Received: (at submit) by debbugs.gnu.org; 23 Mar 2013 15:57:31 +0000 Original-Received: from localhost ([127.0.0.1]:42377 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1UJQp0-0006rc-NE for submit@debbugs.gnu.org; Sat, 23 Mar 2013 11:57:31 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:37335) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1UJQox-0006rQ-SJ for submit@debbugs.gnu.org; Sat, 23 Mar 2013 11:57:29 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UJQmw-0001Q3-FB for submit@debbugs.gnu.org; Sat, 23 Mar 2013 11:55:25 -0400 Original-Received: from lists.gnu.org ([208.118.235.17]:60714) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UJQmw-0001Pz-BT for submit@debbugs.gnu.org; Sat, 23 Mar 2013 11:55:22 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:48688) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UJQmt-0006JX-9q for bug-gnu-emacs@gnu.org; Sat, 23 Mar 2013 11:55:22 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UJQmq-0001Oz-3O for bug-gnu-emacs@gnu.org; Sat, 23 Mar 2013 11:55:19 -0400 Original-Received: from gateway-b.fh-trier.de ([143.93.54.182]:44608) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UJQmp-0001OD-Nj for bug-gnu-emacs@gnu.org; Sat, 23 Mar 2013 11:55:16 -0400 X-Virus-Scanned: by Amavisd-new + McAfee uvscan + ClamAV [Rechenzentrum Hochschule Trier] Original-Received: from luca (dslb-088-068-069-031.pools.arcor-ip.net [88.68.69.31]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: politza) by gateway-b.fh-trier.de (Postfix) with ESMTPSA id BA75D17B422 for ; Sat, 23 Mar 2013 16:54:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha1; c=simple/simple; d=fh-trier.de; s=default; t=1364054099; bh=lD0KvptJyltjwSizXhdlGKGmY48=; h=From:To:Subject:References:Date:In-Reply-To:Message-ID: MIME-Version:Content-Type; b=IvDmQqhdHQCtwcAKqBdwiPpmeA6ZqMNtzH6fpt9ha2mg6XuuFvlemnQQN8lpANfJB 8wjj1FBGE/Z/0y2bnZqJACBJdUSAZrv6FEgERi9yChkxTZHmlDnQzUNGAsP+NdcLJI CWlRdZKnjm1lhrBAZfJ62X414rfdTwH2E9Lck5Bk= Original-Received: from localhost ([127.0.0.1] helo=luca) by luca with esmtp (Exim 4.72) (envelope-from ) id 1UJQmY-0005CD-Dh for bug-gnu-emacs@gnu.org; Sat, 23 Mar 2013 16:54:58 +0100 In-Reply-To: <87ip4iwfoe.fsf@fh-trier.de> (Andreas Politz's message of "Sat, 23 Mar 2013 11:53:21 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x 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:72833 Archived-At: Andreas Politz writes: > "Drew Adams" writes: > >> 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'?) > > Yes in `imenu--in-alist'. It used to be >1, but one is already fixed in > bzr. > >> >>> 2. The advertised calling convention is different from the >>> actual one in `imenu'. >> >> How so? What difference do you see, where? >> > > The last change in `imenu' about cdddr. > >>> 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. > > That's right. > >> >>> ! (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 the documentation states FUNCTION, then it should be a function. > > Let's recap. The three types are: > > (INDEX-NAME . INDEX-POSITION) > (INDEX-NAME INDEX-POSITION FUNCTION ARGUMENTS...) > (MENU-TITLE SUB-ALIST) > > First, I think the documentation is incorrect, and the last one should read > > (MENU-TITLE . SUB-ALIST) > > since SUB-ALIST is supposed to be the cdr of the list (see > e.g. imenu--split-submenus) > > So (listp (cdr item)) would exclude the first type and we are > left with > > (INDEX-POSITION FUNCTION ARGUMENTS...) and > SUB-ALIST = (ITEM ITEM ...) > > and (cadr item) is either a function, an item or nil. I think > that INDEX-NAME and MENU-TITLE (the car of a possible item) are > supposed to be strings, so this should work. > > >> >>> ! (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? >> > > No, it may contain subalists. > >>> ! (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). > > That, and you probably always used the mouse menu. > > A