[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#57673: [PATCH] Parse --help messages for pcomplete
From: |
Stefan Monnier |
Subject: |
bug#57673: [PATCH] Parse --help messages for pcomplete |
Date: |
Thu, 08 Sep 2022 16:49:03 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) |
> +;;; 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
- bug#57673: [PATCH] Parse --help messages for pcomplete, Augusto Stoffel, 2022/09/08
- bug#57673: [PATCH] Parse --help messages for pcomplete,
Stefan Monnier <=
- bug#57673: [PATCH] Parse --help messages for pcomplete, Augusto Stoffel, 2022/09/08
- bug#57673: [PATCH] Parse --help messages for pcomplete, Stefan Monnier, 2022/09/08
- bug#57673: [PATCH] Parse --help messages for pcomplete, Augusto Stoffel, 2022/09/10
- bug#57673: [PATCH] Parse --help messages for pcomplete, Stefan Monnier, 2022/09/10
- bug#57673: [PATCH] Parse --help messages for pcomplete, Augusto Stoffel, 2022/09/10
- bug#57673: [PATCH] Parse --help messages for pcomplete, Augusto Stoffel, 2022/09/14
- bug#57673: [PATCH] Parse --help messages for pcomplete, Lars Ingebrigtsen, 2022/09/14
- bug#57673: [PATCH] Parse --help messages for pcomplete, Augusto Stoffel, 2022/09/14
- bug#57673: [PATCH] Parse --help messages for pcomplete, Lars Ingebrigtsen, 2022/09/14
- bug#57673: [PATCH] Parse --help messages for pcomplete, Augusto Stoffel, 2022/09/14