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

Add a mechanism to provide configuration from a chosen place #564

Closed
Niols opened this issue Jul 7, 2023 · 10 comments · Fixed by #567
Closed

Add a mechanism to provide configuration from a chosen place #564

Niols opened this issue Jul 7, 2023 · 10 comments · Fixed by #567
Labels
P2 major: an upcoming release type: feature request

Comments

@Niols
Copy link
Member

Niols commented Jul 7, 2023

Is your feature request related to a problem? Please describe.

Currently, Topiary takes its configuration from .topiary/languages.toml in the current directory or any directory above, and falls back on ~/.config/topiary/languages.toml if I understand correctly. This is nice, but I would sometimes want to provide a configuration file coming from elsewhere. A typical use case would be for anything that involves integrating Topiary in a Nix workflow, where the configuration file would be generated by Nix itself: I'd want to be able to call Topiary with a configuration file in the Nix store.

Describe the solution you'd like

I would want an environment variable controlling the place where to go look for configuration files. I am not entirely sure how it should behave with the default behaviour described above, as in: should this environment variable add new places where to look or should it define the set of places where to look? In the second case, one could imagine a PATH-like environment variable that can contain relative paths (in which case they are relative to the current directory or any directory above) or absolute paths; it's default would be .topiary/languages.toml:~/.config/topiary/languages.toml or something.

I suppose it would also make sense to provide a command line flag providing the same behaviour as this environment variable.

@aspiwack
Copy link
Member

aspiwack commented Jul 7, 2023

Let's avoid environment variables if at all possible. Environment variables are “untyped”: make a typo, it goes undetected, and ignored. Command-line flags are to be preferred.

@Xophmeister
Copy link
Member

Untyped? Surely CLI arguments are opaque strings, too?

@aspiwack
Copy link
Member

aspiwack commented Jul 7, 2023

From a typing point of view, they're part of an enumerated type, not mere string.

@Niols
Copy link
Member Author

Niols commented Jul 7, 2023

Ah, you mean a typo in the name of the environment variable. Fair enough.

@aspiwack
Copy link
Member

aspiwack commented Jul 7, 2023

Oh! I see the ambiguity now. Sorry. Yes, I meant the name of the option. Environment variables are very brittle in that if you misspell them, you have the wrong behaviour instead of an error.

@Niols
Copy link
Member Author

Niols commented Jul 7, 2023

One thing that I enjoy with environment variables is that they allow to write clever wrappers around a tool by setting some environment variables if they aren't already set for instance. This is what I use for Topiary on OPAM: I set the TOPIARY_LANGUAGES_DIR to the right directory in the OPAM prefix, but if another user defines TOPIARY_LANGUAGES_DIR, then I use that one. But I'm fine with the idea to try and avoid environment variables by default and to add them later if the need arises.

@ErinvanderVeen
Copy link
Collaborator

I agree with @Niols here. The same is also true for nixpkgs, which also sets TOPIARY_LANGUAGES_DIR

I would also like to mention that we would make these environment variables visible in topiary --help:

CLI app for Topiary, the universal code formatter.

Usage: topiary [OPTIONS] <--language <LANGUAGE>|--input-files [<INPUT_FILES>...]>

Options:
...
      --configuration-override <CONFIGURATION_OVERRIDE>
          Override all configuration with the provided file [env: TOPIARY_CONFIGURATION_OVERRIDE=/home/erin/.config/topiary/languages.toml]
...

@aspiwack
Copy link
Member

It's fine to have both environment variables and configuration flags. What we should avoid is options that are only available as environment variables.

@ErinvanderVeen
Copy link
Collaborator

@aspiwack In that case, we totally agree.

@aspiwack
Copy link
Member

I like agreeing. It's not as fun as disagreeing, but it's certainly easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 major: an upcoming release type: feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants