Skip to content
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

fzf: 0.47.0 -> 0.49.0 #298692

Merged
merged 7 commits into from
Apr 7, 2024
Merged

fzf: 0.47.0 -> 0.49.0 #298692

merged 7 commits into from
Apr 7, 2024

Conversation

SebTM
Copy link
Contributor

@SebTM SebTM commented Mar 24, 2024

Description of changes

  • fzf: 0.47.0 -> 0.48.1
  • Update derivation to not install old auto-completions
  • Update module to load completion for bash, fish (new) and zsh (or oh-my-zsh plugin) with changed way through fzf-binary
  • Added change to release note as it is backwards-incompatible.

Superseeds #295978

Home-Manager module will need also an update (just as note here)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@bjornfor
Copy link
Contributor

Please split the 236-char commit message line into a subject (<=50 chars) + blank line + body (wrapped to <= 72 chars).

@@ -77,7 +77,7 @@ In addition to numerous new and updated packages, this release has the following

- [frigate](https://frigate.video), an open source NVR built around real-time AI object detection. Available as [services.frigate](#opt-services.frigate.enable).

- [fzf](https://github.com/junegunn/fzf), a command line fuzzyfinder. Available as [programs.fzf](#opt-programs.fzf.fuzzyCompletion).
- [fzf](https://github.com/junegunn/fzf), a command line fuzzyfinder. Available as [programs.fzf](#opt-programs.fzf.enable).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if that's ok but the manual seems to build against the current state not the state it was made so I get a build-error of the changelog if the old option is still referenced with #opt here. Alternative would be to remove the link "#opt" and leave it as text.

@SebTM
Copy link
Contributor Author

SebTM commented Mar 24, 2024

Please split the 236-char commit message line into a subject (<=50 chars) + blank line + body (wrapped to <= 72 chars).

Thanks for the feedback, all done 🙏🏻

@SebTM
Copy link
Contributor Author

SebTM commented Mar 24, 2024

Result of nixpkgs-review run on x86_64-linux 1

1 package blacklisted:
  • nixos-install-tools
50 packages built:
  • adl
  • ani-cli
  • arsenal
  • arsenal.dist
  • catcli
  • catcli.dist
  • clerk
  • felix-fm
  • fishPlugins.fzf-fish
  • fontpreview
  • fzf
  • fzf-git-sh
  • fzf-zsh
  • fzf.man
  • kakounePlugins.fzf-kak
  • kns
  • license-cli
  • lunarvim
  • mov-cli
  • mov-cli.dist
  • navi
  • ocamlPackages.fzf
  • ocamlPackages.magic-trace
  • python311Packages.pyfzf
  • python311Packages.pyfzf.dist
  • python312Packages.pyfzf
  • python312Packages.pyfzf.dist
  • ripgrep-all
  • spacevim
  • sway-launcher-desktop
  • sysz
  • tmuxPlugins.extrakto
  • tmuxPlugins.fuzzback
  • tmuxPlugins.session-wizard
  • tmuxPlugins.t-smart-tmux-session-manager
  • tmuxPlugins.tmux-fzf
  • unipicker
  • vimPlugins.fzf-hoogle-vim
  • vimPlugins.fzf-lua
  • vimPlugins.fzf-vim
  • vimPlugins.fzfWrapper
  • vimPlugins.telescope-zoxide
  • vimPlugins.vim-fzf-coauthorship
  • vimPlugins.vim-zettel
  • vimPlugins.zoxide-vim
  • xmloscopy
  • yazi
  • ytfzf
  • zoxide
  • zsh-forgit

Update derivation to not install old shell-completions
Update module to load completion for bash, fish (new) and zsh (or oh-my-zsh plugin) with changed way through fzf-binary
Added change to release note as it is backwards-incompatible.
@SebTM
Copy link
Contributor Author

SebTM commented Mar 24, 2024

Fixed issue where systemPackages was accidentally not a list ✌🏻

@marsam
Copy link
Contributor

marsam commented Apr 6, 2024

Would you mind updating it to fzf 0.49.0?

The fzf-share helper was added to help to find the shell scripts [1],
but since fzf≥0.48.0 the it's no longer needed because fzf has its shell
integration scripts embedded.

[1] NixOS#27709
Since fzf 0.43.0, the history command only uses Perl if it's installed
otherwise uses Awk [1].

[1] junegunn/fzf@9f7684f
@@ -30,7 +30,7 @@ in
'';

programs.fish.interactiveShellInit = ''
eval "$(${getExe pkgs.fzf} --fish)"
${getExe pkgs.fzf} --fish | source
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we do this for the others as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's just the fish setup that is different
image

@SebTM
Copy link
Contributor Author

SebTM commented Apr 7, 2024

Thanks for updating, I guess we need another round of review now before we can merge 🙏🏻
Will have a look soonTM ✌🏻

@marsam
Copy link
Contributor

marsam commented Apr 7, 2024

I'm going ahead and merge this, because another fzf bump was merged a few hours ago #301585, and it's likely to cause errors with the fzf module.

Thank you for working on this!

@marsam marsam merged commit 5d58ded into NixOS:master Apr 7, 2024
25 of 26 checks passed
@SebTM SebTM deleted the auto-update/fzf branch April 8, 2024 13:57
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Apr 10, 2024

Update derivation to not install old auto-completions

The shell scripts are just embedded (https://github.com/junegunn/fzf/blob/master/main.go#L15-L28) using go's inbuilt embed feature.
So we lost options here for nothing and we should probably revert this to easily disable completion or keybindings again.

For completion I don't think there is any way to disable them completely https://github.com/junegunn/fzf/blob/master/shell/completion.bash

@SebTM
Copy link
Contributor Author

SebTM commented Apr 10, 2024

Didn't check that as it wasn't an issue for me and I stated it in the MR that it comes with this compromise. I would prefer if both are enabled to still use the official integration which also have the advantage if they don't just print them in the future e.g. use substitution/placeholders the official integration will keep working.

@SebTM SebTM changed the title fzf: 0.47.0 -> 0.48.1 fzf: 0.47.0 -> 0.49.0 Apr 10, 2024
SuperSandro2000 added a commit to SuperSandro2000/nixpkgs that referenced this pull request Apr 10, 2024
SuperSandro2000 added a commit to SuperSandro2000/nixpkgs that referenced this pull request Apr 10, 2024
@SuperSandro2000
Copy link
Member

We can just add an option for that junegunn/fzf#3722 :)

#303213

@SuperSandro2000
Copy link
Member

I've changed #303213 according to upstream feedback and half of the changes done here are being reverted because there is no reason for them and upstream recommend to load the shell files individually, if you want more control, so it seems like there is no urge to switch to the new format which makes removing options unjustified. Please take a look at that PR asap.

SuperSandro2000 added a commit to SuperSandro2000/nixpkgs that referenced this pull request Apr 11, 2024
SuperSandro2000 added a commit to SuperSandro2000/nixpkgs that referenced this pull request Apr 11, 2024
SuperSandro2000 added a commit to SuperSandro2000/nixpkgs that referenced this pull request Apr 11, 2024
@Ma27 Ma27 mentioned this pull request Apr 11, 2024
SebTM added a commit that referenced this pull request Apr 11, 2024
nixos/fzf: bring back keybindings and completion option removed in #298692
SuperSandro2000 added a commit to SuperSandro2000/nixpkgs that referenced this pull request Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants