Skip to content

merge more config file settings into config#28

Merged
dzfrias merged 1 commit intodzfrias:mainfrom
aikomastboom:merge-configfile-into-config
Aug 18, 2023
Merged

merge more config file settings into config#28
dzfrias merged 1 commit intodzfrias:mainfrom
aikomastboom:merge-configfile-into-config

Conversation

@aikomastboom
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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