Consider renaming Logger

[matthew-carroll]

Mason might have too many users already to do anything about this , but I thought I'd point this out.

The term "Logger" appears to be used within the context of hooks to interact with a user. This is a very confusing name. I don't think I've ever seen the term "logger" applied to user interaction. It would never have occurred to me to look at the Logger API to control user input, except that I saw some other source code that was using it for that purpose.

Typically, logging is the passive accumulation of information during execution. Logs might print to stdout, or maybe not. But either way, it's information gathering.

User interaction is a two-way exchange with lots of blocking in between. The goals of this user interaction is very different than that of logging, and loggers would never dream of blocking execution to wait for a response.

You might consider adjusting the associated API names, so that Mason developers know where to go for traditional logging, and where to go for a console-based user interface.

[felangel]

Hey 👋
Thanks for the feedback! Currently the mason_logger API acts as a wrapper around stdout and stdin. What do you recommend in terms of naming?

[matthew-carroll]

The biggest thing is that I'd separate the two concepts. Developers probably do want to log information. And developers definitely want to prompt the user. I'm not confident on the terminology for the prompt system because I don't spend a lot of time working with CLI apps.

One thing I'm wondering is why stdout and stdin need to be wrapped? Is Mason installing some behavior, or intercepting messages or something?

[felangel]

What I meant is mason_logger exposes APIs on top of stdout and stdin to simplify common tasks such as logging progress (with an animation and duration), prompting, writing messages, warnings, errors, etc. basically the goal was for mason_logger to be used for any interactions between cli and user so in this context it’s not really limited to being used for traditional logging. Perhaps this is something that additional docs can help with?

I’ll think about it some more but I’m not opposed to moving the progress and prompting apis to their own package and exposing them separately. We’d just need to align on naming imo 👍

[matthew-carroll]

The Stdout API includes terms like hasTerminal, terminalColumns, and terminalLines. Therefore, the term terminal wouldn't be unreasonable.

void run(HookContext context) {
  context.terminal.writeln("Hello, world!");
}

[felangel]

The Stdout API includes terms like hasTerminal, terminalColumns, and terminalLines. Therefore, the term terminal wouldn't be unreasonable.

void run(HookContext context) {
  context.terminal.writeln("Hello, world!");
}

Just to make sure I understand, you'd prefer to have two different APIs as part of HookContext:

• context.logger for traditional logging
• context.terminal for two-way interactions with the user

If the above is correct then I'd like to discuss what methods would be supported as part of each. For example, currently the logger supports traditional logging APIs like: info, warn, err, etc. which I would expect would remain untouched. The main change would be rather than interacting with the logger to prompt, the user would interact with the terminal:

void run(HookContext context) {
-  final name = context.logger.prompt("What is your name");
+  final name = context.terminal.prompt("What is your name");
}

What about when you just want to output a message to the user? In your snippet you suggested using context.terminal.writeln but you can achieve a similar result using context.logger.write or context.logger.info. I'd personally prefer not to introduce multiple ways to achieve the same result because that will likely lead to more confusion.

At the moment, even though the term logger might be confusing, the logger API is essentially your I/O so you can output and/or collect information for the user with a single set of APIs. Maybe another option here would be to rename context.logger to context.io?

void run(HookContext context) {
-  context.logger.info("Hello World");
+  context.io.info("Hello World");

-  context.logger.err("Oops something went wrong");
+  context.io.err("Oops something went wrong");

-  final name = context.logger.prompt("What is your name");
+  final name = context.io.prompt("What is your name");

-  final progress = context.logger.progress("Doing stuff");
+  final progress = context.io.progress("Doing stuff");
}

Would love to hear your thoughts and thanks again for the feedback -- it's super helpful!

[matthew-carroll]

Just to make sure I understand, you'd prefer to have two different APIs as part of HookContext:

Yes, if you feel that a logger needs to be included in the HookContext at all. It's not yet clear to me why a logger needs to be provided, given that app developers can instantiate the logger of their choice without losing any control. But if you want to include a logger in HookContext, then I think separate APIs is good idea. The logger might output to terminal, but there's no such requirement for logging in general, so combining the logger API with the terminal API is an artificial constraint.

In your snippet you suggested using context.terminal.writeln but you can achieve a similar result using context.logger.write or context.logger.info

Keep in mind, logging isn't just about the terminal. In this case, what you're doing is using your implied knowledge of the loggers implementation to recognize that these logs happen to direct to terminal. So you're using the logger as if it were a terminal.

I'd personally prefer not to introduce multiple ways to achieve the same result because that will likely lead to more confusion

I understand that goal in general. In this case, there are two different behaviors that happen to go to the same place. The user interface is confined by the terminal, and the logger is confined by the terminal, so this is one of those unusual situations where you already have two different message systems directing to the same display. I think in this case you're better off acknowledging that in the API.

Maybe another option here would be to rename context.logger to context.io

I suppose you could do that. In that case I would still cal it context.terminal instead of context.io. There's nothing about a terminal that would make any of the logger methods unapplicable. After all, everybody knows these APIs are talking to a terminal, right?

But the point remains that loggers are not required to output to a terminal. So if all you want to provide is a way for developers to print information to a terminal, and prompt users in a terminal, then having a single terminal API should be sufficient. If you actually want to provide logging capabilities, then I think you'll want to deal with the common use-cases of directing logs to files and servers.

[alestiago]

@felangel, based on the tags: Is this still an enhancement candidate? Is this still waiting for a response?

[felangel]

@felangel, based on the tags: Is this till an enhancement candidate? Is this still waiting for a response?

Imo it feels like docs can address most of the concerns raised and doing a big refactor/rename doesn’t seem worth it but open to thoughts/feedback.