-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
nix-profile{,-daemon}.fish: Do not source twice, fmt #12252
nix-profile{,-daemon}.fish: Do not source twice, fmt #12252
Conversation
@ilya-bobyr Thanks for you pull request. However can you make the formatting changes a separate commit? That will make it easier to review it. |
c1683a9
to
ce22be7
Compare
@Mic92 Thank you for looking at the change. I've split the formatting change into a separate commit. Or do you want the formatting to be a separate PR? P.S. I have 8 more commits on top, to remove all inconsistencies between |
Both commits in one PR is fine. |
`nix-profile.fish` and part of `nix-profile-daemon.fish` use 4 space indentation. Which is also the indentation that the fish shell documentation is using. Reformatting a chunk of `nix-profile-daemon.fish` from 2 space indentation to 4 space indentation for consistency.
In order for the script not be sourced multiple times by the same shell instance, `__ETC_PROFILE_NIX_SOURCED` needs to be set with a `--global` flag. Both files are almost identical. And style differences make it harder to see what is actually different and keep them in sync, when it is required.
ce22be7
to
b36637c
Compare
Rebased to fix an unrelated test failure in |
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 63c0ea5 |
Motivation
nix-profile-daemon.fish
andnix-profile.fish
scripts perform almost identical actions.Yet, they are written with slight differences that make it harder to see what is actually the same and what is different.
This is already discussed in #9441
I think
nix-profile-daemon.fish
andnix-profile.fish
might eventually turn out to be identical or use a common implementation.This change is a step is a step in this direction.
You can see the differences between
nix-profile-daemon.fish
andnix-profile.fish
here:https://gist.github.com/ilya-bobyr/c957a9f998b85833064b2d64b4ae4062
Context
A few bug fixes and indentation changes on both sides:
nix-profile-daemon.fish
:When checking for repeated execution, exit before the helper function
add_path
is defined. Making sure it does not leak to the caller.Check that
$HOME
is set, as it is actually used in the configuration.Use 4 space indentation in the configuration subsection, as this is the indentation used for the rest of the file and in
nix-profile.fish
.Mark
__ETC_PROFILE_NIX_SOURCED
as--global
as, by default,set
creates a--local
variable that might not be visible to a subsequent execution of this script in the same shell. Effectively breaking the protection.nix-profile.fish
:Use early exit when checking for required environment variables and double execution.
Identical to the
nix-profile-daemon.fish
.Use
__ETC_PROFILE_NIX_SOURCED
for double execution protection, similarly tonix-profile-daemon.fish
.Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.