Skip to content

doc: add rules for quoting #986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ that. Don't be disappointed if it does or doesn't happen instantly.

Also, please bear the following coding guidelines in mind:

- See the [API and naming](doc/api-and-naming.md) document for information
about conventions to follow related to those topics.
- See the related documents, [API and naming](doc/api-and-naming.md) and
[Coding style guide](doc/styleguide.md), for information about conventions to
follow related to those topics.

- Do not use Perl, Ruby, Python etc. to do text processing unless the
command for which you are writing the completion code implies the
Expand All @@ -66,11 +67,11 @@ Also, please bear the following coding guidelines in mind:
external programs, which are expensive to fork and execute, so do
make full use of those:

`?(pattern-list)` - match zero or one occurrences of patterns
`*(pattern-list)` - match zero or more occurrences of patterns
`+(pattern-list)` - match one or more occurrences of patterns
`@(pattern-list)` - match exactly one of the given patterns
`!(pattern-list)` - match anything except one of the given patterns
- `?(pattern-list)` - match zero or one occurrences of patterns
- `*(pattern-list)` - match zero or more occurrences of patterns
- `+(pattern-list)` - match one or more occurrences of patterns
- `@(pattern-list)` - match exactly one of the given patterns
- `!(pattern-list)` - match anything except one of the given patterns

- Following on from the last point, be sparing with the use of
external processes whenever you can. Completion functions need to be
Expand Down Expand Up @@ -107,9 +108,10 @@ Also, please bear the following coding guidelines in mind:

- We want our completions to work in `posix` and `nounset` modes.

Unfortunately due to a bash < 5.1 bug, toggling POSIX mode interferes
with keybindings and should not be done. This rules out use of
process substitution which causes syntax errors in POSIX mode.
Unfortunately due to a bash < 5.1 bug, toggling POSIX mode
interferes with keybindings and should not be done. This rules out
use of process substitution which causes syntax errors in POSIX mode
of bash < 5.1.

Instead of toggling `nounset` mode, make sure to test whether
variables are set (e.g. with `[[ -v varname ]]`) or use default
Expand Down
59 changes: 59 additions & 0 deletions doc/styleguide.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,62 @@ it.
## Function and variable names

See [API and naming](api-and-naming.md).

## Quoting of words

To avoid unexpected word splitting and pathname expansions, an argument of
commands needs to be properly quoted when it contains shell expansions such as
`$var`, `$(cmd)`, and `$((expr))`.

When one intentionally wants word splitting and pathname expansions, one should
consider using the utility functions provided by bash-completion. To safely
split a string without being affected by non-standard `IFS` and pathname
expansions, use the shell function `_comp_split`. To safely obtain filenames
by pathname expansions without being affected by `failglob`, etc., use the
shell function `_comp_expand_glob`. Note that `_comp_expand_glob` should be
always used for the pathname patterns even if the pattern does not contain
shell expansions.

In the following contexts, the quoting to suppress word splitting and pathname
expansions are not needed.

- The right-hand sides of variable assignments ... `v=WORD` (e.g. `v=$var`)
- The arguments of conditional commands ... `[[ WORD ]]` (e.g. `[[ $var ]]`)
- The argument specified to `case` statement ... `case WORD in foo) ;; esac`
(e.g. `case $var in foo) ;; esac`)

In bash-completion, we do not quote them by default. However, there are
exceptions where the quoting is still needed for other reasons.

- When the word *directly* contains shell special characters (space, tab,
newline, or a character from ``;|&()<>\\$`'"#!~{``), these characters need to
be quoted. The "*directly*" means that the special characters produced by
shell expansions are excluded here. For example, when one wants to include a
whitespace as a part of the value of the word, the right-hand side can be
quoted as `v="a b"`.
- An empty word (i.e., the word whose value is an empty string) is specified by
`""`. The right-hand side of an assignment technically can be an empty
string as `var=`, but we still use `var=""` there because `shellcheck`
suggests that e.g. `var= cmd` is confusing with `var=cmd`.
- `$*` and `${array[*]}` need to be always quoted because they can be affected
by the word splitting in bash <= 4.2 even in the above contexts.
- In the following contexts, double-quoting of shell expansions is needed
unless the result of expansions is intentionally treated as glob patterns or
regular expressions.
- The right-hand sides of `==`, `!=`, and `=~` in the conditional commands
... `[[ word == "$var" ]]`
- The case patterns ... `case word in "$var") ;; esac`

Note: Here strings `cat <<<$var` are also supposed to be safe against word
splitting and pathname expansions without quoting, but bash <= 4.3 has a bug
[1], so they need to be quoted for as long as we support bash 4.3.

- [koalaman/shellcheck#1009 (comment)](https://github.com/koalaman/shellcheck/issues/1009#issuecomment-488395630)

There are also preferences on the type of quoting, which are though not too
strict. We prefer to use double quotes over single quotes by default. When
the value contains `$`, `` ` ``, `\`, and `"`, we can use single quotes to
avoid backslash escaping or use the one that minimizes the use of backslash
escaping. When the value contains control characters such as a tab and a
newline, we do not directly include them but we use backslash escape sequences
such as `\t` and `\n` in the escape string `$'...'`.