PR#1354: Add shell completions section - packaging-committee

packaging-committee

#1354 Add shell completions section

Opened a month ago by principis. Modified a month ago

principis/packaging-committee shell-completions into master

Arthur Bols • a month ago

c9414e3

file modified

		`@@ -79,6 +79,7 @@`
		`** xref:LibreOfficeExtensions.adoc[LibreOffice Extensions]`
		`** xref:MinGW.adoc[MinGW]`
		`** xref:MPI.adoc[MPI]`
		`+ ** xref:ShellCompletions.adoc[Shell Completions]`
		`** xref:SugarActivityGuidelines.adoc[Sugar activities]`
		`** xref:Web_Assets.adoc[Web Assets]`
		`** xref:WordPress_plugin_packaging_guidelines.adoc[WordPress extensions]`

guidelines/modules/ROOT/pages/ShellCompletions.adoc

file added

+48

		`@@ -0,0 +1,48 @@`
		`+ = Shell Completions`
		`+`
		`+ Shell completions are command line completions for a specific shell,`
		`+ such as Bash, fish or Zsh.`
		`+`
		`+ == RPM Macros`
		`+`
		`+ The following macros MUST be used instead of hardcoding paths.`
		`+`
		`+ [cols=",",options="header"]`
		`+ \|======================================================================`
		`+ \| macro \| definition`
		`+ \| +%{bash_completions_dir}+ \| +%{_datadir}/bash-completion/completions+`
		`+ \| +%{fish_completions_dir}+ \| +%{_datadir}/fish/vendor_completions.d+`
		`+ \| +%{zsh_completions_dir}+ \| +%{_datadir}/zsh/site-functions+`
		`+ \|======================================================================`
		`+`
		`+`
		`+ == Shell Completions Packaging`
		`+`
		`+ Shell completion files MUST use standard file permissions (0644).`
		`+`
		`+ === Example of shell completions packaging`
		`+`
		`+ [source, rpm-spec]`
		`+ ----`
		`+ Name: foo`
		`+ ...`
		`+`
		`+ %install`
		`+ ...`
		`+`
		`+ # Bash completion`
		`+ install -Dpm 0644 foo.bash -t %{buildroot}%{bash_completions_dir}`
		`+`
		`+ # Fish completion`
		`+ install -Dpm 0644 foo.fish -t %{buildroot}%{fish_completions_dir}`
		`+`
		`+ # Zsh completion`
		`+ install -Dpm 0644 _foo -t %{buildroot}%{zsh_completions_dir}`
		`+ ...`
		`+`
		`+ %files`
		`+ %{bash_completions_dir}/foo.bash`
		`+ %{fish_completions_dir}/foo.fish`
		`+ %{zsh_completions_dir}/_foo`
		`+`
		`+ ----`

principis commented a month ago

Adds section for shell-completions and a MUST on using the macros as approved(I think?) here.

decathorpe commented a month ago

Looks pretty good to me, thanks!

Bikeshedding nit-pick: Technically %{buildroot} has a trailing slash, so there doesn't need to be an additional slash between %{buildroot} and %{foo_completions_dir}, but this is somewhat an issue of consistency and / or taste. It certainly looks clearer (at least to me) if the additional slash is present.

gotmax23 commented a month ago

Thanks for working on this! I would remove the extra slashes that Fabio mentioned. Also, I am not a fan of the Recommends: bash-completion. I don't think that was discussed in the parent issue and is not common practice.

Edited a month ago by gotmax23

tibbs commented on line 25 of guidelines/modules/ROOT/pages/ShellCompletions.adoc a month ago

Is this intended to be a requirement (MUST), or something lesser like SHOULD or even just "consider adding Recommends: bash-completion"?

decathorpe commented a month ago

I agree, the Recommends for bash-completion weren't discussed anywhere, and I don't think it is necessary.

principis commented a month ago

Thanks for the remarks!

Removed the slash after %{buildroot}
Removed reference to bash-completion

2 new commits added

`Remove unnecessary/after%{buildroot}```
Remove reference to `bash-completion` package

a month ago

3 new commits added

Remove unnecessary / after %{buildroot}
Remove reference to `bash-completion` package
Add shell completions section

a month ago

principis commented a month ago

Force pushed because pagure wasn't happy with the backticks

tibbs commented on line 22 of guidelines/modules/ROOT/pages/ShellCompletions.adoc a month ago

It's worth at least thinking about whether this should simply be a MUST. Is there any use case at all for executable permissions, or something less accessible like 0600?

gotmax23 commented on line 37 of guidelines/modules/ROOT/pages/ShellCompletions.adoc a month ago

Nit: I would sort the shells alphabetically in the examples and table (bash, fish, zsh).

tibbs commented a month ago

And finally, could you squash these commits so the history doesn't have the unwanted sections added and then removed?

rebased onto 98e76c0

a month ago

rebased onto 4f759d7

a month ago

principis commented a month ago

Done, thanks!

Changed should -> MUST 0644
Sorted shells
squashed commits

tibbs commented a month ago

Thanks. I'll +1 here in case we don't get to meet this week

Metadata Update from @tibbs:
- Pull-request tagged with: meeting

a month ago

decathorpe commented a month ago

LGTM. +1

Not sure if the formatting in the table is correct though.

tibbs commented a month ago

The table appears to render correctly for me when I check out the PR and build locally. At least it renders no differently than the table in KDE Packaging or Naming. I guess most of what's in there could all be monospaced since we not unusual to do that for macros or pathnames, but we don't really do that consistently throughout the guidelines.

rebased onto c9414e3

a month ago

principis commented a month ago

I forgot the second column. Not sure how that happened, fixed it.

decathorpe commented a month ago

Perfect, thanks!