Hi Ville, On Mon, 18 Sep 2023 22:29:58 +0300, Ville Skyttä wrote: > On Mon, 18 Sept 2023 at 15:56, Jean Delvare <jdelv...@suse.de> wrote: > > > 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. > > Done. > > I'm attaching the revised changeset as a single patch attachment here, hope > that's ok (my git/send-email-fu isn't up to dealing with followups > properly). This new one includes also scraping of --string and --type > values as discussed in another message.
A new patch is fine, as I have not committed anything yet. > > +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. > > True. Then again if the prefix or lookup dirs of bash-completion are > actually different from our $(prefix), installing in our $(prefix) will > cause the completions to not be found. I think this is the best we can do. It really depends on how bash completion scripts are handled by the system. For example, on my openSUSE system, scripts can be installed in both /usr/share/bash-completion/completions/ and /etc/bash_completion.d/. I can imagine that some distributions would also look for completions in /usr/local/share/bash-completion/completions/ or /usr/local/etc/bash_completion.d/, the same way /usr/local/bin is part of the PATH on most distributions. On my system, it is also possible to set the environment variable BASH_COMPLETION_COMPAT_DIR to an arbitrary location which contains bash completion scripts (although I'm unsure if this possibility will persist, given its name). So I could decide to set it to /usr/local/share/bash-completion/completions on my account, so that locally installed bash completion scripts are taken into account. Anyway, independently of where bash completion files must reside to be taken into account, I consider "make install" writing outside of $(DESTDIR)$(prefix) by default as not acceptable. While the user is free to overwrite $(compdir) if he/she wants to, it should be within $(prefix) by default, to avoid silently overwriting existing system files. I'm not aware of any other package doing things the way you suggest (i.e. installing files outside of $(DESTDIR)$(prefix) by default). Another thing I'm worried about is the portability of the $(shell) call. So far I tried to ensure that the Makefile is compatible with alternative implementations such as bmake. This is valuable to non-Linux users. Now to the completion scripts themselves. I gave them a try. Seems to work well, but I found one bug: the trailing "n" of string names is eaten during completion: # dmidecode --string <tab> baseboard-asset-tag chassis-asset-tag processor-versio baseboard-manufacturer chassis-manufacturer system-family baseboard-product-name chassis-serial-number system-manufacturer baseboard-serial-number chassis-type system-product-name baseboard-versio chassis-versio system-serial-number bios-release-date firmware-revisio system-sku-number bios-revisio processor-family system-uuid bios-vendor processor-frequency system-versio bios-versio processor-manufacturer # dmidecode --string chassis-ver<tab> completes to "chassis-versio" I suppose this is a mistake in the post-processing of the "dmidecode --string" error output. I'm willing to implement the "--string ?" option we discussed previously if it makes it easier to collect the available string names. -- Jean Delvare SUSE L3 Support
