diff options
Diffstat (limited to 'CONTRIBUTING.md')
| -rw-r--r-- | CONTRIBUTING.md | 93 |
1 files changed, 69 insertions, 24 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9c8292c..d19b5cd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,15 +28,23 @@ your completion to be usable without having bash-completion installed. It's nowhere near clear cut always what is the best place for the completion, upstream project or us. Even if it would seem to be upstream, not all upstreams are interested in shipping completions, or their install systems might not -easily support installing completion files properly. But give it some thought, -and ask if unsure. +easily support installing completion files properly. Or the projects might be +stagnant. But give it some thought, and ask if unsure. If you wish to contribute code to us, volunteering for long term maintainership -of your code within bash-completion is welcome. When exactly you will be asked -to do that depends on the case; don't be disappointed if it does or doesn't -happen instantly. +of your code within bash-completion is welcome, and stating willingness for +that goes a long way in getting your contribution accepted. There are a lot of +completions in bash-completion already, and chances are that existing +maintainers might not want to add completions they don't actively use +themselves into their maintenance workload. When exactly you will be asked to +join the project depends on the case; there are no real, consistent "rules" for +that. Don't be disappointed if it does or doesn't happen instantly. -Also, please bare the following coding guidelines in mind: +Also, please bear the following coding guidelines in mind: + +- 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 @@ -59,11 +67,11 @@ Also, please bare 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 @@ -100,23 +108,30 @@ Also, please bare 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 expansion (e.g. `${varname-}`). -- Prefer `compgen -W '...' -- $cur` over embedding `$cur` in external - command arguments (often e.g. sed, grep etc) unless there's a good - reason to embed it. Embedding user input in command lines can result - in syntax errors and other undesired behavior, or messy quoting - requirements when the input contains unusual characters. Good - reasons for embedding include functionality (if the thing does not - sanely work otherwise) or performance (if it makes a big difference - in speed), but all embedding cases should be documented with - rationale in comments in the code. +- Prefer `_comp_compgen_split -- "$(...)"` over embedding `$cur` in external + command arguments (often e.g. sed, grep etc) unless there's a good reason to + embed it. Embedding user input in command lines can result in syntax errors + and other undesired behavior, or messy quoting requirements when the input + contains unusual characters. Good reasons for embedding include + functionality (if the thing does not sanely work otherwise) or performance + (if it makes a big difference in speed), but all embedding cases should be + documented with rationale in comments in the code. + + Do not use `_comp_compgen -- -W "$(...)"` or `_comp_compgen -- -W '$(...)'` + but always use `_comp_compgen_split -- "$(...)"`. In the former case, when + the command output contains strings looking like shell expansions, the + expansions will be unexpectedly performed, which becomes a vulnerability. In + the latter case, checks by shellcheck and shfmt will not be performed inside + `'...'`. Also, `_comp_compgen_split` is `IFS`-safe. - When completing available options, offer only the most descriptive ones as completion results if there are multiple options that do the @@ -131,7 +146,7 @@ Also, please bare the following coding guidelines in mind: `--something` do the same thing and require an argument, offer only `--something` as a completion when completing option names starting with a dash, but do implement required argument processing for all - `-s`, `-S`, and `--something`. Note that GNU versions of various + `-s`, `-S`, and `--something`. Note that GNU versions of various standard commands tend to have long options while other userland implementations of the same commands may not have them, and it would be good to have the completions work for as many userlands as @@ -149,6 +164,17 @@ Also, please bare the following coding guidelines in mind: - Make small, incremental commits that do one thing. Don't cram unrelated changes into a single commit. +- We use [Conventional Commits](https://www.conventionalcommits.org/) + to format commit messages, with types and most other details from + [commitlint's config-conventional](https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional). + `gitlint` in our pre-commit config checks commit messages for + conformance with these rules. + + It is important to do this correctly; commit types `fix` and `feat` + as well as any change marked as breaking affects what ends up in the + release notes, and what will the next bash-completion release's + (semantic) version be. + - If your code was written for a particular platform, try to make it portable to other platforms, so that everyone may enjoy it. If your code works only with the version of a binary on a particular @@ -181,6 +207,25 @@ Also, please bare the following coding guidelines in mind: - In addition to running the test suite, there are a few scripts in the test/ dir that catch some common issues, see and use for example runLint. +- Make sure you have Python 3.7 or later installed. This is required for + running the development tooling, linters etc. Rest of the development + Python dependencies are specified in `test/requirements-dev.txt` which + can be fed for example to `pip`: + + ```shell + python3 -m pip install -r test/requirements-dev.txt + ``` + +- Install pre-commit and set it up, see <https://pre-commit.com/>. + That'll run a bunch of linters and the like, the same as the + bash-completion CI does. Running it locally and fixing found issues before + commit/push/PR reduces some roundtrips with the review. + After installing it, enable it for stages we use it with like: + + ```shell + pre-commit install --hook-type pre-commit --hook-type commit-msg + ``` + - File bugs, enhancement, and pull requests at GitHub, <https://github.com/scop/bash-completion>. Sending them to the developers might work too, but is really strongly |