feat(wrapperModules.zsh): init

[FluxZA]

Create a wrapper module for zsh

[FluxZA]

Hi. The tests are failing because my checks currently don't pass, I have a question regarding that below. I also have not finished documenting options.

Questions regarding the implementation:

• Do you think it is necessary to add an option to skip reading the /etc/zshenv file? I think it would be possible by overriding the configuration flags used to build the base package.
• I have not set options for common zsh options or features like aliases etc. This is because they would only be able to be added if the zdotFiles option is used, not the zdotdir option. Do you think this is fine or should I add them anyway with a caveat.

Questions regarding the checks:

• I wanted to test that the config in the store was being used, but from what I can see the test is running before the build is fully finished? Trying to ls the store directory fails claiming that it does not exist, however, when I check it manually after the tests stop, the directory exists and contains the file it should.

[BirdeeHub]

Do you think it is necessary to add an option to skip reading the /etc/zshenv file? I think it would be possible by overriding the configuration flags used to build the base package.

Not really no but for completeness maybe it could be offered one day, but overriding the base package means you rebuild it, and that is expensive.

I have not set options for common zsh options or features like aliases etc. This is because they would only be able to be added if the zdotFiles option is used, not the zdotdir option. Do you think this is fine or should I add them anyway with a caveat.

Yeah this is a conundrum slightly.

How about this.

What if you secretly make your own zdotdir every time. If they supply zdotdir then you can source the files from it if they exist as it would from each of your files in your zdotdir, but that would allow you to ALSO accept things from the zdotFiles options.

Also you should link the zdotFiles rather than copying them, or just refer to them when you source them from your secret zdotdir (probably easier to just check for existence then source from the script), that way you can include impure paths.

Also

      type = types.nullOr (types.either types.path types.str);

    type = types.nullOr wlib.types.stringable;

I wanted to test that the config in the store was being used, but from what I can see the test is running before the build is fully finished? Trying to ls the store directory fails claiming that it does not exist, however, when I check it manually after the tests stop, the directory exists and contains the file it should.

It was missing this condition, it might just be things like this.

      env.ZDOTDIR = lib.mkIf (config.zdotdir != null || config.zdotFiles != null) zdotdirEnv;

Because if you build with no arguments right now it refers to a place in placeholder out that doesnt get created. So its probably something of that nature. Your drv.installPhase is most definitely running before your test is, your test needs that whole zshWrapped derivation to resolve before it can get the path to it.

So, whatever is going wrong with the tests, it is not related to sequencing of stuff in the test being ran before stuff in the zshWrapped derivation. It is likely confusing because the test itself is a "build", but it is a runCommand which uses the result of zshWrapped. zshWrapped is computed already.

[BirdeeHub]

I just did

    zdotdir = builtins.path { path = ./test-zdotdir; };

and it worked but IDK why that makes any sense.

Also

    zdotdir = "${./test-zdotdir}";

That also works too for some reason.

I think this is lazy trees.......?

      zdotdirEnv = lib.mkIf (config.zdotdir != null || config.zdotFiles != null) (
        lib.trivial.warnIf (config.zdotdir != null && config.zdotFiles != null)
          "Using both zdotdir and zdotFiles options is not compatible. Only zdotdir will be used."
          (
            if config.zdotdir != null then
              if builtins.isPath config.zdotdir then builtins.path { path = config.zdotdir; } else config.zdotdir
            else
              "${placeholder "out"}/${zdotFilesDirname}"
          )
      );

Weird but the above passes the ls test

However, you should make your own zdotdir and source the stuff we provide, which hopefully will make this issue go away on its own? We will see.

The path to the test-zdotdir is in fact making it into the derivation. And I suppose your store would have that path. But its context somehow isnt making it.

But all I do is pass them to lib.escapeShellArg and then write them into the buildCommand of the drv. The path you are passing does in fact make it into the drv build command. It really should be provisioning this I feel like but maybe I am not understanding how it works?

---

Im going to swap back to upstream and see if this still happens

---

Edit: Ok, so it also happens on upstream. So it is not lazy-trees

But something is making it not provision that path, despite the fact that that path is very much making it into the derivation and is a nix path, and is only being passed through lib.escapeShellArg (this happens on all the backends it is not specific to the binary one)

But regardless, hopefully this goes away on its own once you change the design to always provide an internal zdotdir and source the users stuff yourself from there. If not, then call builtins.path on it if it is a path. I am not sure why this is needed. It seems like when you do stuff like lib.generators.toLua or builtins.toJSON it does this for you, but if you are just using toString it does not necessarily?

I wonder if this is something that I can do in wlib.types.stringable for convenience for the user?

---

Edit:

Should I do this?

  stringable = lib.mkOptionType {
    name = "stringable";
    descriptionClass = "noun";
    description = "str|path|drv";
    check = lib.isStringLike;
    merge =
      loc: defs:
      let
        res = lib.mergeEqualOption loc defs;
      in
      if builtins.isPath res then builtins.path { path = res; } else res;
  };

[FluxZA]

Thank you for the quick feedback!

I like the idea of using a secret zdotdir that sources the given files. If I use the approach of sourcing the zdotFiles, when they aren't impure paths should I check for existence in the store? My understanding of files in vs out of the store is still not the best.

[FluxZA]

I think it would be helpful to include that as part of stringable for the users, as some programs might need to test against directories.

[BirdeeHub]

A string may be an impure path, like "/home/flux/mynixos" or whatever. But if they are path types, they are pure paths, they just might not be provisioned apparently. I think this is because /home/flux/mynixos is a path, but not a pure path. But if something is providing something like that, they are already using --impure, so it still should be safe to call builtins.path on it?

builtins.path can be called multiple times. And you dont have to use wlib.types.stringable but where people would use it they probably would expect that thing to get provisioned. So I think I will do that.

[FluxZA]

A string may be an impure path, like "/home/flux/mynixos" or whatever. But if they are path types, they are pure paths, they just might not be provisioned apparently. I think this is because /home/flux/mynixos is a path, but not a pure path. But if something is providing something like that, they are already using --impure, so it still should be safe to call builtins.path on it?

is that because an absolute path is handled differently to a relative path? I thought that specifying a relative path is a pure path?

Also, what is the difference between provisioning vs a path existing in the store?

[BirdeeHub]

And, you shouldnt check that they are in the store no. They explicitly are wlib.types.stringable which means, anything that I can interpolate into a string, which includes an impure paths. If you want to test it with a path not already in the store, you should check that your secret one is there, and for the other options then you should test it with some paths that are "impure" inside the derivation if you want to, i.e. use a relative path within the test or something.

And yeah, ngl this one was a surprise to me as well, I did not know store paths worked like that. Oddly, you can still import from them even if they are like that, but they arent really "realized" yet, which is weird. This has to be some sort of optimization that allows it to not scan stuff in nixpkgs.

[BirdeeHub]

A string may be an impure path, like "/home/flux/mynixos" or whatever. But if they are path types, they are pure paths, they just might not be provisioned apparently. I think this is because /home/flux/mynixos is a path, but not a pure path. But if something is providing something like that, they are already using --impure, so it still should be safe to call builtins.path on it?

is that because an absolute path is handled differently to a relative path? I thought that specifying a relative path is a pure path?

Also, what is the difference between provisioning vs a path existing in the store?

"/home/flux" is a string, it is a string that looks like a path but wont be provided by nix

"./somedir" and this is the same. It will look in wherever that is at runtime. Nix has no idea where that is.

Colloquially, the above 2 are known as impure paths. This is because nobody acknowledges the next form of impure path:

/home/flux is an impure path. If you dont use --impure and you are using a flake this will throw an error if you try to use it.

./somedir this is a store path. you can call import on it or check for its existence without an IFD. But apparently it isnt always realized. Who knew? I didnt. I mean, I knew you could turn it into a string and then call unsafeDiscardStringContext on it to make sure that it does not get provisioned. But I kinda thought that otherwise it always would be? Generally one operates with these like they are always there, and are shocked when in some odd scenario like now they are not, and then call builtins.path on them to fix it.

[FluxZA]

Okay, thanks for clarifying.

[BirdeeHub]

To the best of my ability anyway 😆

[BirdeeHub]

#286

Gonna add this.

This also takes care of it for wlib.types.file as well. its path value is of the stringable type

Im not really sure if this needs to be done at the makeWrapper level or not yet. Maybe just a different default escapeShellArg that also calls this if it is a path first could be used for esc-fn? Regardless it should still be done in that type as well, and for our purposes here is enough.

[BirdeeHub]

Left the comments in the specific places they were mentioning. They may or may not be relevant by the time you finish implementing the suggestion where you source stuff yourself.

Anyway, hope I was helpful. Good luck!

[FluxZA]

Thank you.

[FluxZA]

One question I forgot to ask. Is there a way to run tests for a specific module only? I tried to look at the ci flake, but I didn't see any way to pass in arguments. Which now that I think of it makes sense as it is a flake.

[BirdeeHub]

One question I forgot to ask. Is there a way to run tests for a specific module only? I tried to look at the ci flake, but I didn't see any way to pass in arguments. Which now that I think of it makes sense as it is a flake.

nix build ./ci#checks.<system>.wrapperModule-zsh

Also with zsh that probably means ur gonna have to escape the #

nix flake check -Lv ./ci runs all the checks outputs of the flake with the checks in it. To run them individually you just build them.

I have the checks in a second flake so that I can do stuff like pull flake-parts to test that at some point, pull random stuff for docs but still run the docgen as part of the tests, mock things that are maybe complicated and need dependencies, etc. while leaving the top-level flake with just 1 input

[FluxZA]

Perfect thank you, that makes sense.

[FluxZA]

What do you think of this approach to building the files:

      baseZshenv = /* bash */ ''
        # zsh-wrapped zshenv: DO NOT EDIT -- this file has been generated automatically.
        # This file is read for all shells.

        # Ensure this is only run once per shell
        if [[ -v __WRAPPED_ZSHENV_SOURCED ]]; then return; fi
        __WRAPPED_ZSHENV_SOURCED=1

        # Cover some of the work done by zsh NixOS program if it is not installed
        if [[ ! (-v __ETC_ZSHENV_SOURCED) ]]
        then
          WRAPPER_MOCK_ETC_ZSHENV=1
          HELPDIR="${pkgs.zsh}/share/zsh/$ZSH_VERSION/help"

          # Tell zsh how to find installed completions.
          for p in ''${(z)NIX_PROFILES}; do
              fpath=($p/share/zsh/site-functions $p/share/zsh/$ZSH_VERSION/functions $p/share/zsh/vendor-completions $fpath)
          done
        fi

        # Get zshenv from wrapped options if they exist
        ${lib.optionalString (config.zdotFiles != null) /* bash */ ''
          if [[ -f "${config.zdotFiles.zshenv.path}" ]]
          then
            source "${config.zdotFiles.zshenv.path}"
          fi
        ''}
        ${lib.optionalString (config.zdotdir != null) /* bash */ ''
          if [[ -f "${config.zdotdir}/.zshenv" ]]
          then
            source "${config.zdotdir}/.zshenv"
          fi
        ''}
      '';
    in
    {
      drv.installPhase = ''
        mkdir $out/wrapped_zdot
        cat >$out/${zdotFilesDirname}/.zshenv <<'EOL'
        ${baseZshenv}
        EOL
      '';

[BirdeeHub]

yeah

You would have to do the thing you did for .zshenv for all of them probably, but basically that yeah.

Looks like you got some (most? all? IDK) of the stuff from nix that one might want to source too with the completions thing.

You should use buildPhase instead of installPhase and you should put the runHook for pre and post build like

drv.buildPhase = ''
  runHook preBuild;
  thestuff;
  ...;
  runHook postBuild
'';

But yes that looks like a fine approach.

Also, you should set config.passthru.shellPath = "/bin/zsh"; as discussed by this discussion about bash

[FluxZA]

I have updated with changes as discussed.

When it comes to testing, I couldn't find a good way to test the .zlogout file, I know it works locally, but it is not covered by the check module.

I was trying to think of a way to package the history file with the module, but I couldn't think of an elegant solution. The idea was that the history would not be placed in the local home directory and could be updated on exit, but I couldn't find a good place to put it outside the store, which is not an option due to being read only. Would be interested to hear any ideas you have.

[BirdeeHub]

I would think the history file should be handled by the user, they can set the location, and provision it either by copying it in their config if the file is not there, or if they are using home manager they can use home.files.<name>.onChange to do the copy. (home manager and nixos have activation scripts, which derivations do not have unless you are loading it as a shell)

---

This looks good actually.

---

One question.

Does -d solve the stuff from this discussion? Specifically the stuff about zenoli's attempt at a zsh wrapper?

I am still trying to figure out the best way to handle that.... It might even require getting your hands dirty redefining wrapperFunction to make it create some sort of sourceable file for the variables to source from the config and only handle the args with the wrapper script itself....

Do you have any ideas on that which do not require such a thing? I feel like such a thing might be worth a helper for that, so, if it does need that, wait for me to add a helper for doing such a thing maybe unless you wanted to try to tackle that yourself?

---

So, I approve of the work you have done on this so far, but unfortunately, recent news is saying that this might not be all that is needed, due to some modifications nixpkgs made to some of the shell packages, and the way they set up /etc/profile

On the bright side, all the work you have done so far is still applicable, and is a big help and should still work in scenarios where you do things such as, launch it from your wezterm drv as the launch command

Maybe as a stop-gap measure we can config.passthru.ZDOTDIR = "${config.wrapper}/${zdotFilesDirname}"; until we find a nice way to handle that? (honestly we should probably do that anyway?)

[BirdeeHub]

Suggested change

	passthru.ZDOTDIR = "${config.wrapper}/${zdotFilesDirname}";

^ One of the things mentioned by this comment #285 (comment) (we use config.wrapper because placeholder "out" is a placeholder and only points where you would expect if placed into that drv build, whereas for internal consumption we use placeholder "out" to avoid infinite recursion)

[FluxZA]

Thanks for the feedback, I had a look at that discussion, and your idea for a helper function seems like it might be the best option. I asked zenoli to test the -d flag for information sake, but it feels more like a workaround than a solution, especially if we are considering all shells.

In the same vein of ideas that work for zsh but not all shells, you can change the build args for zsh to specify where /etc files are read from. This means we could avoid sourcing any system rc files, and I imagine this would include /etc/profile. However, as you mentioned when I asked about this previously, having to rebuild the package is not ideal. I just thought I would throw it out there so all ideas are on the table.

If we decide to use a helper function, I would be happy to give it a try.

[BirdeeHub]

Yeah I don't want to incur a rebuild in this repo if possible (makes tests slow, the only reason .override its ok with me in mpv is because they have mpv-unwrapped which pkgs.mpv just uses, and it works similarly with vim). Feel free to do it via the overrides option in your config, but I don't want to do it here if possible.

Im working on some kind of helper at the moment.

Im going to do the sort and return the sorted DAL with an added type field (which option it came from), esc-fn will not be applied yet, it will be in the set for that item, with the default value from config.escapingFunction substituted if it was null. It will not process argv0 and argv0type

And then Im going to figure out what to do from there, maybe that is enough maybe it is not we will see.

But likely you would import the makeWrapper module but with the wrapperImplementation option excluded, and then use that helper to define wrapperFunction such that it just returns a string which is a build script which constructs an initial wrapper via pkgs.makeBinaryWrapper.

It would filter by the type field for flags (normalized to be like addFlag), addFlag, and appendFlag from the DAL and then along with your ZDOTDIR in placeholder "out", make the base wrapper using pkgs.makeBinaryWrapper

And then in your ZDOTDIR you would filter out the ones you already processed from the DAL, and then stick the instructions for setting the rest of the things in a config file like zshrc or zshenv (the 4 shell functions here should be sourceable by zsh and may come in handy for that)

So, I will add some amount of helper function(s). You can view it/them and decide what you want to do.

If you would rather not deal with that part, I am ok with accepting it and then adding that part to it myself and adding myself as a secondary maintainer to the module.

If I end up doing it, I will still make the part actually in the zsh module as simple as possible, so that you hopefully will actually be able to understand what I did, I don't want to crash your current understanding of how it works.

I obviously had no such concerns for the neovim one, as there is a 134 line pipe with the result of another pipe ++'d into the middle of it in that one 🤣 So, yeah, if I do it, I pledge not to do that to your module 🤣

[FluxZA]

I am happy to implement it, I just need to take a look at what is happening in /etc/profile to try fully understand why we are doing what we are doing. I get that path is being redefined, affecting additional packages, but not how it relates to the other config options.

I also need to try get a proper understanding of the DAL, so doing this myself will be a good way to check how well I am doing there 😂.

The final added benefit is that you won't have to worry about creating another neovim situation in someone else's module XD

[BirdeeHub]

Luckily, a DAG or DAL sounds more complex than it is to use

It is an attrsOf or listOf a wlib.types.spec type

wlib.types.spec is an auto-normalizing type. wlib.types.spec is the part that is hard to understand, you don't need to understand the implementation to use it, but you do need to sorta understand what it does.

{ config, lib, wlib, ... }: {
  options.myspecset = lib.mkOption {
    default = {};
    type = lib.types.attrsOf (wlib.types.spec {
      # yes this is a module, yes this supports type merging
      # If the thing does not have the expected fields, it will place it in the main field.
      options = {
        data = lib.mkOption {
          type = lib.types.str;
          description = "no default makes this the main one";
        };
        extrafield = lib.mkOption {
          type = lib.types.nullOr lib.types.str;
          description = "and then you can have other ones, but they need defaults";
          # You could theoretically set this default via config for this spec submodule as well.
          # It needs to be defined somewhere within the base set of modules passed to the type
          default = null;
        };
      };
    });
  };
  # provide just the value for the main field
  config.myspecset.TESTVAL1 = "hello";
  # or a set where you can set more than just the main field
  config.myspecset.TESTVAL2.data = "hello2";
  config.myspecset.TESTVAL2.extrafield = "hello2";
  config.myspecset.TESTVAL3 = { data = "hello3"; extrafield = "hello3"; };
  config.myspecset.TESTVAL4 = { data = "hello4"; };
  # now:
  # config.myspecset == {
  #   TESTVAL1 = { data = "hello"; extrafield = null; };
  #   TESTVAL2 = { data = "hello2"; extrafield = "hello2"; };
  #   TESTVAL3 = { data = "hello3"; extrafield = "hello3"; };
  #   TESTVAL4 = { data = "hello4"; extrafield = null; };
  # }
}

The module passed to wlib.types.spec for the DAG and DAL types contains name before after and data with data as the main field. The main field is where, if you provided just the value instead of a set, the value will be placed. So, in this case, bare values will be placed in data. The makeWrapper options also add an esc-fn field to the spec type for the DAL, and the flags set also adds a sep field.

The sorting functions from wlib.dag can sort either form of the data structure. The type is auto-normalizing, so you can just call it on the result!

The thing that makes it a DAG are the name before and after fields, and data is a convention relied upon for a few of the functions in wlib.dag. For example, wlib.dag.lmap and wlib.dag.gmap expect a .data field. But wlib.dag.topoSort itself doesn't expect much of anything other than that the thing is a list or set of sets with optional name before and after fields.

As to the behavior of the sort, it gets to be quite simple, thanks to nixpkgs offering lib.toposort

Basically, there was a lot of stuff where you would want some collection of either the value, or a set with the value and some other stuff. And then you would need to do a bunch of normalization yourself. This is the type for that, and it automatically normalizes it. Then you do whatever with that. If it has a name before and after field, you can call the sorting function on it.

[BirdeeHub]

Suggested change

	addFlag = lib.mkIf (config.skipGlobalRC) [ "-d" ];
	flags."-d" = config.skipGlobalRC;

^ lets people config.flags."-d".data = lib.mkForce idk;, lets people grab and use the value from config.flags."-d".data, and lets people before = [ "-d" ]; or after = [ "-d" ]; (you can do the before and after thing with addFlag too but you would need to { name = "-d"; data = [ "-d" ]; } and they would only be able to see or override that easily via your config.skipGlobalRc option)

Tbh, this one is up to you now that I think about it, it is a boolean option, it kinda doesn't matter the effect is more or less the same.

Honestly maybe we remove it? Once we are setting everything other than the args and ZDOTDIR from within the config itself I don't know what conflicts could even happen, and people could always pass it themselves still

If nothing else, hopefully that further explains the DAG vs DAL types.

And also the trade-off between which to use. DAGs have better overridability because you can always index into them without having to do any mapping, but DALs have an inherent ordering and lib.mkBefore and lib.mkAfter are concepts which also make sense. The same trade-off is true of just a regular attrsOf and listOf type, except the DAG and DAL types also lets you describe dependent ordering.

[BirdeeHub]

To make a zsh-sourceable version of these:

  setvarfunc = /* bash */ ''wrapperSetEnv() { export "$1=$2"; }'';
  setvardefaultfunc = /* bash */ ''wrapperSetEnvDefault() { [ -z "''${!1+x}" ] && export "$1=$2"; }'';
  prefixvarfunc = /* bash */ ''wrapperPrefixEnv() { export "$1=''${!1:+$3$2}''${!1:-$3}"; }'';
  suffixvarfunc = /* bash */ ''wrapperSuffixEnv() { export "$1=''${!1:+''${!1}$2}$3"; }'';

You can edit these by just swapping the ${! for ${(P)

prefixvarfunc = /* zsh */ ''wrapperPrefixEnv() { export "$1=''${(P)1:+$3$2}''${(P)1:-$3}" }'';

I think there may be some kind of emulate bash option that could be used instead of editing them also idk, but it is such a small change considering the rest of what has to be done.

The fish version of those is so long, none of these shorthands work. I don't know why people use fish other than for the slightly nicer vi mode tbh, that is whoever is making a fish wrapper module's problem XD

❯ unset TESTVAR
❯ wrapperPrefixEnv() { export "$1=${(P)1:+$3$2}${(P)1:-$3}" }
❯ wrapperPrefixEnv TESTVAR : AGAIN
❯ wrapperPrefixEnv TESTVAR : HELLO
❯ echo "$TESTVAR"
HELLO:AGAIN

^ This is how we ensure uniform escaping rules for all the values under the hood in the nix wrapperFunction implementation. We make them all function arguments. If we were to try to interpolate them directly into what we put as the body of the function, we would end up with no end of escaping issues because we are passing from nix and not bash. It also makes the generated wrappers really nice and readable.

[FluxZA]

Thank you so much for that explanation, it helped a lot.

To check that we are on the same page, I am waiting for you to finish the helper functions you are working on and then I will go about updating the zsh wrapper to handle the options other than those set in the base wrapper as part of the config?

[BirdeeHub]

Yes, it will take a few days though, a bit of re-working to do.

Trying to figure out what to do with the flags set. I kinda want to add an ifs field to it which if ifs = ","; for example allows flags."--arg" = [ "a" "b" "c" ]; to create --arg=a,b,c instead of when it is null where it would do --arg=a --arg=b --arg=c like it does currently.

If I want to do that, now is probably the time to do that, while I am adding helpers for doing the sort and reimplementing wrapperFunction yourself.

I have a helper for the sort, but im still working on the shape of it. I need one for processing the flags, and maybe one or 2 others. In order to determine what I need, I am reimplementing the current ones using the new helper.

But the sort itself will be something like this

{
  config,
  lib,
  wlib,
  sortResult ? true,
  ...
}:
let
  liftDag = type: v: {
    inherit type;
    data = v.data or null;
    before = v.before or [ ];
    after = v.after or [ ];
    name = v.name or null;
    esc-fn = v.esc-fn or null;
    ${if type == "flags" then "sep" else null} = v.sep or null;
    ${if type == "flags" then "ifs" else null} = v.ifs or null;
    value = v;
  };
  pushDownDagNames = builtins.mapAttrs (
    n: v:
    v
    // {
      name =
        if isString (v.name or null) then
          v.name
        else if isString n then
          n
        else
          null;
    }
  );
  mapAndLiftDal = from_option: map (liftDag from_option);
  mapAndLiftDag =
    from_option: dag:
    lib.mapAttrsToList (attr-name: v: liftDag from_option v // { inherit attr-name; }) (pushDownDagNames dag);
  unsorted =
    mapAndLiftDal "unsetVar" (config.unsetVar or [ ])
    ++ mapAndLiftDag "env" (config.env or { })
    ++ mapAndLiftDag "envDefault" (config.envDefault or { })
    ++ mapAndLiftDal "prefixVar" (config.prefixVar or [ ])
    ++ mapAndLiftDal "suffixVar" (config.suffixVar or [ ])
    ++ mapAndLiftDal "prefixContent" (config.prefixContent or [ ])
    ++ mapAndLiftDal "suffixContent" (config.suffixContent or [ ])
    ++ mapAndLiftDal "chdir" (config.chdir or [ ])
    ++ mapAndLiftDal "runShell" (config.runShell or [ ])
    ++ mapAndLiftDag "flags" (config.flags or { })
    ++ mapAndLiftDal "addFlag" (config.addFlag or [ ])
    ++ mapAndLiftDal "appendFlag" (config.appendFlag or [ ]);
in
if sortResult then wlib.dag.unwrapSort "makeWrapper" unsorted else unsorted

Im not sure if I want to have flags + addFlag and appendFlag as their own outputs, or if I just want to output 1 big DAL

I will have a helper for processing the flags set, and maybe one for taking a list of 3 values and turning it into a set with var sep env but I don't know what shape it will be, I will find out by reimplementing the current backends using this

But the idea would be, you get the raw stuff but sorted, and a helper or 2, and then you fold to group the values as desired, likely you pull out flags addFlag and appendFlag and use those plus the argv0 options and config.package config.exePath and config.binName to build the base command which gets launched, and then you map over the rest of them in place into build commands

[FluxZA]

Sounds good, I will only have time to look at it next weekend anyway.

I like the idea for the comma separated args 👍

[BirdeeHub]

Im also going to address #307 as I add this helper.

Edit: Which I hope to be done with by the end of today (now the 27th as of writing this edit) but it isn't the only thing I have to do today, so we will see. I have been busy this week.

I do have some helpers though, I need to figure out how to expose it and use it though.

[BirdeeHub]

I now need to implement the existing wrapperFunction implementations using that, or something very close to that, and figure out how to expose that.

Then it will be ready for you to use, which basically will will entail pulling out the flags using the helper, and mapping --add-flag and --append-flag into them and calling esc-fn on it, grabbing argv0 and argv0type, the path to wrap and the path to output to, and passing that to pkgs.makeBinaryWrapper. Then you would copy what I do in makeWrapperNix for the rest of the things, but swap ${! for ${(P) and put that in a file you source from a zsh config file.

[BirdeeHub]

also you now have a merge conflict but it is a really simple one, someone else snuck into the maintainers list.

[BirdeeHub]

Ok. I have merged the helper things.

Good luck!

You will need to mash the two wrapperImplementation options together, and use pkgs.makeBinaryWrapper for the args and ZDOTDIR and then the nix implementation version for the thing sourced by the zsh config.

[FluxZA]

Thanks for all the info, unfortunately my weekend has filled up so I probably won't get to this until sometime this week. Is there anything I should prioritise to give feedback on this weekend?

[BirdeeHub]

Take your time it will still be here when you have time for it!

If there is anything to give feedback on, its these 2 files I link in this reply, and if they look sane to you or not, but I am pretty confident that what I cooked up is at the very least not harder than it was before 🤣

Fair warning, my helpers did not make it too much less verbose, but they do make it easier to think about and work with and read the resulting code.

They handle aggregating and sorting the options into 1 DAL, and they handle splitting up the args, the flags set, and running stuff on 1 or many variants, so, they cover the hard stuff but the verbose part is the actual backend representation of these things, and that still needs to be done by implementer.

I have no idea what purposes someone may have for redefining it, so I covered as much of the backend agnostic machinery as I could without touching the actual final representation of these things.

So, there is an aggregate function to turn an options set into a DAL and optionally sort it for you, and then there is a helper to split it into args and other, and then there is a helper that turns the flags entries into addFlag entries for you. Then all you have to do is map over everything in place and apply the esc-fn however is relevant for that option, and then concat and provide them to whatever builder format you are doing.

So, like, the pkgs.makeWrapper backend then just intersperses the values with their --add-flag or whatever arg, and then applies esc-fn to everything and concatStringsSep " ", and puts it in the build string providing all that to pkgs.makeWrapper. And the in-house nix one for the args it basically just applies esc-fn and concats them, and then for the other ones, makes a build command which copies whatever they have to do into the wrapper.

You would need to do this

nix-wrapper-modules/lib/makeWrapper/makeWrapper.nix

Lines 11 to 60 in 7025a67

	argv0 =
	if builtins.isString (config.argv0 or null) then
	[
	"--argv0"
	(lib.escapeShellArg config.argv0)
	]
	else if config.argv0type or null == "resolve" then
	[ "--resolve-argv0" ]
	else
	[ "--inherit-argv0" ];
	baseArgs = map lib.escapeShellArg [
	(
	if !builtins.isString (config.exePath or null) || config.exePath == "" then
	"${config.package}"
	else
	"${config.package}/${config.exePath}"
	)
	"${placeholder "out"}/${config.binDir or "bin"}/${config.binName}"
	];
	split = wlib.makeWrapper.splitDal (wlib.makeWrapper.aggregateSingleOptionSet { inherit config; });
	cliArgs = lib.pipe split.args [
	(wlib.makeWrapper.fixArgs { sep = config.flagSeparator or null; })
	(
	{ addFlag, appendFlag }:
	let
	mapArgs =
	name:
	lib.flip lib.pipe [
	(map (
	v:
	let
	esc-fn = if v.esc-fn or null != null then v.esc-fn else config.escapingFunction;
	in
	if builtins.isList (v.data or null) then
	map esc-fn v.data
	else if v ? data && v.data or null != null then
	esc-fn v.data
	else
	[ ]
	))
	lib.flatten
	(builtins.concatMap (v: [
	"--${name}"
	v
	]))
	];
	in
	mapArgs "add-flag" addFlag ++ mapArgs "append-flag" appendFlag
	)
	];

and this

nix-wrapper-modules/lib/makeWrapper/makeWrapper.nix

Lines 114 to 124 in 7025a67

	''
	(
	OLD_OPTS="$(set +o)"
	${srcsetup dieHook}
	${srcsetup (
	if config.wrapperImplementation or null != "binary" then makeWrapper else makeBinaryWrapper
	)}
	eval "$OLD_OPTS"
	makeWrapper ${makeWrapperArgs}
	)
	''

like the pkgs.makeBinaryWrapper backend (and also pass --set ZDOTDIR yourpath to it as well)

And you would do this

nix-wrapper-modules/lib/makeWrapper/makeWrapperNix.nix

Lines 9 to 25 in 7025a67

	prefuncs =
	let
	setvarfunc = /* bash */ ''wrapperSetEnv() { export "$1=$2"; }'';
	setvardefaultfunc = /* bash */ ''wrapperSetEnvDefault() { [ -z "''${!1+x}" ] && export "$1=$2"; }'';
	prefixvarfunc = /* bash */ ''wrapperPrefixEnv() { export "$1=''${!1:+$3$2}''${!1:-$3}"; }'';
	suffixvarfunc = /* bash */ ''wrapperSuffixEnv() { export "$1=''${!1:+''${!1}$2}$3"; }'';
	in
	lib.optional (config.env or { } != { }) setvarfunc
	++ lib.optional (config.envDefault or { } != { }) setvardefaultfunc
	++ lib.optional (config.prefixVar or [ ] != [ ] || config.prefixContent or [ ] != [ ]) prefixvarfunc
	++ lib.optional (
	config.suffixVar or [ ] != [ ] || config.suffixContent or [ ] != [ ]
	) suffixvarfunc;
	outdir = "${placeholder "out"}/${config.binDir or "bin"}";
	outpath = lib.escapeShellArg "${outdir}/${config.binName}";
	wrapcmd = partial: "echo ${lib.escapeShellArg partial} >> ${outpath}";

and this

nix-wrapper-modules/lib/makeWrapper/makeWrapperNix.nix

Lines 78 to 118 in 7025a67

	(wlib.dag.unwrapSort "makeWrapper")
	(builtins.concatMap (
	v:
	let
	esc-fn = if v.esc-fn or null != null then v.esc-fn else config.escapingFunction;
	in
	if v.type or null == "unsetVar" then
	[ (wrapcmd "unset ${esc-fn v.data}") ]
	else if v.type or null == "env" then
	[ (wrapcmd "wrapperSetEnv ${esc-fn v.attr-name} ${esc-fn v.data}") ]
	else if v.type or null == "envDefault" then
	[ (wrapcmd "wrapperSetEnvDefault ${esc-fn v.attr-name} ${esc-fn v.data}") ]
	else if v.type or null == "prefixVar" then
	[ (wrapcmd "wrapperPrefixEnv ${lib.concatMapStringsSep " " esc-fn v.data}") ]
	else if v.type or null == "suffixVar" then
	[ (wrapcmd "wrapperSuffixEnv ${lib.concatMapStringsSep " " esc-fn v.data}") ]
	else if v.type or null == "prefixContent" then
	let
	env = builtins.elemAt v.data 0;
	sep = builtins.elemAt v.data 1;
	val = builtins.elemAt v.data 2;
	cmd = "wrapperPrefixEnv ${esc-fn env} ${esc-fn sep} ";
	in
	[ ''echo ${lib.escapeShellArg cmd}"$(cat ${esc-fn val})" >> ${outpath}'' ]
	else if v.type or null == "suffixContent" then
	let
	env = builtins.elemAt v.data 0;
	sep = builtins.elemAt v.data 1;
	val = builtins.elemAt v.data 2;
	cmd = "wrapperSuffixEnv ${esc-fn env} ${esc-fn sep} ";
	in
	[ ''echo ${lib.escapeShellArg cmd}"$(cat ${esc-fn val})" >> ${outpath}'' ]
	else if v.type or null == "chdir" then
	[ (wrapcmd "cd ${esc-fn v.data}") ]
	else if v.type or null == "runShell" then
	[ (wrapcmd v.data) ]
	else
	[ ]
	))
	(builtins.concatStringsSep "\n")
	];

like the in-house nix backend but write it to a different file which you are going to source from a zsh config (and swap ${! for ${(P) in the 4 helper function things like setvarfunc and the like so it works in zsh)

You would only do that for the main binary probably.

For the wrapper variants, you would just do it kinda like I did for the neovim one where I call the normal function:

nix-wrapper-modules/wrapperModules/n/neovim/makeWrapper/default.nix

Line 22 in cf2acac

wrapNvim + "\n" + wlib.makeWrapper.wrapVariants { inherit config callPackage; }

The file is created by the main one already, so if you want to source it in a variant, you still can.

---

You definitely chose a difficult one as your first wrapper module 🤣 thanks for sticking with it. 🤣