From patchwork Thu Sep 26 10:16:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 97991 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 82771385840C for ; Thu, 26 Sep 2024 10:17:15 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id AAFFF3858D33 for ; Thu, 26 Sep 2024 10:16:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AAFFF3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org AAFFF3858D33 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727345779; cv=none; b=RkxfBEOdqnHxzQyriscMmnT6vr0+umqACnMCupZYxkrtp0OOhjGVdLRiMfH+zz+CfYp3BDKm0I2vSQ/nPXifH1y2JY7GZWDy+FFxgmSkiAb5BXeCGBx7qhVs26qq/7zCmYQGpiMX6w/7ZP4l0MZc1Cb/pI+lS5JUKbcfN3oau4g= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727345779; c=relaxed/simple; bh=S0A8hKsbxVLVS9xXN/PPszYkll5Xc5uuTAixl/Mnf/4=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=RL007rKbs0yxeL5EafXc2bTT6WGpF5zl8OQiZZ4F8AkDPQUZNDeXSrIBqa7gB64gnx9dE2f/o+CnIwcJshUSSRPcT7ya4TG/jcM9aYPN0FNceAH/W5Hjj1T6X5RvxDSLM2tUwCU71biRopDJBZXam5w0lnyV5YnzVw77VMvQPUs= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1727345777; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=wM/OePqApAhRu5Z7iYfsdOPoPpWWvtXSYCjapzox+1k=; b=EaML3shQoT/yN4CaL1PtKYImiLMWcN+7mSGVJ4wxjb/ApO8LaPUZedLRgfpa+HABxwacn8 6F6LAvMzTmHOfis2P0WVtV07wt+Z/3Sg0zxM0XhNa+i0dLl2Sy8zkuyaAnmVuTmuc9J1Gk WFxhpzaFzG5UCJ96tRxEWfK6x4JoLbM= Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-446-pw9QDQn3O3KEfX_T9LG_8w-1; Thu, 26 Sep 2024 06:16:16 -0400 X-MC-Unique: pw9QDQn3O3KEfX_T9LG_8w-1 Received: by mail-ej1-f70.google.com with SMTP id a640c23a62f3a-a8a9094c973so56409266b.2 for ; Thu, 26 Sep 2024 03:16:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727345774; x=1727950574; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=wM/OePqApAhRu5Z7iYfsdOPoPpWWvtXSYCjapzox+1k=; b=qz0FnD8hmWyZvBo0REcAABZjiQRWX8I4BiuUYz/Z00vETUxPoGpHXCTzfQuCl3kobR BuFjMQULE5xANDOXJgVtzhVp4pzHz/mIGdy8Z8uYeBTtX2XkD6G2NieJVCE6zRmoYVk2 RkKDZwaaPWOTJXJm12vGmGclP24zZZUaNlYDOoJDXjY2Nmb3oHTEHcM+2xR3Z2Z/7ZMP zGMMIl3F3ZvEi/ShjALBVguiYbtv1UV7++/rjK8bqo+jMiinYa4yt+Ftyy2Ae/eNlccA qBnO0W4Yhw784vspoCyIVCKoDoWlsMyb53kYr3H35txpZ0hG2ZC4T21GNS87zrbCeWln ss8w== X-Gm-Message-State: AOJu0YzH5m0JPxDqKnsbehFlqIBsTd5DBsaEE8zBizccg2yH/JHrvk2y gyLbAYlpp/lMVGbpaONfreJa/Ie3Gb5Fu65UZiqNykaTYMa0UlIvmh2E9vegFtBuw1vvM0/zkqg 5QiJ+m7JpEv1VrXp54HgvqO6OCrvXtpfXyLA+FjPZ1dyVrusZ8ICEKjFlsJQ9X2bmmYnrcVXm8Q UFpYrrPbM2nuejFPCaTFWCL7Q9sAJIzwoaLTU5gcme1Qo= X-Received: by 2002:a17:906:dc95:b0:a90:430e:6a34 with SMTP id a640c23a62f3a-a93a03b1bafmr619941866b.31.1727345774359; Thu, 26 Sep 2024 03:16:14 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFILGZum3Hq+8koOoI2IJrSsXkIM9zbcPgaocUQYmZU+IBjfTj2eHpmqnY3Dls2sEKvgrLDsA== X-Received: by 2002:a17:906:dc95:b0:a90:430e:6a34 with SMTP id a640c23a62f3a-a93a03b1bafmr619938866b.31.1727345773865; Thu, 26 Sep 2024 03:16:13 -0700 (PDT) Received: from localhost ([62.31.95.162]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a93930f82c4sm332304966b.181.2024.09.26.03.16.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 03:16:13 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Tom Tromey Subject: [PATCHv4 1/2] gdb: fix for completing a second filename for a command Date: Thu, 26 Sep 2024 11:16:08 +0100 Message-Id: <49a1d0865b3edf196de35e416acf5dd7d25136a6.1727345666.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org After the recent filename completion changes I noticed that the following didn't work as expected: (gdb) file "/path/to/some/file" /path/to/so Now, I know that the 'file' command doesn't actually take multiple filenames, but currently (and this was true before the recent filename completion changes too) the completion function doesn't know that the command only expects a single filename, and should complete any number of filenames. And indeed, this works: (gdb) file "/path/to/some/file" "/path/to/so In this case I quoted the second path, and now GDB is happy to offer completions. It turns out that the problem in the first case is an off-by-one bug in gdb_completer_file_name_char_is_quoted. This function tells GDB if a character within the line being completed is escaped or not. An escaped character cannot be a word separator. The algorithm in gdb_completer_file_name_char_is_quoted is to scan forward through the line keeping track of whether we are inside double or single quotes, or if a character follows a backslash. When we find an opening quote we skip forward to the closing quote and then check to see if we skipped over the character we are looking for, if we did then the character is within the quoted string. The problem is that this "is character inside quoted string" check used '>=' instead if '>'. As a consequence a character immediately after a quoted string would be thought of as inside the quoted string. In our first example this means that the single white space character after the quoted string was thought to be quoted, and was not considered a word breaking character. As such, GDB would not try to complete the second path. And indeed, if we tried this: (gdb) file "/path/to/some/file" /path/to/so That is, place multiple spaces after the first path, then GDB would consider the first space as quoted, but the second space is NOT quoted, and would be a word break. Now GDB does complete the second path. By changing '>=' to '>' in gdb_completer_file_name_char_is_quoted this bug is resolved, now the original example works and GDB will correctly complete the second path. For testing I've factored out the core of one testing proc, and I now run those tests multiple times, once with no initial path, once with an initial path in double quotes, once with an initial path in single quotes, and finally, with an unquoted initial path. Reviewed-By: Tom Tromey --- gdb/completer.c | 2 +- .../gdb.base/filename-completion.exp | 222 ++++++++++-------- 2 files changed, 121 insertions(+), 103 deletions(-) diff --git a/gdb/completer.c b/gdb/completer.c index e8f5e7bb577..64c6611f636 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -258,7 +258,7 @@ gdb_completer_file_name_char_is_quoted (char *string, int eindex) as quoted, though this might not be completely correct; the opening and closing quotes are not themselves quoted. But so far this doesn't seem to have caused any issues. */ - if (i >= eindex) + if (i > eindex) return 1; } else diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp index 95a9fbaa857..cdd597f689d 100644 --- a/gdb/testsuite/gdb.base/filename-completion.exp +++ b/gdb/testsuite/gdb.base/filename-completion.exp @@ -113,6 +113,119 @@ proc test_gdb_complete_filename_multiple { $testname } +# Run filename completetion tests for those command that accept quoting and +# escaping of the filename argument. CMD is the inital part of the command +# line, paths to complete will be added after CMD. +# +# ROOT is the base directory as returned from setup_directory_tree, though, +# if ROOT is a sub-directory of the user's home directory ROOT might have +# been modified to replace the $HOME prefix with a single "~" character. +proc run_quoting_and_escaping_tests_1 { root cmd } { + gdb_start + + # Completing 'thread apply all ...' commands uses a custom word + # point. At one point we had a bug where doing this would break + # completion of quoted filenames that contained white space. + test_gdb_complete_unique "thread apply all hel" \ + "thread apply all help" " " false \ + "complete a 'thread apply all' command" + + foreach_with_prefix qc [list "" "'" "\""] { + test_gdb_complete_none "$cmd ${qc}${root}/xx" \ + "expand a non-existent filename" + + test_gdb_complete_unique "$cmd ${qc}${root}/a" \ + "$cmd ${qc}${root}/aaa/" "" false \ + "expand a unique directory name" + + test_gdb_complete_unique "$cmd ${qc}${root}/cc2" \ + "$cmd ${qc}${root}/cc2${qc}" " " false \ + "expand a unique filename" + + test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \ + "b" "b" { + "bb1/" + "bb2/" + } "" "${qc}" false \ + "expand multiple directory names" + + test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \ + "c" "c" { + "cc1/" + "cc2" + } "" "${qc}" false \ + "expand mixed directory and file names" + + if { $qc ne "" } { + set sp " " + } else { + set sp "\\ " + } + + if { $qc eq "'" } { + set dq "\"" + } else { + set dq "\\\"" + } + + test_gdb_complete_unique "${cmd} ${qc}${root}/bb2/dir${sp}1/" \ + "${cmd} ${qc}${root}/bb2/dir${sp}1/unique${sp}file${qc}" " " \ + false \ + "expand a unique file name in a directory containing a space" + + test_gdb_complete_filename_multiple "$cmd ${qc}${root}/bb2/" \ + "d" "ir${sp}" { + "dir 1/" + "dir 2/" + } "" "${qc}" false \ + "expand multiple directory names containing spaces" + + test_gdb_complete_filename_multiple "${cmd} ${qc}${root}/bb2/dir${sp}2/" \ + "f" "ile${sp}" { + "file 1" + "file 2" + } "" "${qc}" false \ + "expand contents of a directory containing a space" + + test_gdb_complete_filename_multiple "$cmd ${qc}${root}/aaa/" \ + "a" "a${sp}" { + "aa bb" + "aa cc" + } "" "${qc}" false \ + "expand filenames containing spaces" + + test_gdb_complete_filename_multiple "$cmd ${qc}${root}/bb1/" \ + "a" "a" { + "aa\"bb" + "aa'bb" + } "" "${qc}" false \ + "expand filenames containing quotes" + + test_gdb_complete_unique "$cmd ${qc}${root}/bb1/aa${dq}" \ + "$cmd ${qc}${root}/bb1/aa${dq}bb${qc}" " " false \ + "expand unique filename containing double quotes" + + # It is not possible to include a single quote character + # within a single quoted string. However, GDB does not do + # anything smart if a user tries to do this. Avoid testing + # this case. Maybe in the future we'll figure a way to avoid + # this situation. + if { $qc ne "'" } { + if { $qc eq "" } { + set sq "\\'" + } else { + set sq "'" + } + + test_gdb_complete_unique "$cmd ${qc}${root}/bb1/aa${sq}" \ + "$cmd ${qc}${root}/bb1/aa${sq}bb${qc}" " " false \ + "expand unique filename containing single quote" + } + } + + gdb_exit +} + # Run filename completetion tests for those command that accept quoting and # escaping of the filename argument. # @@ -127,109 +240,14 @@ proc run_quoting_and_escaping_tests { root } { "target core" "target exec" "target tfile" \ "maint print c-tdesc" "compile file" \ "save gdb-index" "save gdb-index -dwarf-5" } { - gdb_start - - # Completing 'thread apply all ...' commands uses a custom word - # point. At one point we had a bug where doing this would break - # completion of quoted filenames that contained white space. - test_gdb_complete_unique "thread apply all hel" \ - "thread apply all help" " " false \ - "complete a 'thread apply all' command" - - foreach_with_prefix qc [list "" "'" "\""] { - test_gdb_complete_none "$cmd ${qc}${root}/xx" \ - "expand a non-existent filename" - - test_gdb_complete_unique "$cmd ${qc}${root}/a" \ - "$cmd ${qc}${root}/aaa/" "" false \ - "expand a unique directory name" - - test_gdb_complete_unique "$cmd ${qc}${root}/cc2" \ - "$cmd ${qc}${root}/cc2${qc}" " " false \ - "expand a unique filename" - - test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \ - "b" "b" { - "bb1/" - "bb2/" - } "" "${qc}" false \ - "expand multiple directory names" - - test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \ - "c" "c" { - "cc1/" - "cc2" - } "" "${qc}" false \ - "expand mixed directory and file names" - - if { $qc ne "" } { - set sp " " - } else { - set sp "\\ " - } - - if { $qc eq "'" } { - set dq "\"" - } else { - set dq "\\\"" - } - - test_gdb_complete_unique "${cmd} ${qc}${root}/bb2/dir${sp}1/" \ - "${cmd} ${qc}${root}/bb2/dir${sp}1/unique${sp}file${qc}" " " \ - false \ - "expand a unique file name in a directory containing a space" - - test_gdb_complete_filename_multiple "$cmd ${qc}${root}/bb2/" \ - "d" "ir${sp}" { - "dir 1/" - "dir 2/" - } "" "${qc}" false \ - "expand multiple directory names containing spaces" - - test_gdb_complete_filename_multiple "${cmd} ${qc}${root}/bb2/dir${sp}2/" \ - "f" "ile${sp}" { - "file 1" - "file 2" - } "" "${qc}" false \ - "expand contents of a directory containing a space" - - test_gdb_complete_filename_multiple "$cmd ${qc}${root}/aaa/" \ - "a" "a${sp}" { - "aa bb" - "aa cc" - } "" "${qc}" false \ - "expand filenames containing spaces" - - test_gdb_complete_filename_multiple "$cmd ${qc}${root}/bb1/" \ - "a" "a" { - "aa\"bb" - "aa'bb" - } "" "${qc}" false \ - "expand filenames containing quotes" - - test_gdb_complete_unique "$cmd ${qc}${root}/bb1/aa${dq}" \ - "$cmd ${qc}${root}/bb1/aa${dq}bb${qc}" " " false \ - "expand unique filename containing double quotes" - - # It is not possible to include a single quote character - # within a single quoted string. However, GDB does not do - # anything smart if a user tries to do this. Avoid testing - # this case. Maybe in the future we'll figure a way to avoid - # this situation. - if { $qc ne "'" } { - if { $qc eq "" } { - set sq "\\'" - } else { - set sq "'" - } - - test_gdb_complete_unique "$cmd ${qc}${root}/bb1/aa${sq}" \ - "$cmd ${qc}${root}/bb1/aa${sq}bb${qc}" " " false \ - "expand unique filename containing single quote" - } + # Try each test placing the filename as the first argument + # then again with a quoted string immediately after the + # command. This works because the filename completer will + # complete any number of filenames, even if the command only + # takes a single filename. + foreach_with_prefix filler { "" " \"xxx\"" " 'zzz'" " yyy"} { + run_quoting_and_escaping_tests_1 $root "$cmd$filler" } - - gdb_exit } } From patchwork Thu Sep 26 10:16:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 97990 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C56333858C78 for ; Thu, 26 Sep 2024 10:16:54 +0000 (GMT) X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 46FA43858D39 for ; Thu, 26 Sep 2024 10:16:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 46FA43858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 46FA43858D39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727345785; cv=none; b=xuJTA7pVPHm29kN9/IjrfpCRN++gFmJ2LS0vfNa5xnKaVHA+UP/eHFYvF6lmFzURHTn6d4Pe0wMpcVkZppdDPoKM0/sZU+hVMtkUENrrfgGIDkAcjHwQmSV0e+MOG2WkioRzG6X9+n9u73hJERTqN52mBP3MVGz1SjcYNj9gc+I= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1727345785; c=relaxed/simple; bh=aKT6NfYI1g1ukIRSzYCdOiSS3md72e0bGWguvI6muHc=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=AEELjt/8bTeDYlWnFxwY+03nj90SG6jX2WjCSlPziSY4kXJfrIUJLKRJ+rGtANEnxo8ee01UlxJRCg1MuNTXULW0+51W512k7G+0VmOt5j895jw2xk2Ba73T6GZHTwfq4BF0uvFe07IB6JyGuZIX7be6BxhDG2Q10/IT7ht79DY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1727345781; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=D1D6Fl3Yt+qowHY2dcuk0F6D/NV69IQe1We90k+9Zf0=; b=fOAHBK2g8vRye3p7LEDD9RI3qQyKc955VB5fhpOMBuLFz/m7LkM/V6+0kWA7mRbw+0b3Qg ayBTDBHoYbrOBW4NBJsdHZYRq/d43G21RGpNIqxdsp1DQTcSAk40JgfFPITOK5N/J6PmyK FgrVsRtNd8WUhH9EBQ5WYY8ujNJdm7c= Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com [209.85.208.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-107-45nMAE0mOSe1eDq5GN37cg-1; Thu, 26 Sep 2024 06:16:19 -0400 X-MC-Unique: 45nMAE0mOSe1eDq5GN37cg-1 Received: by mail-lj1-f198.google.com with SMTP id 38308e7fff4ca-2f75b13c2c5so6030581fa.1 for ; Thu, 26 Sep 2024 03:16:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727345777; x=1727950577; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=D1D6Fl3Yt+qowHY2dcuk0F6D/NV69IQe1We90k+9Zf0=; b=b3mhJV47U/NEaiSNaSl2kpN3v/PjXzIbEyc/PR+wGpDN4ehqi2q/3hLfLJTQGwyNi3 tjhG0rh02ywRBBOJ5kthXja3vSTD++L/6wECerLQow6U8SrNuhuBc0vpOw9r9jd4zU72 qZ+x7PINPma6uBDi4pGFM28YugIqsFZJCXOMrD+GV3e2x+wV4mT4wBlpIJaB+tUAy7Qu Cfq83HqQPq5WW7d4iD0ORUici6mzRBbQHitR+R55ZJXjwheM2Oj3C4VvmTrzLxN4DWoC o6XerpwPk985FqcHbYPRo+FMF/mq42QM5fZINNT44x9X7GD7q72n5nJ4GL8mLR/RGmhM qH8Q== X-Gm-Message-State: AOJu0Ywl+pZ/3K9MBtdo+RtWY3iNfcH3ObAqcjgxcafzcx7hv+b9Bt5A 5F8/qsHBe8NMurbtVjrhMYEdRVAPEAsVWIpHWyznNK8+DvT435G38Rh9mqTq3k5N7WxpGRth/qi T38ph3XaNhlXlZz5LpFC7xvoS9gCfvz4OhqoblnvmW6BWcOgJMggEl5mZsz38p7390W9CHkJLkj BdB8Km0qSVfmbWQ5qC2tKtR+kn0k/NXQKAwYQPuRR87ho= X-Received: by 2002:a2e:b8d4:0:b0:2f7:4c9d:7a87 with SMTP id 38308e7fff4ca-2f91602f5demr39940551fa.21.1727345776737; Thu, 26 Sep 2024 03:16:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IErfGZLjUO1oUWff1H4GET15dOPtaHZ+rLWLlAGup1B1ec4eFjulanz/klk+FSBk84qs1yPIg== X-Received: by 2002:a2e:b8d4:0:b0:2f7:4c9d:7a87 with SMTP id 38308e7fff4ca-2f91602f5demr39940161fa.21.1727345775984; Thu, 26 Sep 2024 03:16:15 -0700 (PDT) Received: from localhost ([62.31.95.162]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5c879f2dd21sm478042a12.77.2024.09.26.03.16.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 03:16:14 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess , Tom Tromey Subject: [PATCHv4 2/2] gdb: fix filename completion in the middle of a line Date: Thu, 26 Sep 2024 11:16:09 +0100 Message-Id: <680a589f072ce19d6a8735d08072d4bf29b8f1a5.1727345666.git.aburgess@redhat.com> X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~patchwork=sourceware.org@sourceware.org I noticed that filename completion in the middle of a line doesn't work as I would expect it too. For example, assuming '/tmp/filename' exists, and is the only file in '/tmp/' then when I do the following: (gdb) file "/tmp/filen GDB completes to: (gdb) file "/tmp/filename" But, if I type this: (gdb) file "/tmp/filen "xxx" Then move the cursor to the end of '/tmp/filen' and press , GDB will complete the line to: (gdb) file "/tmp/filename "xxx" But GDB will not insert the trailing double quote character. The reason for this is found in readline/readline/complete.c in the function append_to_match. This is the function that appends the trailing closing quote character, however, the closing quote is only inserted if the cursor (rl_point) is at the end (rl_end) of the line being completed. In this patch, what I do instead is add the closing quote in the function gdb_completer_file_name_quote, which is called from readline through the rl_filename_quoting_function hook. The docs for rl_filename_quoting_function say (see 'info readline'): "... The MATCH_TYPE is either 'SINGLE_MATCH', if there is only one completion match, or 'MULT_MATCH'. Some functions use this to decide whether or not to insert a closing quote character. ..." This is exactly what I'm doing in this patch, and clearly this is not an unusual choice. Now after completing a filename that is not at the end of the line GDB will add the closing quote character if appropriate. I have managed to write some tests for this. I send a line of text to GDB which includes a partial filename followed by a trailing string, I then send the escape sequence to move the cursor left, and finally I send the tab character. Obviously, expect doesn't actually see the complete output with the extra text "in place", instead expect sees the original line followed by some escape sequences to reflect the cursor movement, then an escape sequence to indicate that text is being inserted in the middle of a line, followed by the new characters ... it's a bit messy, but I think it holds together. Reviewed-By: Tom Tromey --- gdb/completer.c | 30 +++- .../gdb.base/filename-completion.exp | 163 ++++++++++++++++++ 2 files changed, 189 insertions(+), 4 deletions(-) diff --git a/gdb/completer.c b/gdb/completer.c index 64c6611f636..4735aa5ed10 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -31,6 +31,7 @@ #include "linespec.h" #include "cli/cli-decode.h" #include "gdbsupport/gdb_tilde_expand.h" +#include "readline/readline.h" /* FIXME: This is needed because of lookup_cmd_1 (). We should be calling a hook instead so we eliminate the CLI dependency. */ @@ -393,13 +394,34 @@ gdb_completer_file_name_quote_1 (const char *text, char quote_char) the quote character surrounding TEXT, or points to the null-character if there are no quotes around TEXT. MATCH_TYPE will be one of the readline constants SINGLE_MATCH or MULTI_MATCH depending on if there is one or - many completions. */ + many completions. + + We also add a trailing character, either a '/' of closing quote, if + MATCH_TYPE is 'SINGLE_MATCH'. We do this because readline will only + add this trailing character when completing at the end of a line. */ static char * -gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED, - char *quote_ptr) +gdb_completer_file_name_quote (char *text, int match_type, char *quote_ptr) { - return gdb_completer_file_name_quote_1 (text, *quote_ptr); + char *result = gdb_completer_file_name_quote_1 (text, *quote_ptr); + + if (match_type == SINGLE_MATCH) + { + /* Add trailing '/' if TEXT is a directory, otherwise add a closing + quote character matching *QUOTE_PTR. */ + char c = (gdb_path_isdir (gdb_tilde_expand (text).c_str ()) + ? '/' : *quote_ptr); + + /* Reallocate RESULT adding C to the end. But only if C is + interesting, otherwise we can save the reallocation. */ + if (c != '\0') + { + char buf[2] = { c, '\0' }; + result = reconcat (result, result, buf, nullptr); + } + } + + return result; } /* The function is used to update the completion word MATCH before diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp index cdd597f689d..85fac35d7c8 100644 --- a/gdb/testsuite/gdb.base/filename-completion.exp +++ b/gdb/testsuite/gdb.base/filename-completion.exp @@ -113,6 +113,148 @@ proc test_gdb_complete_filename_multiple { $testname } +# Helper proc. Returns a string containing the escape sequence to move the +# cursor COUNT characters to the left. There's no sanity checking performed +# on COUNT, so the user of this proc must ensure there are more than COUNT +# characters on the current line. +proc c_left { count } { + string repeat "\033\[D" $count +} + +# This proc is based off of test_gdb_complete_tab_multiple in +# completion-support.exp library. This proc however tests completing a +# filename in the middle of a command line. +# +# INPUT_LINE is the line to complete, BACK_COUNT is the number of characters +# to move the cursor left before sending tab to complete the line. +# ADD_COMPLETED_LINE is what we expect to be unconditionally added the first +# time tab is sent. On additional tabs COMPLETION_LIST will be displayed. +# TESTNAME is used as expected. +proc test_tab_complete_within_line_multiple { input_line back_count \ + add_completed_line \ + completion_list \ + testname } { + global gdb_prompt + + # After displaying the completion list the line will be reprinted, but + # now with ADD_COMPLETED_LINE inserted. Build the regexp to match + # against this expanded line. The new content will be inserted + # BACK_COUNT character from the end of the line. + set expanded_line \ + [join [list \ + [string range $input_line 0 end-$back_count] \ + ${add_completed_line} \ + [string range $input_line end-[expr $back_count - 1] end]] \ + ""] + set expanded_line_re [string_to_regexp $expanded_line] + + # Convert input arguments into regexp. + set input_line_re [string_to_regexp $input_line] + set add_completed_line_re [string_to_regexp $add_completed_line] + set completion_list_re [make_tab_completion_list_re $completion_list] + + # Similar to test_tab_complete_within_line_unique, build two + # regexp for matching the line after the first tab. Which regexp + # matches will depend on the version and/or configuration of + # readline. This first regexp moves the cursor backwards and then + # inserts new content into the line. + set after_tab_re1 "^$input_line_re" + set after_tab_re1 "$after_tab_re1\\\x08{$back_count}" + set after_tab_re1 "$after_tab_re1${completion::bell_re}" + set after_tab_re1 "$after_tab_re1\\\x1b\\\x5b[string length $add_completed_line]\\\x40" + set after_tab_re1 "$after_tab_re1$add_completed_line_re\$" + + # This second regexp moves the cursor backwards and overwrites the + # end of the line, then moves the cursor backwards again to the + # correct position within the line. + set after_tab_re2 "^$input_line_re" + set after_tab_re2 "$after_tab_re2\\\x08{$back_count}" + set after_tab_re2 "$after_tab_re2${completion::bell_re}" + set tail [string range $input_line end-[expr $back_count - 1] end] + set after_tab_re2 "$after_tab_re2$add_completed_line_re" + set after_tab_re2 "$after_tab_re2[string_to_regexp $tail]" + set after_tab_re2 "$after_tab_re2\\\x08{$back_count}" + + send_gdb "$input_line[c_left $back_count]\t" + gdb_test_multiple "" "$testname (first tab)" { + -re "(?:(?:$after_tab_re1)|(?:$after_tab_re2))" { + send_gdb "\t" + # If we auto-completed to an ambiguous prefix, we need an + # extra tab to show the matches list. + if {$add_completed_line != ""} { + send_gdb "\t" + set maybe_bell ${completion::bell_re} + } else { + set maybe_bell "" + } + gdb_test_multiple "" "$testname (second tab)" { + -re "^${maybe_bell}\r\n$completion_list_re\r\n$gdb_prompt " { + gdb_test_multiple "" "$testname (second tab)" { + -re "^$expanded_line_re\\\x08{$back_count}$" { + pass $gdb_test_name + } + } + } + -re "${maybe_bell}\r\n.+\r\n$gdb_prompt $" { + fail $gdb_test_name + } + } + } + } + + clear_input_line $testname +} + +# Wrapper around test_gdb_complete_tab_unique to test completing a unique +# item in the middle of a line. INPUT_LINE is the line to complete. +# BACK_COUNT is the number of characters to move left within INPUT_LINE +# before sending tab to perform completion. INSERT_STR is what we expect to +# see inserted by the completion engine in GDB. +proc test_tab_complete_within_line_unique { input_line back_count insert_str } { + # Build regexp for the line after completion has occurred. As + # completion is being performed in the middle of the line the + # sequence of characters we see can vary depending on which + # version of readline is in use, and/or how readline is + # configured. Currently two different approaches are covered as + # RE1 and RE2. Both of these regexp cover the complete possible + # output. + # + # In the first case we see the input line followed by some number + # of characters to move the cursor backwards. After this we see a + # control sequence that tells the terminal that some characters + # are going to be inserted into the middle of the line, the new + # characters are then emitted. The terminal itself is responsible + # for preserving the tail of the line, so these characters are not + # re-emitted. + set re1 [string_to_regexp $input_line] + set re1 $re1\\\x08{$back_count} + set re1 $re1\\\x1b\\\x5b[string length $insert_str]\\\x40 + set re1 $re1[string_to_regexp $insert_str] + + # In this second regexp we again start with the input line + # followed by the control characters to move the cursor backwards. + # This time though readline emits the new characters and then + # re-emits the tail of the original line. This new content will + # overwrite the original output on the terminal. Finally, control + # characters are emitted to move the cursor back to the correct + # place in the middle of the line. + set re2 [string_to_regexp $input_line] + set re2 $re2\\\x08{$back_count} + set re2 $re2[string_to_regexp $insert_str] + set tail [string range $input_line end-[expr $back_count - 1] end] + set re2 $re2[string_to_regexp $tail] + set re2 $re2\\\x08{$back_count} + + # We can now perform the tab-completion, we check for either of + # the possible output regexp patterns. + test_gdb_complete_tab_unique \ + "${input_line}[c_left $back_count]" \ + "(?:(?:$re1)|(?:$re2))" \ + "" \ + "complete unique file within command line" +} + + # Run filename completetion tests for those command that accept quoting and # escaping of the filename argument. CMD is the inital part of the command # line, paths to complete will be added after CMD. @@ -226,6 +368,25 @@ proc run_quoting_and_escaping_tests_1 { root cmd } { gdb_exit } +# Tests for completing a filename in the middle of a command line. This +# represents a user typing out a command line then moving the cursor left +# (e.g. with the left arrow key), editing a filename argument, and then +# using tab completion to try and complete the filename even though there is +# other content on the command line after the filename. +proc run_mid_line_completion_tests { root cmd } { + gdb_start + + test_tab_complete_within_line_unique \ + "$cmd \"$root/bb2/dir 1/unique fi \"xxx\"" 6 "le\"" + + test_tab_complete_within_line_multiple \ + "$cmd \"$root/aaa/a \"xxx\"" 6 "a " \ + [list "aa bb" "aa cc"] \ + "complete filename mid-line with multiple possibilities" + + gdb_exit +} + # Run filename completetion tests for those command that accept quoting and # escaping of the filename argument. # @@ -248,6 +409,8 @@ proc run_quoting_and_escaping_tests { root } { foreach_with_prefix filler { "" " \"xxx\"" " 'zzz'" " yyy"} { run_quoting_and_escaping_tests_1 $root "$cmd$filler" } + + run_mid_line_completion_tests $root $cmd } }