Skip to content

nushell: deprecation of let-env#4292

Merged
ncfavier merged 1 commit intonix-community:masterfrom
happysalada:nushell_let_env
Aug 1, 2023
Merged

nushell: deprecation of let-env#4292
ncfavier merged 1 commit intonix-community:masterfrom
happysalada:nushell_let_env

Conversation

@happysalada
Copy link
Copy Markdown
Contributor

@happysalada happysalada commented Aug 1, 2023

Description

let-env has been deprecated in nushell's latest release.
This PR aims to fix the errors that this causes.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all or nix develop --ignore-environment .#all using Flakes.

  • 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.

Maintainer CC

@happysalada happysalada force-pushed the nushell_let_env branch 3 times, most recently from c042bb0 to 7dc0ab2 Compare August 1, 2023 04:01
@happysalada
Copy link
Copy Markdown
Contributor Author

happysalada commented Aug 1, 2023

Adding a bit of context here.
The code added for zoxide and atuin is just a kludge until those packages update their init script. It will be backward compatible, so I think we can leave that kludge for a while after the packages have been update to be sure no one has problems.

While these fixes will work for version 0.83+ , I don't think they will work with earlier version, so perhaps this shouldn't be merged ? I'm opening this PR more as a discussion and to help other people with the problem as well.

@happysalada
Copy link
Copy Markdown
Contributor Author

happysalada commented Aug 1, 2023

here is where I found the fix ajeetdsouza/zoxide#599

@ncfavier
Copy link
Copy Markdown
Member

ncfavier commented Aug 1, 2023

Does this fix #4291 ?

@ncfavier ncfavier mentioned this pull request Aug 1, 2023
Closed
2 tasks
ncfavier
ncfavier reviewed Aug 1, 2023
View reviewed changes
Comment thread modules/programs/atuin.nix Outdated
mkdir $atuin_cache
}
${cfg.package}/bin/atuin init nu ${flagsStr} | save --force ${config.xdg.cacheHome}/atuin/init.nu
${cfg.package}/bin/atuin init nu ${flagsStr} | str replace --string --all 'let-env ' '$env.' | save --force ${config.xdg.cacheHome}/atuin/init.nu
Copy link
Copy Markdown
Member

@ncfavier ncfavier Aug 1, 2023

Choose a reason for hiding this comment

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

I'd rather this be fixed in nixpkgs by applying atuinsh/atuin#1080

ncfavier
ncfavier reviewed Aug 1, 2023
View reviewed changes
Comment thread modules/programs/zoxide.nix Outdated
mkdir $zoxide_cache
}
${cfg.package}/bin/zoxide init nushell ${cfgOptions} | save --force ${config.xdg.cacheHome}/zoxide/init.nu
${cfg.package}/bin/zoxide init nushell ${cfgOptions} | str replace --string --all 'let-env ' '$env.' | save --force ${config.xdg.cacheHome}/zoxide/init.nu
Copy link
Copy Markdown
Member

@ncfavier ncfavier Aug 1, 2023

Choose a reason for hiding this comment

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

I'd rather this be fixed in nixpkgs by applying ajeetdsouza/zoxide#587

@happysalada
Copy link
Copy Markdown
Contributor Author

happysalada commented Aug 1, 2023

Yes this does fix the issue you referenced.

I agree with the two comments you made. What do we do in the meanwhile though ? My idea behind the fixes on zoxide and atuin is that they can always be removed after the packages have been updated, even after the update, the fix wont break anything. Another release of both packages might be weeks away, thats why i suggested those in the meanwhile.

Let me know what you would rather do. Personally I would tend to leave the two fixes in at least until a release is made for both packages.

@ncfavier
Copy link
Copy Markdown
Member

ncfavier commented Aug 1, 2023

I meant applying both patches in nixpkgs so that we don't have to wait for releases.

@happysalada
Copy link
Copy Markdown
Contributor Author

happysalada commented Aug 1, 2023

I've asked both of the upstream repos for a release (it might not happen, but I thought that it was worth trying at least).
I'm leaving this as is for now, so people can just use it if their environment is broken. If there are no answers in a week, it makes sense to apply the patches manually in nixpkgs.
Thank you for the quick review!

@jcdickinson
Copy link
Copy Markdown

jcdickinson commented Aug 1, 2023

Can we get the home-manager-specific fixes merged in any form?

@happysalada
Copy link
Copy Markdown
Contributor Author

happysalada commented Aug 1, 2023

Alright, ive removed the fixes for zoxide and atuin. It should be ready to merge.

Btw the creator of atuin said she would cut a release this weekend,.

@jcdickinson
Copy link
Copy Markdown

jcdickinson commented Aug 1, 2023

Thanks @happysalada!

@ncfavier ncfavier merged commit 8c73197 into nix-community:master Aug 1, 2023
@happysalada happysalada deleted the nushell_let_env branch August 2, 2023 03:19
SimSaladin added a commit to SimSaladin/home-manager that referenced this pull request Aug 26, 2023
@SimSaladin
Merge remote-tracking branch 'upstream/master' into develop
532f650 
* upstream/master: (352 commits)
  flake.lock: Update
  qcal: add module
  bat: generate cache file in XDG cache home
  nixos: only refer to `users.users` if needed
  home-cursor: remove IFD when linking icon directories
  flake.lock: Update
  home-cursor: improve icon compatibility
  fix(qt): allow theming for apps started by systemd (nix-community#4349)
  flake.lock: Update
  fcitx5: add `fcitx5-config-qt` to test stub
  pqiv: only run tests on Linux platforms
  pqiv: add module
  Translate using Weblate (German)
  xplr: add module
  browserpass: support librewolf
  Translate using Weblate (Romanian)
  Translate using Weblate (Portuguese (Brazil))
  hyprland: remove xwayland.hidpi (nix-community#4302)
  home-manager: handle profile list in Nix >2.17
  lf: simplify option validation (nix-community#4334)
  programs.khal: uncomment locale config (nix-community#4319)
  kanshi: support adaptive sync (nix-community#4328)
  mu: add package option (nix-community#4325)
  modules: types.string throws error now (nix-community#4324)
  taffybar: Avoid restarting too quickly (nix-community#4316)
  aerc: do not use smtp-starttls (nix-community#4272)
  zsh: Add `zsh.history.ignoreAllDups` config option (nix-community#4248)
  jujutsu: fix zsh completion (nix-community#4305)
  Translate using Weblate (Korean)
  Translate using Weblate (Romanian)
  Translate using Weblate (German)
  flake.lock: Update
  network-manager-applet: remove `--sm-disable` flag
  himalaya: update a link to ticket
  nushell: deprecation of let-env (nix-community#4292)
  man: fix caches generation in cross-compiled system (nix-community#4294)
  fish: fix session vars build in cross-compiled system (nix-community#4293)
  Update translation files
  Translate using Weblate (Chinese (Simplified))
  gh: option to enable helper for additional hosts (nix-community#4288)
  home-manager: rework news command
  home-manager: skip manual in uninstall configuration
  gnome-terminal: add assertion on profile names
  hyprland: use `toKeyValue`'s `indent` argument (nix-community#4274)
  gpg: fix typo (nix-community#4277)
  firefox: make package nullable (nix-community#4113)
  git-sync: add news entry for darwin
  tmate: don't generate empty config file (nix-community#4271)
  gh-dash: add module
  git-sync: add darwin support
  flake.lock: Update
  hyprland: prioritize variables and beziers (nix-community#4263)
  hyprland: add module
  script-directory: fix documentation link (nix-community#4258)
  Translate using Weblate (Indonesian)
  Translate using Weblate (Ukrainian)
  Translate using Weblate (Spanish)
  Translate using Weblate (Swedish)
  docs: hide `_module.*` from NixOS/nix-darwin docs
  jujutsu: update for Jujutsu 0.8.0 (nix-community#4250)
  ripgrep: don't set env. variable if no config (nix-community#4254)
  docs: update for Markdown migration
  treewide: remove now-redundant `lib.mdDoc` calls
  docs: clean up after Markdown migration
  flake: remove temporary nixpkgs pin
  treewide: convert all option docs to Markdown
  treewide: adjust some DocBook for conversion
  treewide: `mkAliasOption` -> `mkAliasOptionMD`
  treewide: `mkPackageOption` -> `mkPackageOptionMD`
  treewide: convert options with tables to Markdown
  treewide: convert options with lists to Markdown
  treewide: convert custom `enable` docs to Markdown
  treewide: convert parameterized docs to Markdown
  treewide: manually convert some docs to Markdown
  treewide: fix `mkEnableOption` arguments
  treewide: add missing option descriptions
  docs: use `nixosOptionsDoc`
  flake: temporarily pin nixpkgs version
  version: add `isReleaseBranch`
  manual: add test
  Update translation files
  flake.lock: Update
  home-manager: remove Home Manager default paths
  imapnotify: Use JSON type for extraConfig (nix-community#4238)
  Translate using Weblate (French)
  zsh: fix custom syntax highlighting styles (nix-community#4236)
  i3-sway: allow arbitrary floating modifier (nix-community#4229)
  i3-sway: multiple outputs (nix-community#4223)
  home-cursor.nix: enable gtk module when enabling gtk config generation (nix-community#4144)
  aerc: add assertion to limit per-account extraConfig to UI config (nix-community#4196)
  flake.lock: Update
  ssh-agent: add assertion and fix news entry (nix-community#4210)
  imapnotify: move test
  imapnotify: use direct nix store path for config
  flake.lock: Update
  nushell: add login.nu configuration option
  himalaya: fix notmuch backend
  swayosd: add module
  pyenv: add module
  darcs: add module
  tests: some minor cleanups
  goimapnotify: remove test dependency on notmuch
  unison: Allow using same option multiple times (nix-community#4208)
  imapnotify: Add launchd agent (nix-community#3291)
  i3: remove deprecated example in i3.config.startup (nix-community#4201)
  antidote: static file move to /tmp
  Translate using Weblate (Indonesian)
  PULL_REQUEST_TEMPLATE.md: add maintainer cc section (nix-community#4193)
  programs.khal
  vdirsyncer: synchronize metadata as well (nix-community#4167)
  i18n: Use glibcLocales from NixOS if possible (nix-community#2333) (nix-community#4177)
  xfconf: remove properties with null values (nix-community#4192)
  home-manager: Use `path:` URI type for flake default (nix-community#3646)
  ci: autolabel calendars PRs (nix-community#4165)
  flake.lock: Update (nix-community#4161)
  news: fix typo in programs.zsh.antidote entry
  ssh-agent: init module (nix-community#4178)
  chromium: fix `commandLineArgs` to use the user specified package (nix-community#4175)
  sxhkd: allow usage of derivations as keybind commands (nix-community#4169)
  README: Remove the pills (nix-community#4181)
  lsd: use -A instead of -a in aliases (nix-community#4173)
  kakoune: add defaultEditor option
  docs: update link to allowed users setting (nix-community#4176)
  zsh: allow setting custom syntax highlighting styles (nix-community#4122)
  flake: add formatter (nix-community#3620)
  broot: fix test (nix-community#4170)
  antidote: fix .dot path
  ci: build manual and push to home-manager.dev
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ncfavier ncfavier ncfavier approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@happysalada @ncfavier @jcdickinson