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

zsh-abbr: adding completions + removing redudant bin directory #348162

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

Icy-Thought
Copy link
Contributor

@Icy-Thought Icy-Thought commented Oct 12, 2024

Things done

  1. Renamed the installed plugin to zsh-abbr.plugin.zsh because most plugins share that structure and automating the installation process would be kinder if we omit this exception.
  2. Install completions/_abbr, which allows completions for the abbr command in zsh. Only benefits people who are not creating their abbreviation file through nix.
  3. Rewrite comment a little. (unnecessary but I think it is better written this way? (subjective I know))
  4. Remove the $out/bin because it only contains the changelog file. Not necessary for using the package.

Also, it should resolve #348110.

  • 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.11 Release Notes (or backporting 23.11 and 24.05 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.

@Icy-Thought Icy-Thought changed the title Zsh abbr zsh-abbr: adding completions + removing redudant bin directory Oct 12, 2024
@Icy-Thought
Copy link
Contributor Author

@Lewenhaupt test out this PR and tell me if it works for you. :)

@Lewenhaupt
Copy link

The rename of the plugin breaks the home-manager module which relies on the old naming.

@Icy-Thought
Copy link
Contributor Author

Icy-Thought commented Oct 13, 2024

That we can fix after the merger of this PR since it's not a nix issue, but a home-manager one. (more like an exception too)

@Lewenhaupt
Copy link

That we can fix after the merger of this PR since it's not a nix issue, but a home-manager one. (more like an exception too)

But won't that break the home-manager module?

@Icy-Thought
Copy link
Contributor Author

I'll update the home-manager module to make sure that it works with after this PR merger. Apologies for the confusion earlier, got lost in my train of thoughts.

pkgs/shells/zsh/zsh-abbr/default.nix Outdated Show resolved Hide resolved
pkgs/shells/zsh/zsh-abbr/default.nix Outdated Show resolved Hide resolved
@Icy-Thought
Copy link
Contributor Author

Icy-Thought commented Oct 18, 2024

I might've done something funny because of magit. Be right back with a fix if necessary. (in a learning phase still...)

@Icy-Thought
Copy link
Contributor Author

The issues should be fixed now. Do ping me if not!

Comment on lines 22 to 23
install zsh-abbr.plugin.zsh zsh-abbr.zsh -Dt $out/share/zsh/${pname}/
install completions/_abbr -Dt $out/share/zsh/${pname}/completions/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
install zsh-abbr.plugin.zsh zsh-abbr.zsh -Dt $out/share/zsh/${pname}/
install completions/_abbr -Dt $out/share/zsh/${pname}/completions/
install zsh-abbr.plugin.zsh zsh-abbr.zsh -Dt $out/share/zsh/zsh-abbr/
install completions/_abbr -Dt $out/share/zsh/zsh-abbr/completions/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this a trivial change? Since it already outputs the files to that exact directory.

@Icy-Thought
Copy link
Contributor Author

At this point it's rewriting the whole installPhase -> one unified commit.

@Aleksanaa Aleksanaa merged commit 73a23a5 into NixOS:master Oct 29, 2024
28 checks passed
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.

zsh-abbr does not include completions
5 participants