From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13930 invoked by alias); 21 May 2014 07:48:27 -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 13914 invoked by uid 89); 21 May 2014 07:48:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f41.google.com Received: from mail-pa0-f41.google.com (HELO mail-pa0-f41.google.com) (209.85.220.41) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 21 May 2014 07:48:25 +0000 Received: by mail-pa0-f41.google.com with SMTP id lj1so1170760pab.0 for ; Wed, 21 May 2014 00:48:23 -0700 (PDT) X-Received: by 10.66.164.5 with SMTP id ym5mr56794858pab.50.1400658503553; Wed, 21 May 2014 00:48:23 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-50-sfba.hfc.comcastbusiness.net. [173.13.178.50]) by mx.google.com with ESMTPSA id it4sm6701583pbc.39.2014.05.21.00.48.22 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 21 May 2014 00:48:22 -0700 (PDT) From: Doug Evans To: Sergio Durigan Junior Subject: Re: [PATCH] PR python/16699: GDB Python command completion with overriden complete vs. completer class References: <87a9btqhe3.fsf@fleche.redhat.com> <87y4xwqn71.fsf@fleche.redhat.com> cc: gdb-patches@sourceware.org Date: Wed, 21 May 2014 07:48:00 -0000 In-Reply-To: (Sergio Durigan Junior's message of "Tue, 20 May 2014 23:09:45 -0300") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2014-05/txt/msg00460.txt.bz2 Sergio Durigan Junior writes: > [...] > Thanks. WDYT of the following patch? Hi. fwiw it's too bad the ability to plug in different completers isn't more, I dunno, parameterized (couldn't pick a better term, apologies - I thought of "object oriented" but that carries its own baggage). Performing completion obviously involves specifying more than a just single function (witness the comparison of the completer with specific functions). Plus it's more than specifying brkchars. Witness code like this: /* Many commands which want to complete on file names accept several file names, as in "run foo bar >>baz". So we don't want to complete the entire text after the command, just the last word. To this end, we need to find the beginning of the file name by starting at `word' and going backwards. */ for (p = word; p > tmp_command && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL; p--) ; IWBN if a "completer" object described how to do all these three things. Then the special case code for filename_completer (and location_completer) in completer.c could disappear. But maybe that's a patch for another day. Regarding the hack of using a static local to pass data from handle_brkchars to handle_completion, I know it's a hacky pragmatic choice. To get the reference counting right the code assumes that if the handle_brkchars phase is done then the handle_completion phase will be done too, right? I wonder if a SIGINT could sneak in there between the two passes (either today or tomorrow). Maybe the code in cmdpy_completer_helper for handle_brkchars_p could first check whether resultobj is already non-NULL, and decrement its reference count before setting it to NULL? And cmdpy_completer_helper could be defined to return a borrowed reference to resultobj? Dunno, just thinking out loud. Something puzzles me though: If it's ok to cache the completion result from the handle_brkchars pass to the handle_completion pass, why have two passes? It feels like there must be a case where this caching of the result in a static local from one pass to the next won't work. Another question: I noticed complete_command doesn't do this two-phase dance of handle_brkchars followed by handle_completions. Should it? It just goes straight to handle_completions. [Maybe that explains the difference from using TAB. Dunno off hand.] It seems like complete_command is trying to hand-code its own handle_brkchars handling: static void complete_command (char *arg, int from_tty) { int argpoint; char *point, *arg_prefix; VEC (char_ptr) *completions; dont_repeat (); if (arg == NULL) arg = ""; argpoint = strlen (arg); /* complete_line assumes that its first argument is somewhere within, and except for filenames at the beginning of, the word to be completed. The following crude imitation of readline's word-breaking tries to accomodate this. */ point = arg + argpoint; while (point > arg) { if (strchr (rl_completer_word_break_characters, point[-1]) != 0) break; point--; } arg_prefix = alloca (point - arg + 1); memcpy (arg_prefix, arg, point - arg); arg_prefix[point - arg] = 0; completions = complete_line (point, arg, argpoint); ... } TAB and the complete command should work identically of course, but for your testcase, maybe you should test both just to verify both work reasonably well (even if not identically). Given that complete_command doesn't do the two phase dance, does it work with your patch? Maybe it does, but IWBN to confirm that.