theory / pgenv

PostgreSQL binary manager
MIT License
311 stars 27 forks source link

Maybe don't delete .pgenv.default.conf #47

Closed theory closed 2 years ago

theory commented 2 years ago

I'm setting up a new computer so starting from scratch. I installed 14, then pgenv removed it, and was surprised to see that it deleted .pgenv.default.conf. It probably should not, right?

Also, pgenv write does not load any config files before it writes them. Shouldn't it at least load .pgenv.default.conf first?

fluca1978 commented 2 years ago

I'm setting up a new computer so starting from scratch. I installed 14, then pgenv removed it, and was surprised to see that it deleted .pgenv.default.conf. It probably should not, right?

Uhm, I guess it is a matter of taste. The program clearly states that the default configuration file cannot be deleted if there are installed PostgreSQL versions, while it deletes silently when there are no more, that is your case. I think we could either avoid deletion of default configuration or not. The problem is that pgenv config delete will not work with default and must tells the user to manually delete the file. I have no particular opinion about this change, so let me know and I will implement it.

Also, pgenv write does not load any config files before it writes them. Shouldn't it at least load .pgenv.default.conf first?

Again, I have no particular opinion. We could add a clone subcommand to write a new configuration starting from an existing one or explicitly ask the user about what to do when config write is issued (but this will make the script less scriptable).

theory commented 2 years ago

The problem is that pgenv config delete will not work with default and must tells the user to manually delete the file.

I think that's reasonable, no? I suspect most people won't want it deleted, as they will have edited it. That at least was my case (and I kept installing and uninstalling v14.0 as I worked to figure out how to deal with multiple CFLAGS, solved in #48. Was surprised to see the file disappear (fortunately it was open in an editor and I could just save it again).

I think we should not delete it.

Again, I have no particular opinion. We could add a clone subcommand to write a new configuration starting from an existing one or explicitly ask the user about what to do when config write is issued (but this will make the script less scriptable).

Well right now config write is more like config init: it writes a new default configuration file. Frankly I'm wondering at the need for a default config file at all if it's just meant to show default configuration. ISTM we have a few use cases:

  1. Show the default config without any changes from other config files; Perhaps config init would create the file if it does not already exist.
  2. Allow the user to create and edit a configuration file that provides the default configurations for all versions, and which ought not to be deleted.
  3. Show the configuration given the loading of all relevant config files for a given context (defaults or for a specific version)

Given all that, I think renaming config write to config init would be sensible, and then create a new config write that writes out the fully-evaluated configuration for a specific version (but not the default file)

Does that make sense to you?

fluca1978 commented 2 years ago

I think I've implemented what you are looking for, can you see if this sounds good?

fluca1978 commented 2 years ago

Closed by #49