From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#57673: [PATCH] Parse --help messages for pcomplete Date: Thu, 08 Sep 2022 22:47:26 -0400 Message-ID: References: <87zgfagqfs.fsf@gmail.com> <87sfl1leip.fsf@gmail.com> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="23964"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: 57673@debbugs.gnu.org To: Augusto Stoffel Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Sep 09 04:48:12 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oWU3v-00065M-Tt for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 09 Sep 2022 04:48:12 +0200 Original-Received: from localhost ([::1]:55588 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oWU3u-0002pM-Ci for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 08 Sep 2022 22:48:10 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36200) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oWU3n-0002ox-LF for bug-gnu-emacs@gnu.org; Thu, 08 Sep 2022 22:48:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:43659) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oWU3m-0006xI-QI for bug-gnu-emacs@gnu.org; Thu, 08 Sep 2022 22:48:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oWU3m-0006dF-Ac for bug-gnu-emacs@gnu.org; Thu, 08 Sep 2022 22:48:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 09 Sep 2022 02:48:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 57673 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 57673-submit@debbugs.gnu.org id=B57673.166269166025368 (code B ref 57673); Fri, 09 Sep 2022 02:48:02 +0000 Original-Received: (at 57673) by debbugs.gnu.org; 9 Sep 2022 02:47:40 +0000 Original-Received: from localhost ([127.0.0.1]:60591 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oWU3P-0006b3-RI for submit@debbugs.gnu.org; Thu, 08 Sep 2022 22:47:40 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:49439) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oWU3L-0006aj-5v for 57673@debbugs.gnu.org; Thu, 08 Sep 2022 22:47:39 -0400 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 5696A4422D7; Thu, 8 Sep 2022 22:47:29 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id AAC544422D1; Thu, 8 Sep 2022 22:47:27 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1662691647; bh=gQBNHkI+WUWnx5s68Wrlc5LSAzZzeUlkUlax0L9vMgE=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=Ta+QHoyJKHr/cgfgf1ADNbEu3mTF4aZ2FUbmwcIbRVBoFpkAsgMN81+jMBTCp4AH/ qCyGt+5+tnV5uDaMD3+0kQMQV9xvr7YXxvm47b5ZPNpPP07gTZzTCAebNChb1hgcey hGLzmngKDG55fcnVNMToN1XO5aFY2uIiWZcudImuYOsnBLjlA8mMSx1JTc3fFqoXYK txRJ2ln/c4VuUDkO3GMsXbYuBoMnzRniBCfPYt+C1fMbxydAdGVMYpVjmM0S+5Qxw2 h7bs6CVJ8dt57NrgOS+mCLtbQU88YjQX1KPzkXqZA7mB45XZA4jTSzn8UCTEuGQloQ +1Uz75kW9NWDw== Original-Received: from pastel (unknown [157.52.9.190]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 5CD56120639; Thu, 8 Sep 2022 22:47:27 -0400 (EDT) In-Reply-To: <87sfl1leip.fsf@gmail.com> (Augusto Stoffel's message of "Thu, 08 Sep 2022 23:53:02 +0200") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:241950 Archived-At: Augusto Stoffel [2022-09-08 23:53:02] wrote: > thanks for the detailed comments. I was also curious, on a more high > level, about your opinion on parsing the --help messages. It's a bit of > a heuristic thing and it will probably lead to some false positives here > and there. I'm all for using `--help`. I think any problems should be pushed upstream: the commands should be self-describing so if `--help` can't be used reliably, they should provide something else that can. >>> +;;; Commentary: >>> + >> I don't fully agree with you here. > I thought this was pretty uncontroversial, but I don't feel too strongly > about it. [ OK, you got me at my own game: I can't tell if you're pushing the joke further or if you're serious :-) ] >>> + (when-let ((files (ignore-errors >>> + (process-lines vc-git-program "ls-files"))) >> >> Is it normal&common for `ls-files` to return an error? If not, then >> maybe we should use `with-demoted-errors` so that the users are made >> aware of an error if it occurs. > > I think that being outside of a repo is the main error situation. But > in this particular spot I'd rather fail silently, because the > consequence of an error is rather innocuous -- we'll lack a predicate > function and therefore show some extra files completions. Fair, enough, tho being outside of a repo should be rather unusual, no (unless Emacs's dir tracker has gone hooftracks, admittedly). In any case, you might want to add a brief comment about why you decided to use `ignore-errors`. >>> +;;;###autoload >>> +(defun pcomplete/git () >>> + "Completion for the `git' command." >>> + (let ((subcmds (pcomplete-parse-help "git help -a" >>> + :margin "^\\( +\\)[a-z]" >>> + :argument "[-a-z]+"))) >>> + (while (not (member (pcomplete-arg 1) subcmds)) >>> + (pcomplete-here (append subcmds >>> + (pcomplete-parse-help "git help" >>> + :margin "\\(\\[\\)= -" >>> + :separator " | " >>> + :description "\\`"= )))) >> >> I don't quite understand this `while` loop. IIUC this is to handle the >> case where `--foo` args are passed before the `subcmd`, but if we want >> to support that use case, don't we need to change the code so it doesn't >> hardcode the `1` in (pcomplete-arg 1)? > > The thing here is that each call to `pcomplete-here' shifts the args. > So `1' in this context means =E2=80=9Cthe previous arg=E2=80=9D. Oh, right it's not the absolute position, now I get it, thanks. Looks good, then. >>> + ;; Complete tracked files >>> + ((or "mv" "rm" "restore" "grep" "status" "commit") >>> + (pcomplete-here >>> + (pcomplete-entries nil (pcmpl-git--tracked-file-predi= cate)))) >> >> Regarding `git commit`: >> - I never specify files on `git commit` without using `--` first. > > Well, `--' is among the completion candidates. Is that what you want? No, I'm just not sure we should be completing file names before a "--" was given. Maybe it's OK, tho. >> - I often appreciate the completion of `--amend`. > You can complete switches after `git commit'. This is taken care of by > the (guard (pcomplete-match "\\`-" 0)) case of the pcase. Indeed, I missed that. >> - It probably makes sense to only complete files that have been modified. > So, this is brings up an important point. > I'm advocating for a =E2=80=9Cworse is better=E2=80=9D method where we tr= y to have a > more or less comprehensive list of completion candidates but we don't > try to be too precise about the context in which each thing can appear. Fair enough. Especially since being more precise risks missing some files which can legitimately appear. Stefan