-
Notifications
You must be signed in to change notification settings - Fork 2.3k
[Discussion] Enabling bash-it components to communicate messages to users #2181
-
[Adapted from recent Gitter posts]
Q: Seeing how bash-it appears to have a fail silently mantra to prevent unwanted terminal output, is there a means for bash-it to communicate issues/messages to the user?
At first I was thinking something simple like bash-it logs and give components a means for creating log messages to communicate error conditions (ie. functions like log_error, log_warning, etc)
But then I thought, in addition to that, maybe something like bash-it messages which would contain messages that components generated specifically for the user to see (where as logs could contain many types of messaging, auto-generated statuses, etc) ... (ie. user_message function)
THEN we could also design a prompt segment for letting the user know when messages are waiting to be read.
Also, feels like we could give ourselves permission to message the user when initiating a login session. Even if it was just to say something like "You have bash-it messages waiting to be read. Use 'bash-it messages' to read them."
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 2
Replies: 13 comments
-
Good points! Here's the thing about failing silently. This was mostly based on the fact that error messages printed during Bash-it initialization were causing issues with things like scp, where the scp process would stall or fail due to the unexpected output.
If we can ensure that Bash-it is not initialized on non-interactive shell sessions (like scp or sftp), then we should be able to print some messages. I think some changes have been made (to bash_profile.template.bash to check for interactive shells - we would need to validate that this works as expected, and potentially do that check in bash_is.sh as well, so we don't rely on the user's profile.
If we can rule out that messages are printed in undesirable situations, we could add a couple of sensible messages. We certainly don't want to flood the user with dozens of messages, but a note that something is wrong might be OK.
Personally, I like the way things like Homebrew handle this. There's a brew doctor command that goes through some self checks and then shows what's wrong (and how it could be fixed). A bash-it doctor command could print the collected issues on demand.
This of course would mean that we introduce a logging system in Bash-it. When a plugin is enabled, but the required command is not installed, the plugin would need to log a message for that, which would be shown when you run bash-it doctor.
The bash-it doctor command would basically go through a full initialization of Bash-it, but instead of swallowing the log messages would print them, this could be triggered through an environment variable that is set when you run bash-it doctor.
So the following main blocks of work:
- Provide an internal log function that by default doesn't print anything.
- Provide a
bash-it doctorcommand that loads the Bash-it config and prints log messages while going through the initalization. - Adjust all plugins to use the log function. As an alternative, we could embed the logging into the
_command_existsfunction, which is supposed to be used in plugins anyway.
Thoughts?
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
+1 for bash-it doctor great idea. I think this would be super helpful, especially for plugin authors / contributors.
Beta Was this translation helpful? Give feedback.
All reactions
-
FYI the fix for non-interactive bash sessions, e.g. SCP, was this one: #1325
Beta Was this translation helpful? Give feedback.
All reactions
-
My 0ドル.02 (only worth 0ドル.01): I agree with @nwinkler. If we can ensure initialization is covered, it'd be great not to have silent failures. #1533 is a great first step (plus it fixes macos users' double initializing).
Beta Was this translation helpful? Give feedback.
All reactions
-
@cornfeedhobo, @davidpfarrell, @rico-chet, @jpmcb - please see the implementation of the bash-it doctor command that @NoahGorny has done in #1623 - this is a great start.
Since there are no log statements so far, the command does not print anything at the moment.
As a start, I thought that we might want to add some output to the _command_exists function, since that should be used consistently for checking for dependencies, e.g. when initializing a plugin. I thought about adding a WARN message to that. Ideally, we find a way to also print the name of the respective plugin when calling the _command_exists function - either from the callstack, or through an additional (optional) parameter.
Other thoughts about places where logging makes sense?
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1 -
👀 1
-
@nwinkler Thanks for the update - #1623 does look like a good start.
Some immediate thoughts:
Log Context
Ideally, we find a way to also print the name of the respective plugin
I'm thinking that the core bash-it code maintain a 'logging context' which will effectively act as a log message prefix at the time the message is logged.
I see it as an array of strings, ie ( 'plugins' 'git' ), can be pushed/popped by the core code as it transitions through its actions.
The logging function could then turn this into a prefix for the log message, ie plugins:git:DEBUG: message
Aborted Plugins
I think one of the first areas to add logging would be to plugins, adding error messages when plugins abort without activating (due to missing commands, folders, etc).
Logging in _command_exists
I think adding logging here is probably a good idea, especially in the short term where little/no other logging will exist.
I thought about adding a WARN message to that
Wondering if DEBUG might be more apropos, since the function itself doesn't know how the caller will treat a failure. It will also be noisy. I guess if we're concerned about it being the only logging in the short term, a case can be made for initially using WARN with a plan to shift to DEBUG over time.
That's all for now - Looking forward to seeing how this progresses !
-DF
cc: @NoahGorny
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
@davidpfarrell Thanks for the summary - that looks good to me!
- Log Context: Indeed, the Bash-it loader code could populate that, e.g. which plugin is being loaded at the moment.
_command_exists:DEBUGshould be fine as well - we'll have to play with the noisiness (?) to tune it to the right amount. The currentbash-it doctorimplementation shows all messages (includingDEBUG) by default.
Beta Was this translation helpful? Give feedback.
All reactions
-
If adding logging to _command_exists, we might want to include the caller.
Beta Was this translation helpful? Give feedback.
All reactions
-
Since the logging has been added to _command_existsin #1628, this would be a new feature. FWIW, the logs include the concept of a BASH_IT_LOG_PREFIX that can be set to indicate who's creating the log entries at the moment. The bash_it.sh script uses that prefix to indicate e.g. which plugin is currently being loaded - that's almost like the caller.
A more advanced version with a callstack has been discussed above and in #1628, but was not implemented at the time. If someone wants to build a more advanced version, that would be cool.
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
This issue is very stale, so I close it
Beta Was this translation helpful? Give feedback.
All reactions
-
I'd like to reopen this. An easy first step would be to just change the default from "fail silently" to "report errors and warnings" (set $BASH_IT_LOG_LEVEL=2), and cleanup any messages that aren't appropriate at their current levels. Silent errors are...not preferred... 😉
Beta Was this translation helpful? Give feedback.
All reactions
-
I am reopening- to leave this as a follow-up mission to enhance our logging information.
However- logging on every warning/error can be quite annoying IMO, maybe only on errors, and this is also debatable
Beta Was this translation helpful? Give feedback.
All reactions
-
❤️ 1
-
logging on every warning/error can be quite annoying IMO, maybe only on errors, and this is also debatable
My mental model of logging levels (some of which Bash It doesn't have):
0. Fatal means we done forked up and better to just stop now; let the grownups clean up later.
- Errors mean something is broken and we should not continue the current action.
- Warnings mean something is unexpected in a way that might mean something is broken, but not necessarily, so handle it and keep going.
- Notice means ń̥o҉̙̻̖̲̮͓̩̀t̵̬̳͔ḩ̡̪͇͈i̼̦̮̙͝ng is wrong but the user should be told something.
- Info means normal things going on, only nerds would care.
- Debug is for developer tracing, like we're trying to find which line sets a variable.
I strongly feel that Bash It should run without errors unless something is broken. (There's still a lot of work to do before we can make this claim, but it's a reasonable near-term goal!)
If something is broken, Bash It should complain (loudly, IMO)!
Warnings I think should alsö be reported to the user, but again I alsö think that Bash It should run without warnings. This may be more of a medium-term goal.
Notice seems like what commands should use to tell the user things, like in plugins/proxy notifying that proxies are now enabled.
I'm working towards normal operation without errors or warnings, so hopefully that will come soon. 😃
Beta Was this translation helpful? Give feedback.