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.

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

Group versus Indent? #13

Closed
kitsonk opened this issue Aug 5, 2022 · 4 comments
Closed

Group versus Indent? #13

kitsonk opened this issue Aug 5, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@kitsonk
Copy link

kitsonk commented Aug 5, 2022

I have found the .logIndent() API to be odd/confusing. Having to wrap everything in some sort of async handler makes it very difficult to write straight forward imperative code. I personally like the console.group() model, where there is a set state which gets indented and outdented in the order of execution.

@dsherret
Copy link
Owner

dsherret commented Aug 5, 2022

I didn't even think about console.group here and I think changing the code to use that instead and rename to logGroup would be beneficial. What I was trying to avoid though is people doing stuff like this:

$.logGroup("Some title");
await $`some_command`;
$.logGroupEnd();

When they really should be writing:

$.logGroup("Some title");
try {
  await $`some_command`;
} finally {
  $.logGroupEnd();
}

So, the provided function (which can be synchronous or asynchronous), forces the try/finally pattern under the hood which is why I prefer it.

Maybe the api should allow people to choose which one they want to do especially where errors being thrown means the process exists, but I kind of like the idea of forcing the try/finally pattern under the hood. I don't have strong feelings though and maybe someone forgetting to do try/finally isn't such a big deal. Thoughts?

@dsherret dsherret added the enhancement New feature or request label Aug 5, 2022
@dsherret
Copy link
Owner

dsherret commented Aug 5, 2022

Yeah, I think I'll implement both apis after thinking about it, but recommend using the passed in function in API code rather than some top level script.

@kitsonk
Copy link
Author

kitsonk commented Aug 5, 2022

options are great I think... You raise a good point about the potentially needing to do finally, but so far, the way I have handled errors, it wouldn't be a requirement for me... either they get caught and logged, or they terminate the script and the indentation would be outwith that block.

Not to over complicate things, but if you go with the stated version as well, it would be good to be able to determine the current depth.

@dsherret
Copy link
Owner

Working on upgrading some of our scripts today, so implemented this. You can use $.logDepth to get or set the current indent depth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants