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

merge more config file settings into config #28

Merged
merged 1 commit into from
Aug 18, 2023

Conversation

aikomastboom
Copy link

When I tried setting editor_cmd in my .projectable.toml file.. expecting it to override both the default vi and $EDITOR env.

It doesn't get used by the application.

This PR adds a few "missing" fields to the merge part of the configuration.

If these are omitted by design.. please feel free to discard this PR.

NOTE: this change gives (local) config precedence over env which might not be what is desired. Maybe the order of evaluation should be 1): env -> local config -> global config -> default
With this PR it appears to become 2): local config -> global config -> env -> default
Without this PR it is 3): env -> default

@Absobel
Copy link
Contributor

Absobel commented Aug 18, 2023

Ah you're right ! I totally forgot this was a thing when i added editor_cmd

@dzfrias
Copy link
Owner

dzfrias commented Aug 18, 2023

Great catch! The current merging implementation is definitely not ideal...

Anyway, thanks for the PR and I'll merge this in shortly!

@dzfrias dzfrias merged commit 3524d8e into dzfrias:main Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants