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

should not assume default config.nu and env.nu #6787

Open
PerBothner opened this issue Oct 17, 2022 · 5 comments
Open

should not assume default config.nu and env.nu #6787

PerBothner opened this issue Oct 17, 2022 · 5 comments
Labels
configuration Issue related to nu's configuration enhancement New feature or request

Comments

@PerBothner
Copy link
Contributor

Related problem

Automatically installing default config.nu and env.nu files is convenient for developers but is it fragile and user-unfriendly.

  • It makes it difficult for users to distinguish their own deliberate changes from recommended defaults.
  • It makes configuration files cluttered and unwieldy.
  • It makes it difficult to add new configuration options, at least if the default depends on the environment or other settings. The problem is they will be be missing from existing config.nu and env.nu and it is unreasonable to ask users to update.
  • It adds an extra step for new users - they are expected to evaluate if the config is right for them.

In my opinion, this should be fixed/changed before 1.0.

Describe the solution you'd like

Any setting that has a recommended default that is currently written into these files should be figured out internally (like buffer_editor). A setting should not need to be in a configuration file if it follows the recommended default.

On startup, config.nu and env.nu should never be created automatically, and nu should not complain or warn if they're missing.

Describe alternatives you've considered

For developer convenience, an alternative is to process a built-in startup file before the user's startup files. However, it is faster and better to do it in Rust code.

Additional context and details

No response

@PerBothner PerBothner added the enhancement New feature or request label Oct 17, 2022
@PerBothner
Copy link
Contributor Author

Somewhat tricky is how to add logic for handling default configuration settings - without changing the Config type, and all the places that make use of Config fields. A suggestion (which I can try implementing if it makes sense) is to inline the into_config function into its single call-site in merge_env. Then merge_env can contain logic to check if a field (such as buffer_edtor or use_domterm_features) has been explicitly set; if not, it can set a default value based environent variable or other parts of the EngineState. For example we can add a private field buffer_editor_set: bool if buffer_editor has been explicitly set; if not we figure out a default value.

@PerBothner
Copy link
Contributor Author

Just now I updated nu from git and got:

$ target/debug/nu
$config.animate_prompt is an unknown config setting
$config.disable_table_indexes is an unknown config setting

I never added these config settings, and now I get warnings about them. It is easy enough to fix - but it's a bug that I would have to.

@fdncred
Copy link
Collaborator

fdncred commented Oct 28, 2022

I believe this means that these things are in your config file. You must not have updated for a bit because animate_prompt is very old and disable_table_indexes name and usage was changed. If you have invalid things in your config, i think nushell should warn you about it.

For the most part, we decided that the config.nu should contain the default settings as if when you don't have them, those settings that are in there now take effect. I'm not sure if that's the case for all of them right now, but originally, that was our intent. One exception to this is when git completions were added. It also serves as some level of documentation.

@PerBothner
Copy link
Contributor Author

"I believe this means that these things are in your config file."
Well, yes, but I never put them there: they are not my configuration preferences. They are (or rather were) Nu's default or recommended preferences. They do not belong in the same file as my configuration preferences.
Those two preferences I mentioned were an example (symptom) of the sort of problems you get when you try to mix system defaults and user preferences in the same file. You get these unnecessary and annoying update hassles. config reset is semi-useless because it doesn't give you the changes you made from old default: To update you need to do a 3-way merge missing one of the 3 files.
"It also serves as some level of documentation."
Documentation in the wrong place.

@zoechi
Copy link
Contributor

zoechi commented Jan 19, 2023

Looks similar to #7371

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Issue related to nu's configuration enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants