Skip to content

programs.fish: add useBabelfish option - #3843

Closed
elohmeier wants to merge 1 commit into
nix-community:masterfrom
elohmeier:fish-babelfish
Closed

elohmeier wants to merge 1 commit into
nix-community:masterfrom
elohmeier:fish-babelfish

Conversation

@elohmeier

Copy link
Copy Markdown

Description

Added useBabelfish option to programs.fish. Speeds up fish startup noticeably. Similar implementation as in NixOS fish module.

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.

@elohmeier
elohmeier requested a review from rycee as a code owner April 3, 2023 19:36
@emilazy

emilazy commented May 24, 2023

Copy link
Copy Markdown
Contributor

See also: #4012

Sorry I didn't see this before writing mine...

@ncfavier ncfavier left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops, sorry I didn't see this PR!

Comment thread modules/programs/fish.nix
${if cfg.useBabelfish then ''
source ${
babelfishTranslate hm-session-vars "hm-session-vars"
} > /dev/null

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why > /dev/null?

@emilazy emilazy May 24, 2023 •

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 foreign-env version branch uses it (in home-manager, NixOS and nix-darwin); I don't know why. Maybe fenv prints some additional output? babelfish just does a straight translation so it shouldn't be necessary.

@emilazy

emilazy commented May 24, 2023

Copy link
Copy Markdown
Contributor

FWIW, this duplicates the generation of hm-session-vars.sh across home-environment.nix and fish.nix which I think is probably best to avoid. Otherwise no substantive difference between the current state of two PRs other than keeping the option or not.

@ncfavier

Copy link
Copy Markdown
Member

Superseded by #4012

@ncfavier ncfavier closed this May 31, 2023
@elohmeier
elohmeier deleted the fish-babelfish branch June 1, 2023 06:20
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