Skip to content

fish: remove promptInit in favor of interactiveShellInit#2231

Merged
sumnerevans merged 5 commits intonix-community:masterfrom
kidonng:patch-3
Oct 27, 2021
Merged

fish: remove promptInit in favor of interactiveShellInit#2231
sumnerevans merged 5 commits intonix-community:masterfrom
kidonng:patch-3

Conversation

@kidonng
Copy link
Copy Markdown
Contributor

@kidonng kidonng commented Jul 29, 2021

Description

promptInit does nothing special and can already be implemented via interactiveShellInit, so I suggest just keep the latter.

However there are some things I'm not too sure:

  • This may break some people's config. I don't know the process of deprecating an option in Home Manager, so help welcome.
  • There is already a programs.fish.interactiveShellInit (which "add completions generated by Home Manager to $fish_complete_path") config, but is it a default value? Will it take effect if users provide a custom interactiveShellInit? If that's the case then it should be fixed (and the code looks horrible to me, so we may also have a chance to rewrite it at the same time).

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@kidonng kidonng requested a review from rycee as a code owner July 29, 2021 11:58
@kidonng kidonng marked this pull request as draft July 29, 2021 11:58
Comment thread modules/programs/fish.nix
@stale
Copy link
Copy Markdown

stale bot commented Oct 27, 2021

Thank you for your contribution! I marked this pull request as stale due to inactivity. If this remains inactive for another 7 days, I will close this PR. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously, when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the issue

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Oct 27, 2021
@kidonng
Copy link
Copy Markdown
Contributor Author

kidonng commented Oct 27, 2021

Bump, this is still relevant.

@stale stale bot removed the status: stale label Oct 27, 2021
Copy link
Copy Markdown
Member

@berbiche berbiche left a comment

Choose a reason for hiding this comment

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

LGTM I guess

@sumnerevans sumnerevans merged commit 9282dbc into nix-community:master Oct 27, 2021
betaboon added a commit to betaboon/home-manager that referenced this pull request Oct 28, 2021
danth added a commit to nix-community/stylix that referenced this pull request Oct 28, 2021
peterhoeg pushed a commit to peterhoeg/home-manager that referenced this pull request Nov 4, 2021
…munity#2231)

* fish: remove `promptInit` in favor of `interactiveShellInit`

* Oops

* starship: replace `promptInit` with `interactiveShellInit`

* autojump: replace `promptInit` with `interactiveShellInit`

* Add `mkRemovedOptionModule` for `programs.fish.promptInit`
peterhoeg pushed a commit to peterhoeg/home-manager that referenced this pull request Nov 4, 2021
yvt added a commit to yvt/dotfiles that referenced this pull request Nov 28, 2021
…veShellInit`

This change was necessitated by the following PR:
<nix-community/home-manager#2231>
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.

3 participants