Hi Ville, On Sat, 22 Jul 2023 22:55:31 +0300, Ville Skyttä wrote: > Offer only long options, but recognize short ones, too, per rationale at > https://github.com/scop/bash-completion/blob/4d0bffb791c34c96114aeb2e4f6726b80aa8698e/CONTRIBUTING.md?plain=1#L136-L153 > > Signed-off-by: Ville Skyttä <ville.sky...@iki.fi> > --- > Makefile | 14 ++++++-- > biosdecode.bash | 40 +++++++++++++++++++++ > dmidecode.bash | 95 +++++++++++++++++++++++++++++++++++++++++++++++++ > ownership.bash | 33 +++++++++++++++++ > vpddecode.bash | 45 +++++++++++++++++++++++
Please create a directory for completion files, the root directory of the project is already cluttered enough as is. > 5 files changed, 225 insertions(+), 2 deletions(-) > create mode 100644 biosdecode.bash > create mode 100644 dmidecode.bash > create mode 100644 ownership.bash > create mode 100644 vpddecode.bash > > diff --git a/Makefile b/Makefile > index 7aa729d..66dc2d9 100644 > --- a/Makefile > +++ b/Makefile > @@ -36,6 +36,7 @@ sbindir = $(prefix)/sbin > mandir = $(prefix)/share/man > man8dir = $(mandir)/man8 > docdir = $(prefix)/share/doc/dmidecode > +compdir = $(shell pkg-config --variable=completionsdir bash-completion > 2>/dev/null || echo $(prefix)/etc/bash_completion.d) This is going to potentially install files outside of $(prefix) by default, which is questionable. > > INSTALL := install > INSTALL_DATA := $(INSTALL) -m 644 > @@ -113,9 +114,9 @@ util.o : util.c types.h util.h config.h > strip : $(PROGRAMS) > strip $(PROGRAMS) > > -install : install-bin install-man install-doc > +install : install-bin install-man install-doc install-completion I'm worried about this being enabled by default while the system may not even have bash installed. Can we enable this by default only if $(compdir) exists? This directory would typically be owned by the bash package. > > -uninstall : uninstall-bin uninstall-man uninstall-doc > +uninstall : uninstall-bin uninstall-man uninstall-doc uninstall-completion > > install-bin : $(PROGRAMS) > $(INSTALL_DIR) $(DESTDIR)$(sbindir) > @@ -144,5 +145,14 @@ install-doc : > uninstall-doc : > $(RM) -r $(DESTDIR)$(docdir) > > +install-completion : > + $(INSTALL_DIR) $(DESTDIR)$(compdir) > + for program in $(PROGRAMS) ; do \ > + $(INSTALL_DATA) $$program.bash $(DESTDIR)$(compdir)/$$program ; done > + > +uninstall-completion : > + for program in $(PROGRAMS) ; do \ > + $(RM) $(DESTDIR)$(compdir)/$$program ; done > + > clean : > $(RM) *.o $(PROGRAMS) core > diff --git a/biosdecode.bash b/biosdecode.bash > new file mode 100644 > index 0000000..42e0fae > --- /dev/null > +++ b/biosdecode.bash > @@ -0,0 +1,40 @@ > (...) I'm not going to review the completion scripts themselves, I haven't dealt with such files for a long time so I wouldn't know what to look for. But I'll install them on my system to give them some testing. Thanks, -- Jean Delvare SUSE L3 Support
