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 16:49:03 -0400 Message-ID: References: <87zgfagqfs.fsf@gmail.com> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="2483"; 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 Thu Sep 08 22:50:47 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 1oWOU3-0000Ul-4O for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 08 Sep 2022 22:50:47 +0200 Original-Received: from localhost ([::1]:48614 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oWOU1-0006dw-Gm for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 08 Sep 2022 16:50:45 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:60232) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oWOTK-0006d1-MA for bug-gnu-emacs@gnu.org; Thu, 08 Sep 2022 16:50:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:43347) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oWOTK-0001JS-CO for bug-gnu-emacs@gnu.org; Thu, 08 Sep 2022 16:50:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oWOTK-0006BG-7Q for bug-gnu-emacs@gnu.org; Thu, 08 Sep 2022 16:50: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: Thu, 08 Sep 2022 20:50: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.166267015723588 (code B ref 57673); Thu, 08 Sep 2022 20:50:02 +0000 Original-Received: (at 57673) by debbugs.gnu.org; 8 Sep 2022 20:49:17 +0000 Original-Received: from localhost ([127.0.0.1]:60238 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oWOSb-00068O-Be for submit@debbugs.gnu.org; Thu, 08 Sep 2022 16:49:17 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:6118) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oWOSW-000687-Jt for 57673@debbugs.gnu.org; Thu, 08 Sep 2022 16:49:15 -0400 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id C9CE944226A; Thu, 8 Sep 2022 16:49:05 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 13775442269; Thu, 8 Sep 2022 16:49:04 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1662670144; bh=ku+cFJhj7DkwRK/i1KRESYXMkASv9x5Hw4NAuDieW4s=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=pTvcv2plNiakRGr/3R/9OadlE/CytWCbxmAChsfJiaoi8Yws7/eEV481rXRqiDr/H 0MSVxcaDKdmOtJ3I0Bc+3cZC/LRMq/tt3PuSRifOCl6LaO8Jqmt+6CGtPhuBj1YwV6 gf2VzkWlMe4rGXfmpBhI0H9MzlfLF/fawvyqQjuFHBzjJUYp8GsTmQppoDBlholWv3 kvboErIjMZ43Z84iMO7aovGRPZ7ruQ61fngdnk49G+3gKNqaKxPzdXWsn7D9f0Drh8 8MJgswyrgcb07CJqxZ80/ZJqWy3eDCitERUIDom0ggpj6tDiudDemPfwnEdMDxcplS ppII0tKBivzOg== Original-Received: from lechazo (lechon.iro.umontreal.ca [132.204.27.242]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 005071202AD; Thu, 8 Sep 2022 16:49:03 -0400 (EDT) In-Reply-To: <87zgfagqfs.fsf@gmail.com> (Augusto Stoffel's message of "Thu, 08 Sep 2022 11:34:31 +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:241944 Archived-At: > +;;; Commentary: > + I don't fully agree with you here. > +(defun pcmpl-git--tracked-file-predicate () > + "Return a predicate function determining if a file is tracked by git." I'd capitalize "git". > + (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. > +(defun pcmpl-git--remote-refs (remote) > + "List the locally known git revisions from REMOTE. > +If REMOTE is nil, return the list of remotes." > + (if (null remote) AFAICT you could just as well have two separate functions here since all callers either always provide a nil arg or never provide a nil arg. > + (ignore-errors > + (process-lines vc-git-program "remote")) > + (delq nil > + (mapcar > + (let ((re (concat (regexp-quote remote) "/\\(.*\\)"))) > + (lambda (s) (when (string-match re s) (match-string 1 s)))) > + (vc-git-revision-table nil))))) I think the `re` needs to be anchored to avoid false positives. > +;;;###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)? Maybe we should follow a scheme similar to that used in `pcomplete-cvs`, tho it'd probably require a new replacement for `pcomplete-opt` > + (let ((subcmd (pcomplete-arg 1))) > + (while (pcase subcmd > + ((guard (pcomplete-match "\\`-" 0)) > + (pcomplete-here > + (pcmpl-git--expand-flags > + (pcomplete-parse-help (format "git help %s" subcmd) > + :argument "-+\\(?:\\[no-\\]\\)?[a-z-]+=?")))) `subcmd` may contain funny chars like `&`, so we should `shell-quote-argument` it (tho see further down). > + ;; Complete tracked files > + ((or "mv" "rm" "restore" "grep" "status" "commit") > + (pcomplete-here > + (pcomplete-entries nil (pcmpl-git--tracked-file-predicate)))) Regarding `git commit`: - I never specify files on `git commit` without using `--` first. - I often appreciate the completion of `--amend`. - It probably makes sense to only complete files that have been modified. > +(cl-defun pcomplete-parse-help (command AFAICT, this "command" never uses any functionality of the shell, so we should be using `call-process` rather than `call-process-shell-command`, since the use of a shell here only brings trouble (it's less efficient and it forces us to `shell-quote-arguments` to try and avoid pathological errors in corner cases). Stefan