Skip to content

Add fish shell completions - #1860

Merged
faern merged 1 commit into
mullvad:masterfrom
leowzukw:fish-completions
Jun 26, 2020
Merged

Add fish shell completions#1860
faern merged 1 commit into
mullvad:masterfrom
leowzukw:fish-completions

Conversation

@cljoly

@cljoly cljoly commented Jun 20, 2020

Copy link
Copy Markdown
Contributor

Generate fish shell completions at build time. Empower fish users with
completions.

Completions are copied in the recommended directory.

Describe what this PR changes. Why this is wanted. And, if needed, how it does it.

Git checklist:


This change is Reviewable

@faern
faern self-requested a review June 24, 2020 08:48

@faern faern left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes look good! Thank you for this contribution.

I have some nitpicks about the changelog that I would like you to fix.

The git history is also currently in a form we can't merge. We do not merge master into feature branches and then back. Please rebase the feature branch on latest master and squash commits in such a way that the feature branch has no merge commits. This small feature can preferably just be a single commit.

Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md
Generate fish shell completions at build time. Empower fish users with
completions.

Completions are copied in the [recommended directory](https://fishshell.com/docs/current/#where-to-put-completions)
@cljoly

cljoly commented Jun 24, 2020

Copy link
Copy Markdown
Contributor Author

CHANGELOG.md, line 28 at r2 (raw file):

Previously, faern (Linus Färnstrand) wrote…

My bad! It's for two platforms. So keeping it under the generic header is fine. It could be extended to say "on Linux and macOS" but that's just nitpick. I don't think fish exists for Windows anyway? Resolved.

Yes, no windows support for fish: fish-shell/fish-shell#319

@cljoly
cljoly requested a review from faern June 24, 2020 19:23
@cljoly

cljoly commented Jun 24, 2020

Copy link
Copy Markdown
Contributor Author

Thanks for your review. I think I have addressed your comments.

@faern

faern commented Jun 25, 2020

Copy link
Copy Markdown
Contributor

Thank you very much. I will merge this asap.

@cljoly

cljoly commented Jun 25, 2020

Copy link
Copy Markdown
Contributor Author

Awesome!

@faern
faern merged commit 143208d into mullvad:master Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants