fish: suppress expected warnings in fish init on NixOS #27236
Conversation
|
@therealpxc, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jgillich, @rnhmjoj and @Profpatsch to be potential reviewers. |
There was a problem hiding this comment.
I really hate to be that guy, but this solution does not match the requirements I specified.
Now, it might be that others want to approve it, but all the issues resulting from this will be on them.
Why do you implement this, while it so clearly doesn't meet the requirements?
|
Thanks for your effort to fix this issue. I decided to merge the other pull request for the reason given here: #27227 (comment) |
| # `fenv` should not attempt to modify $_, but it seems to do so when | ||
| # used to run the bash `source` builtin. We don't need to hear about it, | ||
| # either. | ||
| set warnings_filter_pattern $warnings_filter_pattern"|"'|^set: Tried to change the read-only variable “_”$' |
There was a problem hiding this comment.
ok, I did not considered this one on the first spot. I will also look into it.
There was a problem hiding this comment.
would it not be easier to just use:
fenv 'source ${config.system.build.setEnvironment}; unset _'
instead here?
There was a problem hiding this comment.
I think something like that would work, if only because fenv doesn't correctly detect and address erased variables. If fenv did that, it would detect a change and still attempt to set (because $_ is always see to something initially). Should we go for it anyway?
|
I'm glad a solution was merged, even if I'm partial to the one I made just because I made it. It was good to be part of the discussion and to see fish support become smoother on NixOS. :-) |
|
@therealpxc Your solution is superior to the one currently merged. Perhaps @Mic92 will reconsider. I think @Mic92 is also optimizing for likeability in order to make the number of Nixpkgs contributors grow. It's difficult to say what's optimal behavior, but I would rather set the bar higher and be as consistent as possible. Thanks for playing regardless. |
Motivation for this change
it fixes the cosmetic issues related to #27195 by suppressing a specific, expected error message only when necessary
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)