Skip to content

Conversation

@madkins23
Copy link

I honestly don't expect this PR to be accepted. I'm pretty sure I'm the only person who thinks this is useful. Plus it's not thread safe. Fortunately for me, I have my own fork to play with so I can cope with the rejection. 😞

A description of what this is supposed to do (with an example) is in the adjusted README.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 22.22222% with 21 lines in your changes missing coverage. Please review.

Project coverage is 94.28%. Comparing base (d4a3eec) to head (cf3525a).

Files with missing lines Patch % Lines
handler.go 22.22% 21 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #19      +/-   ##
==========================================
- Coverage   98.83%   94.28%   -4.55%     
==========================================
  Files           5        5              
  Lines         429      455      +26     
==========================================
+ Hits          424      429       +5     
- Misses          4       25      +21     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@madkins23
Copy link
Author

I've been thinking about this and I could probably "fix" the thread safety issue by:

  • configuring this behavior as part of console.HandlerOptions (where it belongs),
  • aquiring the depth argument from a key/value pairs, and
  • getting rid of the increment/decrement functions.

This is actually what I originally intended to do, but there's no way to access the key/value pairs except by looping through them. This didn't immediately seem like a good idea. That's when I created the new interface and so forth.

Providing the depth argument in the key/value pairs would sort of kick the thread safety issue back to the calling code at the cost of iterating through the pairs twice (though only for handlers that set indentation in the options).

I may create another branch and try this out. Later. Maybe much later. 😉

@madkins23 madkins23 mentioned this pull request Feb 15, 2025
@madkins23
Copy link
Author

The alternate version desccribed above is in #20.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants