From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id qlT5AAPuLGA3ZAAAWB0awg (envelope-from ) for ; Wed, 17 Feb 2021 05:20:51 -0500 Received: by simark.ca (Postfix, from userid 112) id C3D051EF78; Wed, 17 Feb 2021 05:20:50 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_NONE,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 2F5741E4A3 for ; Wed, 17 Feb 2021 05:20:49 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 56EC03887027; Wed, 17 Feb 2021 10:20:48 +0000 (GMT) Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by sourceware.org (Postfix) with ESMTPS id 794AC3887027 for ; Wed, 17 Feb 2021 10:20:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 794AC3887027 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x429.google.com with SMTP id n8so16734091wrm.10 for ; Wed, 17 Feb 2021 02:20:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=h4ZXmnC9w7m0m0gCrGYlMvC3CDyflLqr5GBL80iWL5c=; b=Y9HcDWU7vMdfJArXaOn3fcy7Dp20Vmq2YdeOyVeao0IZVPfcecNC5XG8nDgygCHqu/ a5CxWwAJP8fdMG6vaHmqL0UK8vLu/vAPEunLgHj4Ktj8JzIItJ8xWNIJFCUip8EjAeEH 4xx4EzlysJxGoWJzJmn9DDzA66nRW3FbqdDAoRyParUYGVGng1i3C45E0Mt3wxAysUL8 vK8a/+HLiTc8ykfku3mN7tzrrG6gLaKYNhLi549y0xWtR5sNyfp9UTrbxdlzBeBsHVMV jSgyfa1PlUFy1yPcQ3T9V7UJqxmkk1cKZ6nhKNVNMw6uF5hO/x9j3uXLm7E3Sum6U0zX fZCw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=h4ZXmnC9w7m0m0gCrGYlMvC3CDyflLqr5GBL80iWL5c=; b=OPSEmyX9MFLseEyMQmMtwCxiB1lbPVRvQ4jjPBHkCvVA1O8FAQG5yXu8byOgvatx1/ 8HLMG369owmg6It0r4LErgfIKYGa5gqIk6/ZHHZNxoxqIM//kHqN6l3ZHVUeVS87bIkJ DpA6JZTwilJR16FjAzBsmIMFFkL4KoSlzO9uYKK7PV48PDktnmR+BPD15G9X+vnBw0xd ynDpz6PN5XbtgiTzUyUNLGfBMAhjab2/BqcnuC0ylnvF6pbrlX1gQaLuHl9gnsCFgZSS F60BjAjnYXccEzoGYsV2JMvLlsq7TZK/JJJdUKXZQDnTt53zmXBhpUII+iKeHHLqbDGC aZ8Q== X-Gm-Message-State: AOAM532vKBkZdeoSEJn5tUSUl1nj6BHDkpn3zSjPGCmFDhZbd/E30/n2 fskIq2JynA4I7gxUAc/2KWjWWJ4t77zrhQ== X-Google-Smtp-Source: ABdhPJzUyzLaHoYKrUBjX4MfaOEn3pav6xkcBN6NMz7sD71yuZXQ6/XodiExzeFm0MG8Z9oyAWOJ5Q== X-Received: by 2002:adf:e884:: with SMTP id d4mr27565156wrm.275.1613557244474; Wed, 17 Feb 2021 02:20:44 -0800 (PST) Received: from localhost (host86-186-80-154.range86-186.btcentralplus.com. [86.186.80.154]) by smtp.gmail.com with ESMTPSA id l7sm3291578wrn.11.2021.02.17.02.20.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Feb 2021 02:20:43 -0800 (PST) Date: Wed, 17 Feb 2021 10:20:42 +0000 From: Andrew Burgess To: Lancelot SIX Subject: Re: [PATCH 1/3] gdb::option: Add support for filename option. Message-ID: <20210217102042.GW265215@embecosm.com> References: <20210213220752.32581-1-lsix@lancelotsix.com> <20210213220752.32581-2-lsix@lancelotsix.com> <20210216174501.GS265215@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: X-Operating-System: Linux/5.8.13-100.fc31.x86_64 (x86_64) X-Uptime: 10:16:59 up 70 days, 15:01, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" * Lancelot SIX [2021-02-16 18:52:45 +0000]: > Le Tue, Feb 16, 2021 at 05:45:01PM +0000, Andrew Burgess a =C3=A9crit : > > * Lancelot SIX via Gdb-patches [2021-02-13= 22:07:50 +0000]: > >=20 > > > This commit adds support for filename option for the gdb::option pars= ing > > > framework. It is meant to help the user enter values that could be > > > parsed by built_argv. > > >=20 > > > Tab completion works as expected, i.e. similar to bash's behavior. > > >=20 > > > Considering the following tree structure: > > >=20 > > > somedir/ > > > =E2=94=9C=E2=94=80=E2=94=80 a folder > >=20 > > You probably want a trailing / after 'a folder' to make it clear it is > > a folder, similar to how somedir/ has the trailing character. > >=20 > > > =E2=94=94=E2=94=80=E2=94=80 a file > > >=20 > > > The following completions are available: > > >=20 > > > (gdb) maintenance test-options unknown-is-error -filename somedir= /a\ > > > a file a folder/ > > >=20 > > > or > > >=20 > > > (gdb) maintenance test-options unknown-is-error -filename "somedi= r/a > > > a file a folder/ > > >=20 > > > The complete command command also provides useful completions which a= re > > > escaped or quoted depending on what the user did input initially. > > >=20 > > > (gdb) complete maintenance test-options unknown-is-error -filenam= e somedir/a > > > maintenance test-options unknown-is-error -filename somedir/a\ fi= le > > > maintenance test-options unknown-is-error -filename somedir/a\ fo= lder/ > > > (gdb) complete maintenance test-options unknown-is-error -filenam= e 'somedir/a > > > maintenance test-options unknown-is-error -filename 'somedir/a fi= le > > > maintenance test-options unknown-is-error -filename 'somedir/a fo= lder/ > > >=20 > > > Note that since entering a path is an interactive process, the comple= ter > > > will not append the closing quote after a directory name. Instead it > > > adds a '/' and expect the user to provide further input. > > >=20 > > > Having both of those behaviors at the same time requires that the > > > completer knows if it has been called by readline after the user > > > pressed the tab key or by some other mechanism. In the first scenari= o it > > > should display non quoted/escaped completions so the user sees actual > > > filenames but complete quoted/escaped filenames. In the second scena= rio > > > it should only provide quoted/escaped completions. For that purpose, > > > this commit adds a m_from_readline property to the completion_tracker > > > object and initializes it accordingly to the calling context. > > >=20 > > > This filename_completer has slightly different behavior than the one > > > available in gdb/completer.c mainly with regard to escaping/quoting. > > > The original completer is kept as is to maintain stable behavior for > > > commands whose completer might depend on this implementation. If thi= s is > > > desirable, I=E2=80=99ll happily replace the existing filename complet= er with the > > > one currently proposed in the gdb::options::filename namespace instead > > > of having both with slightly different behaviors (but I an not sure if > > > users are relying on the current behavior of the filename > > > completer). > >=20 > > I wonder if you could expand more on what the differences are, maybe > > with some examples. Your new code sounds better, and I don't know why > > we would want to keep the existing code. > >=20 > The current completer does not handle spaces nor any > escape mechanism. >=20 > > Ideally I'd like to see some examples of things that the old completer > > allows us to do that would now break, or behave significantly > > differently if we just changed over to your code. > >=20 >=20 > The only case I see where using the new completer would break things is > a command which does not use built_argv to parse the argument line: >=20 > static void > foo_command (const char *args, int from_tty) > { > /* Handle args as a raw filename, or consider that everything that > follows a '--' is a filename. */ > const char *filename =3D args; > foo(filename); > } >=20 > Other than that, using a completer that handles space escaping or > quoting would only improve things. >=20 > I have not tracked down usages of the current completer to make sure no > command ingests the filename without any sort of processing. I=E2=80=99l= l do that > before sending a v2 of this patch series. Based on the above explanation then it sounds like moving to your completer globally. It should be fairly easy I'd think to track down which commands register with the old completer, and from there check how they process the filename. If any of them need fixing then I'd do that as a separate patch (in the same series) first. Then introduce your new completer and switch everyone over to it. Thanks, Andrew