From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 6ECBB3858D3C for ; Fri, 12 Apr 2024 22:20:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6ECBB3858D3C 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 6ECBB3858D3C 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=1712960440; cv=none; b=CzzEKO34txpFj7jqEVFBEF2bgmr4yVXwzrkFj8zChiCcVNh35lBtitcIch9U3a3cSMC4h60FDMGzZvtx8wsXTfz7T6JCeoTxbOX+Mz8pgzlzSS+iFwoF32mhm9rQmbqQD5PLGUsYzCmbksv5ubliF7IWPSRdunqCO9Wg/rAO4m8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712960440; c=relaxed/simple; bh=KrXhHWB+tIzo86BGp8spKqjgEBCXBLIQGH+7OQXgNY8=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=QXNOTbF0AJ/bae+RwtJqOWcMauqfZ6uql+/rxjw/f5eJ4+GSVXGRJ3B5m6E7icueXmTeCWSQM/SxmLcX1Z5v6vPioChiwN2ulcMqoDJZGUJpzpv6/tN6TBO1ki8YtiHaAS2FATmcruOVGvPaUuTVFqXtIsO3WTpPgd08h4zGvAk= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1712960438; 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: in-reply-to:in-reply-to:references:references; bh=NfC52CI+Pq9Xb/ARECs8iv3Wdc8QAccd7G5ik3+TLUM=; b=e4Sm+7Fek+SWDqTw5d+RvBdl3Z+5EgC4iySCT5SvYHyvHpsxEpp9pmGLLxfeXyO6RNOywu wszdAgxGXKrpFmkYoicXfEqYG7MkPt5q7TI7wve1fynLKMvbKsc0kIqlswf1Z5kGFrD2tD njDUl8MEOfs0pb27fCnWw005M9SAciI= Received: from mail-lj1-f200.google.com (mail-lj1-f200.google.com [209.85.208.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-275-2lV83kLONu--ZKH98w-e0A-1; Fri, 12 Apr 2024 18:20:36 -0400 X-MC-Unique: 2lV83kLONu--ZKH98w-e0A-1 Received: by mail-lj1-f200.google.com with SMTP id 38308e7fff4ca-2d85db10d1dso11627221fa.2 for ; Fri, 12 Apr 2024 15:20:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712960435; x=1713565235; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=NfC52CI+Pq9Xb/ARECs8iv3Wdc8QAccd7G5ik3+TLUM=; b=jBwE9p1EyKxr+cGnYNqC434z+14M7VihdSoYTXRBrxSrgCVMi8Q2flYTvwz1YFxKiq hXiM6MnT6/BOqkuT4EUACt7oZX2JiToWblvU5bVJ34B4f+V5tydPkvMufEbtmM6e+X3k QoXRJHZ9CW5tWs3tjd5tSCTLplkTYS1HPWYGF0t+cyWhBJ93TT4uot+/TL24WeymFnIG r31zM89VZtjQZg5oSnIYSf9w8NWZkJlNeoc06QLElhpFvdvDKfmqXa8hz7xPoSNwdFh4 Y7qfg5jdFXf02AKD87zMX7cyfCgBixsGV6kvUAtZKJw5Rv+AYPz7bgcdXaOnKnuDce/O 5aqw== X-Forwarded-Encrypted: i=1; AJvYcCWJBmSRmTRYsQAZaOfmWwcbWqmmibB1MfVZWx11EitSTutYJOTJRPg2xS+m0bylxTDHO4WAJoHArO/z7Z+mQn5w2ylMRtBr0bf5lA== X-Gm-Message-State: AOJu0YzSYSgey19OAILzrWEVvcCbOwCjejdNjOEY1Jzg3ai3Gq+3K5vD q1yWJW6pkpzYDd5BdoR1UCJRXwls7fRLSr0waIcT/pkCNr0Vtvl1JCzUkA1J3Dnj8ZcP3pWXas2 zLSKBHp1DuvsuQj9hDqi9EtzPgFQEMN3yeLW2VizF/G39BorASz2zDmUsGcmLu1CbYsQ= X-Received: by 2002:a2e:87ce:0:b0:2d9:ec13:3349 with SMTP id v14-20020a2e87ce000000b002d9ec133349mr2436483ljj.2.1712960434627; Fri, 12 Apr 2024 15:20:34 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEAj3I1rZH3VR0vAYLkevw2tC1Hsd4yXJyLjqiDgo2fJxOs4v2FOWWmCdSPTJtm47icT5GeYQ== X-Received: by 2002:a2e:87ce:0:b0:2d9:ec13:3349 with SMTP id v14-20020a2e87ce000000b002d9ec133349mr2436472ljj.2.1712960434010; Fri, 12 Apr 2024 15:20:34 -0700 (PDT) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id m11-20020a05600c4f4b00b0041816c3049csm1174225wmq.11.2024.04.12.15.20.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Apr 2024 15:20:33 -0700 (PDT) From: Andrew Burgess To: Eli Zaretskii Cc: lsix@lancelotsix.com, gdb-patches@sourceware.org Subject: Re: [PATCH 2/6] gdb: move display of completion results into completion_result class In-Reply-To: <867ch2s9rw.fsf@gnu.org> References: <86v855dyls.fsf@gnu.org> <20240330233038.sol5j3cp7vsym4uz@octopus> <86v853ar3c.fsf@gnu.org> <87v84m4hps.fsf@redhat.com> <867ch2s9rw.fsf@gnu.org> Date: Fri, 12 Apr 2024 23:20:33 +0100 Message-ID: <87le5i440e.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,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 List-Id: Eli Zaretskii writes: >> From: Andrew Burgess >> Cc: gdb-patches@sourceware.org >> Date: Fri, 12 Apr 2024 18:24:31 +0100 >> >> Eli Zaretskii writes: >> >> > Next, if 'file' indeed expects shell file-name semantics, then the >> > question is: which shell? Should the above behave differently on, for >> > example, MS-Windows, since file-name quoting there works differently? >> > For example, escaping whitespace with a backslash will not work on >> > Windows. >> >> I think calling this "shell file-name semantics" makes this change seem >> worse than it is. If we want to call it anything, then it should be >> called gdb-shell semantics. > > What is "gdb-shell"? Saying "gdb-shell semantics" is only useful if > the term "gdb-shell" is known and understood by many. Otherwise, we > should not use this terminology, because it doesn't explain anything. It's a name I made up for the thing you type at after a (gdb) prompt. The point is, it's not a "shell", it's GDB. It has its own rules. > >> > And last, but not least: the manual says about the 'complete' command: >> > >> > This is intended for use by GNU Emacs. >> > >> > So one other thing we should consider is how Emacs handles these cases >> > and what it expects from GDB in response. >> >> Happy to test such a thing. Can you point me at any instruction/guides >> for how to trigger completion via emacs? > > Invoke either "M-x gdb" or "M-x gud-gdb" from Emacs, and then, after > the debugging session starts, type TAB to complete file names in > commands that accept file names. Thank you. I got this working and did some tests, I'll write up my results at the end of this email. > >> My hope is that this change will fix things rather than break them. >> Previously the 'complete' command would output a partial completion that >> was invalid, that is, if the user passes emacs a short string and then >> triggers completion, GDB would provide a longer string which was >> invalid. If emacs then feeds that longs string back to GDB the command >> isn't going to work. After this change that should no longer be the >> case. > > I hope you are right. Let's see. In general, Emacs doesn't need any > quoting when it receives file names from a sub-process (in this case, > from GDB). See below. > >> >> The problem Andrew is trying to solve is that the current completer for >> >> this command completes to something that is not a valid input. >> > >> > My point is that we need to have a clear idea what is a "valid input" >> > before we decide whether the current behavior is invalid, and if so, >> > how to fix it. >> >> I do disagree with the "decide whether the current behaviour is invalid" >> part of this text. Completion of files containing whitespace doesn't >> work right now, so I'd suggest the current behaviour is self-evidently >> invalid. But ... >> >> > IOW, we should step back and discuss the valid >> > behavior first. >> >> I'm totally happy to take suggestions on what the working behaviour >> should look like. > > I made one alternative suggestion about completing a file name that > begins with a quote, for example. Whether it is a good suggestion > depends on what we want the behavior to be, so that should preferably > be discussed before deciding on the solution. OK. But right now GDB uses readline for its shell/session. And readline completion works in a particular way. We (GDB) don't get much say over that unless we want to start seriously hacking on readline. These patches aren't me coming up with some radical new approach and trying to push it onto GDB. This is just me connecting up readline and GDB a little more fully, and letting readline do its thing. How tab completion works is really a readline feature. Now where I have "invented" things, to some degree, is with the 'complete' command, which, as you point out, emacs uses. My assumption was that the 'complete' was used like this: 1. User types a partial command in emacs buffer, 2. User invokes completion, 3. Emacs grabs the partial command and sends it to GDB's 'complete' command, 4. GDB returns a set of partial completions, 5. Emacs presents each completion to the user, and allows the user to select one, 6. Emacs inserts the selected text into the buffer, extending the partially completed command, 7. User hits and the command is executed. Having had a dig through gdb-mi.el and gud.el (from the emacs source) I think I was pretty much correct with my assumptions. And all the changes I've made to the 'complete' command have been intended to support this use case, e.g. adding the escapes to the 'complete' command output. Notice that readline by default does NOT add the escaping, e.g.: (gdb) file /tmp/qqq/aa => file /tmp/qqq/aa\ aa bb/ aa cc/ Here the ' ' in 'aa bb/' and 'aa cc/' is not escaped. However, I made sure that if I do: (gdb) complete file /tmp/eee/aa file /tmp/eee/aa\ bb/ file /tmp/eee/aa\ cc/ (gdb) Then the whitespace is escaped, because if it wasn't and emacs just dropped the text in without the escaping, then GDB would see two separate words and do the wrong thing. OK, so now the bad news. My patched GDB doesn't "just work" with emacs. Remember though, an unpatched GDB doesn't work either when trying to complete a filename with spaces in, so it's not that I've made things worse. I tracked at least one bug down to this function in gud.el: (defun gud-gdb-completion-at-point () "Return the data to complete the GDB command before point." (let ((end (point)) (start (save-excursion (skip-chars-backward "^ " (comint-line-beginning-position)) (point)))) ;; FIXME: `gud-gdb-run-command-fetch-lines' has some nasty side-effects on ;; the buffer (via `gud-delete-prompt-marker'): it removes the prompt and ;; then re-adds it later, thus messing up markers and overlays along the ;; way (bug#18282). ;; We use an "insert-before" marker for `start', since it's typically right ;; after the prompt, which works around the problem, but is a hack (and ;; comes with other downsides, e.g. if completion adds text at `start'). (list (copy-marker start t) end (completion-table-dynamic (apply-partially gud-gdb-completion-function (buffer-substring (comint-line-beginning-position) start)))))) This is trying to figure out everything before the completion word on the current line and doing the apply-partial with this context on gud-gdb-completion-function. So if we are completing 'file /tmp/abc' then we'd end up doing: (apply-partial gud-gdb-completion-function "file ") The completion context is calculated as running from the start of the line up to 'start', which is calculated in the 'let'. And 'start' is calculated by starting from 'point' and moving backward to the first whitespace. That's not going to work for completing any line that contains a space. I think this function would need to get smarter to understand about escaping and quoting. And, finally, some good news. If a filename _doesn't_ include any spaces, then as far as I can tell, from my limited testing, everything works fine in emacs, and is now improved. On an unpatched GDB, given a file /tmp/qqq/f1, within emacs: (gdb) file /tmp/qq => file /tmp/qqq *Sole Completion* That's it. At this point emacs will not help me complete this at all. I have to manually add the '/' and only then will emacs offer the next level of completion. Compare this to plain GDB: (gdb) file /tmp/qq => file /tmp/qqq/ => file /tmp/qqq/f1 With a patched GDB, emacs is now as good as plain GDB. This is thanks to me adding the trailing '/' character in the 'complete' command output. And on the subject of quotes. With an unpatched GDB emacs is perfectly happy completing a file containing quotes (well, as happy as it is with an unquoted file name), e.g.: (gdb) file "/tmp/qq => file "/tmp/qqq But, when I get to the final file completion, emacs doesn't add the trailing quote, so: (gdb) file "/tmp/qqq/f => file "/tmp/qqq/f1 In contrast and *unpatched* GDB at the CLI will add the final quote. With a patched GDB, emacs is just as happy handling the quotes, only now it adds the final trailing quote on, just like GDB's CLI: (gdb) file "/tmp/qqq/f => file "/tmp/qqq/f1" OK, so summary of where we're at: + I think we're restricted to doing file completion at the GDB CLI the readline way. I think trying to force readline to do things differently is just going to create maintenance overhead. Of course, if someone wants to support such a thing, I love to see their plan. But for now, I think we should keep it simple, do things the readline way. Of course, if there's real problems that this causes then maybe we have to do things differently. Let me know if any problems are found, + The complete command changes, as far as I can tell, only improve the emacs experience for filenames that _don't_ include white space. Nothing appears to be broken, but my testing has been limited. Please report any issues found, + For filenames that include white space, sadly, emacs doesn't "just work". I've found one issue with the emacs code that isn't helping, but I suspect there's likely to be others beyond that. That said, this is NOT a regression. In fact, I think emacs is getting further through the completion process now (GDB is now sending it actual completions rather than junk), it's just emacs has never seen completions like this before (previous GDB would just send junk), so it's not really a surprise that this has exposed some bugs. I'm willing to commit to creating emacs bugs for the completion features if/when this patch is merged so that these things can be addressed on the emacs side, + We are agreed that there's clearly a documentation issue. However, I would argue that the documentation is missing for current functionality (i.e. the current rules for quoting and escaping file names). We wouldn't document this stuff as part of "command completion", that doesn't really make sense I think. I will start working on a separate patch to try and improve the documentation for how file names are quoted / escaped currently, + Lancelot has highlighted an issue with filename completion within 'set' values, I haven't looked into that yet, but will do. Are there any concerns that I haven't addressed? I don't want to miss anything that's concerning you. Thanks, Andrew