Deprecate dpkg completion#1639
Conversation
|
Sorry, just noticed now that this was against the now stale master branch, will rebase tomorrow against main instead and force push. |
|
We've already moved those files so that no conflicts happen. This PR seems to rename non-existent files, though I'm not sure why GitHub interface doesn't report conflicts. You base on an old commit between 2.13.0--2.14.0. If you intend to submit a patch to a certain version packaged in a distribution, could you please submit the patch to the package maintainer? If you intend to submit it here, could you please rebase the branch on top of the latest |
guillemj
force-pushed
the
pu/upstream-dpkg
branch
from
c9290bb to
f3997df
Compare
Right, see my comment which seems crossed at the same time as yours. I've now rebased it and force pushed. |
guillemj
force-pushed
the
pu/upstream-dpkg
branch
from
f3997df to
1daa158
Compare
scop
left a comment
scop
left a comment
There was a problem hiding this comment.
Another partial review round here, will need to look into this some more still.
As a general note, it seems this PR combines a number of changes in one commit that could have been split to multiple standalone/followup commits. It would be much easier to review it if was submitted as multiple logical commits (could review one commit at a time).
Because the single commit description needs an update anyway (conventional commits, dpkg vs non-dpkg system considerations / some of these are not deprecated everywhere), maybe look into splitting it, too?
guillemj
force-pushed
the
pu/upstream-dpkg
branch
from
1daa158 to
e076c75
Compare
There are at least two major implementations for the alternatives system. The original one from dpkg called update-alternatives used by dpkg-based systems and other systems that extract the update-alternatives code from the dpkg repository, and the one from chkconfig called alternatives and also exposed via a symlink as update-alternatives used mainly by Red Hat originated distributions. These implementations have diverged in their interface, while they still share some common parts. Split the completions of the two to be able to clearer mark the alternatives one deprecated in favor of upstream one included in dpkg >= 1.23.8 on applicable systems.
So that it won't inadvertently disappear if upstream dpkg completion from dpkg >= 1.23.8 is installed and overrides the one we have -- dpkg-reconfigure completion is not included there.
Upstream completion is available in dpkg >= 1.23.8.
Upstream completion is available in dpkg >= 1.23.8. Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
Upstream one is included in dpkg >= 1.23.8. Because the dpkg completion provides public interfaces used by other completion scripts, the completion imported into dpkg will preserve those public functions for backwards compatibility for now. Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
scop
left a comment
scop
left a comment
There was a problem hiding this comment.
I split the completions somewhat further and looked through them, looks good to me. Adjusted commit messages while at that. Hopefully the 1.23.8 version timeline will hold :)
Approved as far as I'm concerned, but would not mind another look by @akinomyoga or @yedayak if you happen to have time. If not, I'll merge shortly (assuming CI passes).
|
|
||
| _comp_cmd_alternatives__installed() | ||
| { | ||
| local i admindir |
There was a problem hiding this comment.
For localvar_inherit, local variables need to be explicitly initialized.
| local i admindir | |
| local i admindir= |
| break | ||
| fi | ||
| done | ||
| [[ -d $admindir ]] && _comp_compgen_split -- "$(command ls "$admindir")" |
There was a problem hiding this comment.
This just lists the filenames, so there is no need to use the external executable ls. I thought _comp_compgen -C "$admindir" filedir would work, but I realized that it would also generate backslashes / when the word matches a directory name.
One may first call _comp_compgen_filedir with an empty cur and then filter the results:
| [[ -d $admindir ]] && _comp_compgen_split -- "$(command ls "$admindir")" | |
| if [[ -d $admindir ]]; then | |
| local installed | |
| _comp_compgen -Rv installed -C "$admindir" filedir && | |
| _comp_compgen -U installed -- -W '"${installed[@]}"' | |
| fi |
Or, actually, it may be simpler to use _comp_expand_glob.
| [[ -d $admindir ]] && _comp_compgen_split -- "$(command ls "$admindir")" | |
| if [[ -d $admindir ]]; then | |
| local installed | |
| _comp_expand_glob installed '"$admindir"/*' && | |
| _comp_compgen -U installed -- -W '"${installed[@]#"$admindir"}"' | |
| fi |
| *) | ||
| case $((args % 4)) in | ||
| 0 | 2) | ||
| _comp_compgen_filedir | ||
| ;; | ||
| 1) | ||
| _comp_compgen -- -W '--slave' | ||
| ;; | ||
| 3) | ||
| _comp_cmd_alternatives__installed | ||
| ;; |
There was a problem hiding this comment.
As far as I see the command-line parser of the redhat implementation, alternatives doesn't fix the orderings of the command-line options. In addition, there are extensions --family <1 arg>, --initscript <1 arg>, and --follower <3 args>. The orderings in the man page must be only for presentation.
We should explicitly see the nearest option name to generate the command line arguments.
3 participants
To make it possible for dpkg to provide its own bash completion, the support here needs to be namespaced. In addition, the support for dpkg-reconfigure (which is actually provided by debconf not dpkg), needs to be split.
Because the dpkg completion provides public interfaces used by other completion scripts, the completion imported into dpkg will preserve those public functions for backwards compatibility for now. Ideally bash-completions would switch those functions into another file. But that's left for later to be able to unblock the integration with the next dpkg upstream release 1.23.8.
Fixes: #694