NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.3k stars 14.27k forks source link

zsh-abbr does not include completions #348110

Closed Lewenhaupt closed 3 weeks ago

Lewenhaupt commented 1 month ago

Describe the bug

zsh-abbr package does not include completions file in the install.

Steps To Reproduce

Steps to reproduce the behavior:

  1. add zsh-abbr package
  2. check if completions are present in the store
  3. see that completions are missing. Only the abbr.plugin.zsh file is present

Expected behavior

abbr completions should be available in the store such that it can be manually sourced or automatically sourced via the home-manager program for zsh-abbr.

Notify maintainers

@icy-thought

Metadata

Icy-Thought commented 1 month ago

Are you sure you are sourcing the right file because when I updated the latest package and sourced it to my zsh, the completions worked just fine.

Icy-Thought commented 1 month ago

Since my zsh config directory is in ~/.config/zsh, placing abbreviations file in that directory is enough. But I can try to rebuild and re-source + test the abbreviations for the sake of it.

Lewenhaupt commented 1 month ago

Hmm, what I thought was odd was that the zsh-abbr package only installs the zsh-abbr.zsh file from https://github.com/olets/zsh-abbr and completions are located under a folder which I'm now unable to locate. There's also a zsh-abbr.plugin.zsh which I think is zsh-abbrs own intended way of sourcing it into zsh but I can't find that file either. I might entirely be missing something obvious though, but from what I can tell I'm unable to find the completions folder/file.

Icy-Thought commented 1 month ago

How are you installing the zsh plugin btw? Also what does abbr list return?

Lewenhaupt commented 1 month ago

I'm installing it via the zsh home-manager module so

programs.zsh = {
  zsh-abbr = {
    enable = true;
  };
};

abbr list works as expected but I have no completions for the abbreviations. In zsh-abbr they add the completions directory for fpath using fpath+=${0:A:h}/completions but I can't find that in the store (which I suspect is because the zsh-abbr package does not install it).

Icy-Thought commented 1 month ago

Was not aware hat someone wrote an HM module for the plugin, cool! Also, do you find a file in ~/.config/zsh-abbr/user-abbreviations? Because that is where the HM module is saving the abbreviations.

Icy-Thought commented 1 month ago

Also what does echo $ABBR_USER_ABBREVIATIONS_FILE return?

Lewenhaupt commented 1 month ago

It returns my abbreviation file. But that's not the issue. abbr is working as intended, but the zsh-abbr nix package only includes the function and man-page, not the completions file from the repository.

Lewenhaupt commented 1 month ago

I did my own derivation in my flake and this is what I came up with. Basically just adding some more files from zsh-abbr such that I can add completions to fpath. Now I get completions for all my zsh-abbr abbreviations :)

{
  stdenv,
  lib,
  fetchFromGitHub,
}:
stdenv.mkDerivation rec {
  pname = "zsh-abbr-v2";
  version = "5.8.3";

  src = fetchFromGitHub {
    owner = "olets";
    repo = "zsh-abbr";
    rev = "v${version}";
    hash = "sha256-Kl98S1S4Ds9TF3H1YOjwds38da++/5rpgO/oAfKwRrc=";
  };

  strictDeps = true;

  installPhase = ''
    install -D zsh-abbr.zsh $out/share/zsh/${pname}/abbr.plugin.zsh
    install -D zsh-abbr.zsh $out/share/zsh/${pname}/zsh-abbr.zsh
    install -D zsh-abbr.plugin.zsh $out/share/zsh/${pname}/zsh-abbr.plugin.zsh
    mkdir -p $out/share/zsh/${pname}/completions
    install -D completions/_abbr $out/share/zsh/${pname}/completions/_abbr
    # Needed so that `man` can find the manpage, since it looks via PATH
    mkdir -p $out/bin
    mv man $out/share/man
  '';

  meta = with lib; {
    homepage = "https://github.com/olets/zsh-abbr";
    description = "Zsh manager for auto-expanding abbreviations, inspired by fish shell";
    license = with licenses; [
      cc-by-nc-nd-40
      hl3
    ];
    maintainers = with maintainers; [ icy-thought ];
    platforms = platforms.all;
  };
}
Lewenhaupt commented 1 month ago

Also I think I misunderstood part of the completions, they are only for abbr commands, not for the abbreviations.

Icy-Thought commented 1 month ago

I mistook your intention with the word "completion" as in completion of abbreviations. The reason why I did not include that bit was because I'd assumed people would want to nixify their abbreviations (mistake) and therefore that file won't be overwritten.

Guess I can make a PR to update the package to include it.

Icy-Thought commented 1 month ago

Now that you mention the code, I think we can omit the bin creation since it only contains the changelog file.

Lewenhaupt commented 1 month ago

Which file do you mean get overwritten? The completions/_abbr file?

Icy-Thought commented 1 month ago

Nope, since using the abbr commands it will try to modify your user_abbreviation file and when you try to create that file through nix it becomes an impossible feat. Hence adding the completions/_abbr is redundant. But then I forgot some people don't like to nixify everything.

Lewenhaupt commented 1 month ago

Oh right yeah that makes sense actually. I'm still in the beginning of my nix journey hehe.

Icy-Thought commented 1 month ago

Welcome aboard on this long and wonderful journey! xD Something that you could benefit from, you don't need to mkdir when using install -D. ;)

Icy-Thought commented 1 month ago

Also since you are installing the package through the home-manager module, how did the module fail to pick up the completions directory? Because home-manager does not follow the package structure that I have defined in the installPhase, but it does its own thing.

Lewenhaupt commented 1 month ago

Welcome aboard on this long and wonderful journey! xD Something that you could benefit from, you don't need to mkdir when using install -D. ;)

Haha thanks! Yeah I thought I shouldn't after reading the manual, I must have done something else wrong when I couldn't get it to work first hehe. I mean it didn't "fail" per-se, since the file wasn't include in the package.