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.

Sign up for GitHub

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

Jump to bottom

[ocamlformat]: Version detection #41

Merged
merged 5 commits into from Feb 9, 2023
Merged

[ocamlformat]: Version detection #41

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b73fff3
ocamlformat: Modify package option to support version detection
huwaireb Feb 6, 2023
82f2a03
ocamlformat: Allow for custom package sets when specifying path
huwaireb Feb 6, 2023
a763fb0
ocamlformat: Format the module and modify the error message for clarity
huwaireb Feb 6, 2023
d6b2e55
ocamlformat: fix case where only a file is passed
huwaireb Feb 7, 2023
45d73a8
ocamlformat: Separate version detection from the package option onto …
huwaireb Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 34 additions & 2 deletions programs/ocamlformat.nix
View file
Open in desktop
Expand Up @@ -4,17 +4,49 @@
, ...
}:
let
inherit (lib) types;

cfg = config.programs.ocamlformat;

detectVersion = configFile: pkgSet:
let
optionValue = list:
assert lib.assertMsg (list != [ ]) "treefmt-nix: Unable to detect ocamlformat version from file";
lib.elemAt list (lib.length list - 1);
in
builtins.getAttr "ocamlformat_${
builtins.replaceStrings ["."] ["_"]
(optionValue (lib.findFirst (option: builtins.head option == "version") []
(builtins.map (n: lib.splitString "=" n) (lib.splitString "\n" (builtins.readFile configFile)))))
Copy link
Member

Choose a reason for hiding this comment

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

It's a bit problematic that this is depending on an Import From Derivation to work. The builtins.readFile is reading the build result of the ocamlformat package that's passed in.

A better solution would be to set the meta.mainProgram of the package in nixpkgs. This would make ocamlformat work out of the box.

Copy link
Member

Choose a reason for hiding this comment

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

But thanks for the contribution!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a bit problematic that this is depending on an Import From Derivation to work. The builtins.readFile is reading the build result of the ocamlformat package that's passed in.

I don't follow? builtins.readFile is not reading the build result of the ocamformat package nor is it reading any other derivation, what it is reading however is a path passed as a module option, which should be fine because its known pre-eval.

There is no IFD?

Copy link
Member

Choose a reason for hiding this comment

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

My bad. Can you show me a usage example?

Copy link
Contributor Author

@huwaireb huwaireb Feb 7, 2023 •
edited

Choose a reason for hiding this comment

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

My bad. Can you show me a usage example?

programs.ocamlformat = {
     enable = true;
     package = ./.ocamlformat;
 };

.ocamlformat:

profile = default
version = 0.24.1

Copy link
Member

Choose a reason for hiding this comment

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

I see. Let's split it out into three different options so they can be better documented:

  • package - leave that as before. Is only used if the ocamlformat file is not passed.
  • pkgs - the package set for ocamlformat file to look into. defaults to nixpkgs.
  • ocamlformat (name to be discussed) - type: none or path - path to the .ocamlformat file. Used to pick the right version of ocamlformat if passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense,

I pushed up a commit, see if thats any good
I used configFile as opposed to ocamlformat to refer to the name

}"
pkgSet;
in
{
options.programs.ocamlformat = {
enable = lib.mkEnableOption "ocamlformat";
package = lib.mkPackageOption pkgs "ocamlformat" { };
package = lib.mkOption {
type = types.oneOf [
types.path
types.package
(types.submodule {
options = {
path = lib.mkOption { type = types.path; };
pkgs = lib.mkOption {
type = types.lazyAttrsOf types.raw;
};
};
})
];
default = pkgs.ocamlformat;
};
};

config = lib.mkIf cfg.enable {
settings.formatter.ocamlformat = {
command = cfg.package;
command =
if lib.isDerivation cfg.package
then cfg.package
else detectVersion (cfg.package.path or cfg.package) (cfg.package.pkgs or pkgs);
options = [ "-i" ];
includes = [ "*.ml" "*.mli" ];
};
Expand Down