-
Notifications
You must be signed in to change notification settings - Fork 405
fix: miscellaneous fixes after separation of directories #1536
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
base: main
Are you sure you want to change the base?
Changes from all commits
300d165
43ca26b
e9ac873
fd36105
48d41cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -176,18 +176,22 @@ A. [ Disclaimer: Here, how to make the completion code visible to | |
| Install it in one of the directories pointed to by bash-completion's | ||
| `pkgconfig` file variables. There are two alternatives: | ||
|
|
||
| - The recommended directory is `completionsdir`, which you can get with | ||
| - The recommended directory is `<completionsdir>`, which you can get with | ||
| `pkg-config --variable=completionsdir bash-completion`. From this | ||
| directory, completions are automatically loaded on demand based on invoked | ||
| commands' names, so be sure to name your completion file accordingly, and | ||
| to include (for example) symbolic links in case the file provides | ||
| completions for more than one command. The completion filename for | ||
| command `foo` in this directory should be either `foo`, or `foo.bash`. | ||
| (Underscore prefixed `_foo` works too, but is reserved for | ||
| bash-completion internal use as a deprecation/fallback marker.) | ||
| - The other directory which is only present for backwards compatibility, | ||
| its usage is no longer recommended, is `compatdir` (get it with | ||
| `pkg-config --variable=compatdir bash-completion`). From this | ||
| completions for more than one command. The completion filename for command | ||
| `foo` in this directory should be `foo.bash`. Unsuffixed `foo` also | ||
| works, but it is deprecated in >= 2.18. | ||
| - Helper scripts used by completions may be placed in the directory | ||
| `<helpersdir>`, which can be retrieved with `pkg-config | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not related to this particular line, but the first commit message refers to |
||
| --variable=helpersdir bash-completion`. The completion files in | ||
| `<completionsdir>` can reference the helper script `<helpersdir>/<helper>` | ||
| as `${BASH_SOURCE[0]%/*}/../helpers/<helper>`. | ||
| - The other directory, which is only present for backwards compatibility and | ||
| is not recommended to use, is `<compatdir>` (get it with | ||
| `pkg-config --variable=compatdir bash-completion`). From this | ||
| directory, files are loaded eagerly when `bash_completion` is loaded. | ||
|
|
||
| For packages using GNU autotools the installation can be handled | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||||||||
| # Third-party completions directory - bash-completion | ||||||||||||
|
|
||||||||||||
| This directory `completions` is the place to put **third-party completion | ||||||||||||
| files** that are loaded by | ||||||||||||
|
Comment on lines
+3
to
+4
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe clarify here (and in helpers) what we mean by "third party". There's also a mention of "external" in the note below. How about something like this to unify the terms?
Suggested change
|
||||||||||||
| [bash-completion](https://github.com/scop/bash-completion) on demand. | ||||||||||||
|
|
||||||||||||
| > [!NOTE] | ||||||||||||
| > | ||||||||||||
| > The core `bash-completion` framework (<= 2.17) has provided also *completion | ||||||||||||
| > files bundled with the bash-completion framework itself* in this directory, | ||||||||||||
| > but we separated those files into `completions-core` and | ||||||||||||
| > `completions-fallback`. However, external completion provider should | ||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| > continue to install their completion files in `completions`. The new | ||||||||||||
| > directories `completions-core` and `completions-fallback` are internal | ||||||||||||
| > directories intended to be used by the core `bash-completion`. | ||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| The programmable completion for the command `<cmd>` is supposed to be provided | ||||||||||||
| as the file `completions/<cmd>.bash`. The implementation examples are found in | ||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| `completions-core`, which uses the latest bash-completion API (v3). If you | ||||||||||||
| want to make the completion file to work also with an older version of | ||||||||||||
| `bash-completion`, you need to use the previous API of v2, which is contiuned | ||||||||||||
| to be supported. For examples with API v2, please reference the completion | ||||||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| files in `completions` of | ||||||||||||
| [bash-completion-2.11](https://github.com/scop/bash-completion/releases/tag/2.11). | ||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,4 @@ | ||
| helpersdir = $(datadir)/$(PACKAGE)/helpers | ||
| helpers_DATA = | ||
| helpers_DATA = README.md | ||
|
|
||
| EXTRA_DIST = $(helpers_DATA) | ||
|
|
||
| install-data-local: | ||
| $(MKDIR_P) $(DESTDIR)$(datadir)/bash-completion/$(subdir) | ||
|
|
||
| uninstall-local: | ||
| -rmdir $(DESTDIR)$(datadir)/bash-completion/$(subdir) |
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggesting similar changes here as in completions/README.md above. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Third-party helpers directory - bash-completion | ||
|
|
||
| This directory `helpers` is the place to put **third-party helper scripts** | ||
| that are used by third-party completion files in the `completions` directory, | ||
| written for [bash-completion](https://github.com/scop/bash-completion). | ||
|
|
||
| > [!NOTE] | ||
| > | ||
| > The core `bash-completion` framework (<= 2.17) has provided also *helper | ||
| > scripts included in the bash-completion framework itself* in this directory, | ||
| > but we separated those files into `helpers-core`. However, external | ||
| > completion provider should continue to install their helper scripts in | ||
| > `helpers`. The new directory `helpers-core` is an internal directory | ||
| > intended to be used by the core `bash-completion`. | ||
|
|
||
| The helper script may be referenced by a completion script in `completions` | ||
| with `${BASH_COMPLETION[0]%/*}/../helpers/<helper>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Remove double space after period, there are some more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is there a specific preference or a rule for double-space sentence separators? The double-space style is the tradition in typewriters, while a single space is more typical in modern text files.
The problem is that I use GNU Emacs, and GNU Emacs recognizes "a period plus double spaces" as the sentence terminator, while "a period plus a space" is treated as a period after an abbreviation (like Mr., Inc., Rev., etc), following the convention in typewriters. If the sentence-end double spaces are converted to single spaces, it affects the behavior of the auto line breaking at column 79. I don't want to manually convert all single spaces at the end of sentences to double spaces and convert back double spaces to single spaces every time I edit a paragraph and adjust line breaking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I look at the codebase, both styles are mixed. @scop also seems to use the double-space style (though I'm not sure about its frequency compared to that of the single-space style). For example, the message in the deprecated files
# Use of this file is deprecated. Upstream completion is available in(with double spaces) was introduced by @scop in e.g. 861be75.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have really have a specific preference, I'm just used to the single space. It's fine to leave it likes this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I've noticed this issue too, though I have never raised it in a discussion. It's a subtle issue about the conflict between the modern style and the text editor behavior, so I haven't tried to normalize the style either way. If both of you are fine with it, let me just leave it as is until an actual issue occurs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a long time Emacs user as well, and have had the habit of using two for ages, but I'm not paying that much attention to it these days, as not doing so does not interfere with anything I routinely do. So either style is fine with me, but I'd prefer consistency and even better if there's a linter or something that whines about the "wrong" one. Flagging double is quite a bit easier to implement if one wants to write one, and some are in existence already, for example https://github.com/Automattic/harper advises against using "French spaces" (i.e. the double ones).