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

Populate a default config file #37

Closed
shilangyu opened this issue Feb 26, 2020 · 6 comments · Fixed by #57
Closed

Populate a default config file #37

shilangyu opened this issue Feb 26, 2020 · 6 comments · Fixed by #57
Labels
feature Requests for a new feature.

Comments

@shilangyu
Copy link
Contributor

Describe the feature request

Perhaps it is a good idea to create a default config file (same as the one that is bundled into release) in the config path (as specified here) if it does not exist?

Additional context/details

Why it would be beneficial:

  • no more need for bundling the default config file with a release (single executable ftw)
  • the config file will anyways end up in this path, so why not put it automatically
  • easier to get started, just run btm once and you can go straight to configuring with lets say vim ~/.config/bottom/bottom.toml
@shilangyu shilangyu added the feature Requests for a new feature. label Feb 26, 2020
@shilangyu
Copy link
Contributor Author

If you accept contributions I would like to tackle this issue, seems like a good first issue

@ClementTsang
Copy link
Owner

Sure! I thought about doing this originally but wasn't sure what a good way to tackle this was.

@ClementTsang ClementTsang removed their assignment Feb 26, 2020
@shilangyu
Copy link
Contributor Author

shilangyu commented Feb 26, 2020

The approach I have been using in my projects that use config files is the following:

  • the program starts
  • looks for a config file
    • if it exists, load the config
    • if it does not exist, create it with a ready config that has comments around each field (similar to alacritty) and load it
  • overwrite the config with commandline flags
  • use defaults for missing fields

What do you think?

EDIT: meant override, not overwrite

@ClementTsang
Copy link
Owner

ClementTsang commented Feb 27, 2020

overwrite the config with commandline flags

While I like the idea behind this part, it's kinda 50/50 for me. I'm just not 100% on setting values for the user without them explicitly knowing that they're set, especially since if an option is set to true on the config file, then there's no way to actually turn the thing off from the command line - only on.

So either notifying the user what values are set, or just (imo the easier solution I guess) leaving it default entirely if we create one is probably what I would rather do.

The rest of it I like though.

@shilangyu
Copy link
Contributor Author

What I meant by overwrite the config with commandline flags is what is already currently implemented. If my config file says the temperature should be Kelvin, but I run bottom with btm -c it will prioritize the Celsius flag and show the temperature in Celsius

@ClementTsang
Copy link
Owner

Ah, okay. Thought you meant something like, if you run say btm -a with no config file it would generate a config file with the average_cpu flag set to true.

Looks good then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Requests for a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants