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.

Sign up for GitHub

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

Jump to bottom

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

Closed
theory opened this issue Nov 5, 2021 · 4 comments
Closed

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

theory opened this issue Nov 5, 2021 · 4 comments

Comments

@theory
Copy link
Owner

theory commented Nov 5, 2021

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
Copy link
Collaborator

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
Copy link
Owner Author

theory commented Nov 7, 2021

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 added a commit to fluca1978/pgenv that referenced this issue Nov 8, 2021
@fluca1978
Renamed config write to config init.
f3c4fec 
`config init` will create a new existing file with blank settings.
See <theory#47 (comment)>
fluca1978 added a commit to fluca1978/pgenv that referenced this issue Nov 8, 2021
@fluca1978
Split the config delete into the config nuke command.
db142d5 
`config nuke` does the same as `config delete` but allows the user to delete
the default configuration file. Now `config delete` does not allow for
removal of the default configuration file, but `config nuke` allows for
the removal of the default configuration file.

Specifying `config delete default` or `config nuke` will provide the same
behavior, that is deletion of the default configuration.

See <theory#47 (comment)>
fluca1978 added a commit to fluca1978/pgenv that referenced this issue Nov 8, 2021
@fluca1978
Documentation update.
1681ebf 
Introduction to the new behavior of the `config` subcommands.

See <theory#47 (comment)>
@fluca1978 fluca1978 mentioned this issue Nov 8, 2021
Merged
@fluca1978
Copy link
Collaborator

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

@fluca1978 fluca1978 mentioned this issue Nov 10, 2021
Closed
fluca1978 added a commit that referenced this issue Nov 15, 2021
@fluca1978
Delete default configuration (#49)
e46d35f 
* Append timestamp information to configuration backup file.

Use `date` to place a timestamp like string to configuration backup files,
created when overwriting an existing configuration file.

* Renamed `config write` to `config init`.

`config init` will create a new existing file with blank settings.
See <#47 (comment)>

* Remove backup files when running `config delete`.

Since now the configuration files include the timestamp string,
the `config delete` command must delete everything that ends in `.backup`.

* Split the `config delete` into the `config nuke` command.

`config nuke` does the same as `config delete` but allows the user to delete
the default configuration file. Now `config delete` does not allow for
removal of the default configuration file, but `config nuke` allows for
the removal of the default configuration file.

Specifying `config delete default` or `config nuke` will provide the same
behavior, that is deletion of the default configuration.

See <#47 (comment)>

* Refactoring of `config write`.

Now the command loads the current default configuration and
dumps it as the specified file.

* Documentation update.

Introduction to the new behavior of the `config` subcommands.

See <#47 (comment)>

* Avoid re-initializing a configuration file if that exists.

Check for the existance of the configuration file when `config init` is run
and avoid re-doing initialization then.

* Remove `config nuke` and leave `config delete default`.

The latter is enough explicit to warn the user to do the deletion.

* Documentation fix.

Remove the `config nuke` references.
Explain the workflow of `config init`.

* Modified warning message when re-init configuration file.

Placed also a debug message to explain which version was in use to double
check the program has "guessed" the right file.

* Fix warning message for `config delete`.

When trying to delete the default configuration by not specifying the
version, the warning message provides more details.
Also removed a reference to `nuke`.

* Update `available` example output

Fix documentation.

Also, example with a serious editor ;)

* Remove the shell '$' prompt from examples.

Started by theory, removed everywhere for coherence.

* Improve warning message about `config delete`.

* Add debug message for `config delete`.

* Fix missing escape sequence.

Last debug message introduction missed an escape sequence that screwed
up all the script.

* Bump version number

* Four spaces in markdown list

* Remove extra blank lines.

* Fix shell examples in documentation.

Applied 'sh' and '$' prompt coherently across all the examples.
Fixed a few markdown lists here and there.
Removed the timestamp from the log outputs.

* Fix sentences in messages.

* Reduce redundancy in debug message.

* Fix markdown shell code blocks.

Use 'sh' only if there are commands without output, use '$' as prompt only
when there is an user input and output.
@fluca1978
Copy link
Collaborator

Closed by #49

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@theory @fluca1978