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

Message verbosity #22

Open
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@fluca1978
Collaborator

fluca1978 commented Oct 21, 2018

This is related to discussion about issue #21 and the need for displaying messages at different level of verbosity.
The idea is to replace pgenv_debug with a more general pgenv_message that accepts the verbosity level as an argument (so pgenv_message 'debug' is the same as the old pgenv_debug). The message is printed only if the required level is lesser than the PGENV_VERBOSITY configuration, which defaults to print every message if not specified in the configuration (the above replaces the variable PGENV_DEBUG).
Levels are kept in an associative array.

Created a function to display messages at different levels.
There is now a `pgenv_message` functions that accepts a message and a level
(as a text string) and displays the message (on stderr) only if the current
level is the sameor greater of the specified level.

Message levels are kept in a globa array `msg_levels` used as an associative
array where the message level (e.g., 'info') is the key, and the value is a
numeric value. Each time a message is passed to `pgenv_message` the
string value is converted to a numeric value and if this is greater or equal
to the setting PGENV_VERBOSITY the message will be printed.

Advantages: it makes all the messages formatted in the same manner, and
it prints all the messages to stderr (good for redirection).

All calls to removed function `pgenv_debug` has been modified
as `pgenv_message ... 'debug'`.

Also converted the initial call to show PGENV_ROOT to a call of level 'warn'.
See issue #21

Replaced all the "interactive" occurencies of 'echo ' to 'pgenv_message'.
@theory

Seems alright other than the odd level ordering. Needs documentation, though.

bin/pgenv Outdated
msg_levels[info]=10
msg_levels[warn]=20
msg_levels[error]=25
msg_levels[always]=999

This comment has been minimized.

@theory

theory Oct 21, 2018

Owner

These levels are decreasingly verbose as the numbers go up, until always, which is the most verbose. Shouldn't it be 1 (or 0)? Or should the levels be reversed other than always?

This comment has been minimized.

@fluca1978

fluca1978 Oct 22, 2018

Collaborator

Uhm...not seeing commits 390a139 and df56220 from my branch here.
The two commits above should solve your questions.

This comment has been minimized.

@fluca1978

fluca1978 Oct 22, 2018

Collaborator

Uhm...not seeing commits 390a139 and df56220 from my branch here.
The two commits above should solve your questions.

@theory

This comment has been minimized.

Owner

theory commented Oct 24, 2018

Doesn't work on macOS Mojave, sadly:

> pgenv versions
/Users/david/pg/bin/pgenv: line 8: declare: -A: invalid option
declare: usage: declare [-afFirtx] [-p] [name[=value] ...]
> bash --version
GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin18)
Copyright (C) 2007 Free Software Foundation, Inc.

Maybe use an array and declare variables that hold the log levels?

Substitute 'msg_levels' array into a set of variables.
Improves portability, since to use an associative array in Bash
there is the need to use 'declare -A'.
See discussion <#22 (review)>.
@fluca1978

This comment has been minimized.

Collaborator

fluca1978 commented Oct 24, 2018

@theory I've replaced the array with an ugly case and a set of numeric variables, works fine here.

@xzilla

This comment has been minimized.

Contributor

xzilla commented Oct 25, 2018

ISTM most of the messages are set at "info" level, even though a number of them really seem to be warnings or errors. Is there interest in cleaning that up?

Use new levels when printing a message.
Now all errors and warning use the appropriate level.
Removed wrong 'debug' levels that caused all message to be printed
as 'info'.
See suggestion in <#22 (comment)>
@fluca1978

This comment has been minimized.

Collaborator

fluca1978 commented Oct 26, 2018

@xzilla good point! I've placed correct (or I think they are correct) levels in almost every message that deserved. Can you please help improving it (even just reviewing)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment