feat(wrapperModules.bat): init#494
Conversation
|
https://github.com/sharkdp/bat#adding-new-syntaxes--language-definitions ^ this is in our dir too? What to do about this? |
| }) config.themes; | ||
| syntaxes-constructFiles = lib.concatMapAttrs (name: value: { | ||
| "syntaxes-${name}" = { | ||
| content = builtins.readFile value.path; |
There was a problem hiding this comment.
Can these be directories too or just files?
If they are only files, this is technically ok but I am still not super sure how I feel about readFile here to be honest. It means you can't use an impure path, as you would be able to if you just did ln -s for example, and runs the risk of IFD.
Could you use constructFiles.<name>.builder instead to build the value of content if nonempty or copy it if it is empty? Or something to that effect?
If they can be directories too, I am not super sure exactly how you want to do that, maybe just accept just wlib.types.stringable for them and let them make or get the file themselves if they want it and pass the path there, and then you just ln -s them via config.buildCommand?
There was a problem hiding this comment.
They can be directories, too. So let's just accept paths.
| syntaxes-constructFiles = lib.concatMapAttrs (name: value: { | ||
| "syntaxes-${name}" = { | ||
| content = builtins.readFile value.path; | ||
| content = builtins.readFile value; |
There was a problem hiding this comment.
Can we either use .builder here to do ln -s, or config.buildCommand to do ln -s?
Then they can be impure paths also
Thats why the readFile was bothering me too.
There was a problem hiding this comment.
If .builder does ln -s, what should we set .content to? And how does .builder know about the file it should symlink then?
There was a problem hiding this comment.
Since we are only accepting paths, because they can be directories, lets not use constructFiles for that
buildCommand.makeBatSyntaxes =
"mkdir -p ${placeholder "out"}/syntaxes\n"
+ lib.concatMapAttrsStringSep "\n" (
n: v: "ln -s ${v} ${placeholder "out"}/syntaxes/${n}"
) config.syntaxes;Something like that is probably better?
Regardless, if we are going to be using readFile, there isn't a point to accepting a path.
There was a problem hiding this comment.
Yes, you are right, that's a good solution. Should we do the same for themes?
| }; | ||
| syntaxes = lib.mkOption { | ||
| type = lib.types.attrsOf (wlib.types.file pkgs); | ||
| type = lib.types.attrsOf lib.types.string; |
There was a problem hiding this comment.
wlib.types.stringable is the type for anything that can be "${interpolated}"
2 participants
Wrapper Module for the bat CLI.