From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31944 invoked by alias); 9 May 2013 23:45:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 31928 invoked by uid 89); 9 May 2013 23:45:56 -0000 X-Spam-SWARE-Status: No, score=-4.5 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.1 Received: from mail-vc0-f176.google.com (HELO mail-vc0-f176.google.com) (209.85.220.176) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 09 May 2013 23:45:55 +0000 Received: by mail-vc0-f176.google.com with SMTP id ib11so3153500vcb.7 for ; Thu, 09 May 2013 16:45:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:x-gm-message-state; bh=B7OJxjWzp8y91hbqaDNBe6KFq5pPnm5y7aHeYIVJIHE=; b=gUfpFB+jXPZx64AWEhtW4g5upSaZeNACM0nHcMT9509Tk3cXN9Kwep2tjaqSNJ4NEJ /WoK1yd7YycAlyYhdHrEhiXDPMEPOBfCANW2xsZl7++FNeTZ6tp3Jud2FHkn/AhciNi0 lKCQdiOA4VFjzb5kWmjM+nar43N5OecOtMM0dW2K0E5XIq31DAKcJVKozVbkUjcWrtxj FYa2EBwfnNyff75Xm8Eg4i5qraINmBp8v3iLpPc8neG1swKKI4c4jYeVDwReoVQRkhYf Vn35EPocMX94a51pNpYK9BqhGV57g/TRgizlL7Lgem0Cdka5invCymLjy0aW4/rCQi8c ZQNw== MIME-Version: 1.0 X-Received: by 10.52.70.206 with SMTP id o14mr8314172vdu.63.1368143153351; Thu, 09 May 2013 16:45:53 -0700 (PDT) Received: by 10.220.229.138 with HTTP; Thu, 9 May 2013 16:45:53 -0700 (PDT) In-Reply-To: <51895A2F.8000504@redhat.com> References: <51895A2F.8000504@redhat.com> Date: Thu, 09 May 2013 23:45:00 -0000 Message-ID: Subject: Re: [RFC] Cleanup for make_source_files_completion_list From: Doug Evans To: Keith Seitz Cc: "gdb-patches@sourceware.org ml" Content-Type: text/plain; charset=ISO-8859-1 X-Gm-Message-State: ALoCoQmdWd1sFtphnfze7NxQogsGoUi8vyo8CmA371oe2FCsgDQ8EHt1OXKOHAinFL6NGYXywQDAQUXnGvsQkH6Sv3Dbw1Me8QMhAUa7OcyxpHagL/VcvkBm33cIFVK04lUIXYL5LUZVeyH9s/ZX3WdkWrelNmIthjr+htd9cqwsnxN+PAdPI7pBIb5Kbwj1ebbY5j5LIH/qGkcr4pZltygRHH+/RPV0DQ== X-SW-Source: 2013-05/txt/msg00365.txt.bz2 On Tue, May 7, 2013 at 12:46 PM, Keith Seitz wrote: > Hi, > > make_source_files_completion_list is only ever called with the first > parameter twice, e.g., > > fn_list = make_source_files_completion_list (text, text); > > Is there any reason to not whack the second parameter as the following patch > does? I looked into this a bit. All make_foo_completion_list functions take "text" and "word". And you can see how they're used in, e.g., completion_list_add_name and add_filename_to_list. [grep for " + 5" in symtab.c. Heh.] The caller of make_source_files_completion_list explains the decision to pass "text" for "word" here: /* If we only have file names as possible completion, we should bring them in sync with what rl_complete expects. The problem is that if the user types "break /foo/b TAB", and the possible completions are "/foo/bar" and "/foo/baz" rl_complete expects us to return "bar" and "baz", without the leading directories, as possible completions, because `word' starts at the "b". But we ignore the value of `word' when we call make_source_files_completion_list above (because that would not DTRT when the completion results in both symbols and file names), so make_source_files_completion_list returns the full "/foo/bar" and "/foo/baz" strings. This produces wrong results when, e.g., there's only one possible completion, because rl_complete will prepend "/foo/" to each candidate completion. The loop below removes that leading part. */ Note that if you remove "word" from make_source_files_completion_list then add_filename_to_list collapses to a trivial function (which would otherwise be great except there's basic core functionality that I think should be kept). :-) So to maintain consistency with all "make_foo_completion_list" functions I think I'd like to keep make_source_files_completion_list as is. Could be missing something of course. OTOH the "text" arg to completion_list_add_name can go, it's unused.