Remove shebang to bash completion file

[elboulangero]

Bash completion files are meant to be sourced, not executed, hence there
should be no shebang at the beginning of the file.

For reference, this is what Lintian (Debian tool to find errors in
packages) says about it (see [1]):

This file starts with the #! sequence that marks interpreted scripts,
but it is a bash completion script that is merely intended to be
sourced.

Please remove the line with hashbang, including any designated
interpreter.

On my Debian system, I looked at the other bash completions scripts, and
indeed the utmost majority has no shebang:

$ pwd
/usr/share/bash-completion/completions

$ ls -1 | wc -l
1077

$ grep '^#!' * | wc -l
15

Hence this commit removes the shebang, and in order to preserve
indentation and highlighting in common code editors, it adds two hints:

• an Emacs "file variable" on the first line ('-- ... --'), see [2]
• a Vim modeline on the last line ('ex: '), see [3]

Once again, one can look at other bash completion files in
/usr/share/bash-completion to see that these 2 hints are present in the
majority of the files.

[1] https://lintian.debian.org/tags/bash-completion-with-hashbang.html
[2] https://www.gnu.org/software/emacs/manual/html_node/emacs/Specifying-File-Variables.html
[3] https://vi.stackexchange.com/a/11558

[albers]

I do not see much benefits in removing the hashbang aside from making the Lintian linter happy.
Do you know of any security risks associated with this finding?

On a Ubuntu 18.04 based system, vim offers substantly decreased syntax highlighting after your change.
This is perhaps due to the fact that there are multiple shells, so "shell script" is not specific enough.

And there are other editors and viewers aside from vim end emacs. For example, GoLand will not recognize the hasbangless versions as bash scripts.

[elboulangero]

I do not see much benefits in removing the hashbang aside from making the Lintian linter happy.

Yes indeed, this is a nitpick, but if you're into nitpicks like me, this commit is the right thing to do!! :) Because a shebang line is for executable scripts, not for files that are meant to be sourced.

Do you know of any security risks associated with this finding?

No.

And there are other editors and viewers aside from vim end emacs. For example, GoLand will not recognize the hasbangless versions as bash scripts.

I understand your point. I don't know of any better solution than what I proposed here, ie. "emacs file variable" and "vim modeline". Not surprising if other editors don't read or interpret these hints though. If contributors lose syntax highlighting due to this change, it's not great :/

So that was just a nitpick. Feel free to close the issue. If ever I'm aware of a better solution in the future I might pass by again :)

[albers]

Let's get more opinions on this.
@thaJeztah @tianon , WDYT about hashbangs in completion scripts?

[thaJeztah]

I see shellcheck currently fails because of the missing shebang, so looks like the linters are somewhat in disagreement 😂

scripts/validate/shellcheck

In contrib/completion/bash/docker line 1:
# bash completion for docker                                -*- shell-script -*-
^-- SC2148: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

For more information:
  https://www.shellcheck.net/wiki/SC2148 -- Tips depend on target shell and y...
make: *** [Makefile:85: shellcheck] Error 1

That link (https://github.com/koalaman/shellcheck/wiki/SC2148) suggests;

Add a shebang line to the top of your script:

#!/bin/bash

Or, for scripts that will not be executed (e.g., ~/.bashrc), use a directive:

# shellcheck shell=bash

If neither of those options are possible or desirable, you can invoke ShellCheck with the --shell switch:

shellcheck --shell=sh without-shebang.sh

The shellcheck directive could've been an alternative, however, I gave that a quick test, and at least my editor (Golang / Intellij) does not look to take the # shellcheck shell=xx directive into account for code highlighting; not sure if other editors do take these into account?

So, while I can see that having a shebang is not needed for this script to use it, it does provide substantial benefits (currently) to help when editing / validating in editors.

Also a bit on the fence on adding editor-specific directives in files. I understand vim and emacs can be popular choices, but other options may as well (VSCode, Intellij, Goland), and I know we tried to keep editor-specific options out of the code, e.g. see

cli/.gitignore

Lines 1 to 2 in 55451d3

	# if you want to ignore files created by your editor/tools,
	# please consider a global .gitignore https://help.github.com/articles/ignoring-files

. shellcheck is a bit of an exception there, because it's used as part of our tests/CI.

I'm open to alternative suggestions though

Also wondering if the lintian linter allows for specific checks to be ignored (e.g. a nolint directive?)

[elboulangero]

Also wondering if the lintian linter allows for specific checks to be ignored (e.g. a nolint directive?)

The way to ignore Lintian messages is to add a "lintian override" file to the Debian packaging files. It's also straightforward to just patch the shebang out of the file to make Lintian happy. So it's no big deal really.

[tianon]

IMO, the shebang is a useful (standard) signifier of the file's language, whether it's executed as-is or not -- I personally always include a shebang on any Bash file, even if it's only ever sourced. I think the fact that getting proper syntax highlighting back requires two additional declarations is a pretty compelling case unto itself.

On a separate note, with my Debian hat on, the only things I ever commit a real lintian override for are things that are auto-rejects. Otherwise, I typically use https://nthykier.wordpress.com/2012/02/23/some-sponsors-are-evil-and-pedantic/ (the syntax of which might have changed slightly since that was written) to ensure that there's enough stuff that isn't worth fixing (or in many cases doesn't even make sense to fix, like this one) that I can't possibly dream of being "lintian clean", which is a dubious goal anyhow.

[albers]

Looks like the hashbang here provides substantial benefits for developers and maintainers that outweigh functional purity.

[thaJeztah]

Agreed.

Thanks for contributing @elboulangero, and sorry we didn't accept this one! (it's still appreciated though!)