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

Add a trivial callback sink #2610

Merged
merged 4 commits into from
Jan 19, 2023
Merged

Add a trivial callback sink #2610

merged 4 commits into from
Jan 19, 2023

Conversation

maghorbani
Copy link
Contributor

I needed such a functionality, to receive logs in a callback function, so I added this sink to the library.
The new sink, will call a custom callback, each time something is logged.
The code was initially cloned from basic_file_sink.h.
Required test, example and readme sample, added.

it will call a custom callback, each time something is logged.
the code was initially cloned from `basic_file_sink.h`
required test, example and readme sample, added
@gabime
Copy link
Owner

gabime commented Jan 17, 2023

Thanks @maghorbani . I am not sure. Whats exactly the use case here? This can be achieved already by implementing custom sink just as easily.

@maghorbani
Copy link
Contributor Author

maghorbani commented Jan 17, 2023

Well I needed it to be like this, so my custom sink would not be an additional patch to spdlog in every single project of mine. I just thought that, it is not harmful to have a general purpose sink.
Anyway merging it or not, that's your decision, but I will keep my fork sync with the main repo and use it in my projects that use spdlog. I saw an issue or two, asking for the same kind of sink.

@gabime
Copy link
Owner

gabime commented Jan 17, 2023

If there a popular demand for such think, I don't mind adding this. Which issues mention this?
As a side note, I would make it simpler for use by getting rid of the struct and just providing 2 constructor overloads that accept the lambdas.
Also note that the conversion to string in the second lambda is not cheap since string allocates. Better to reuse a private member string.

@maghorbani
Copy link
Contributor Author

maghorbani commented Jan 17, 2023

here are some that I found, searching callback keyword in issues:
2469, 1328, 1016
or 863, they did not know how to implement a new sink, but such a callback sink would meet their need
and an old one (i just saw it) 205, they implemented such a callback, but it was not general enough to be merged.

I'm not sure if it worth it to be merged, if so, please let me know, and I will improve the new sink to meet your reasonable advice:

As a side note, I would make it simpler for use by getting rid of the struct and just providing 2 constructor overloads that accept the lambdas.
Also note that the conversion to string in the second lambda is not cheap since string allocates. Better to reuse a private member string.

@gabime
Copy link
Owner

gabime commented Jan 17, 2023

I'm not sure if it worth it to be merged

Lets start with a very basic, single lambda callback (the one with the the log_msg arg).
This I think should cover most of the cases.
If more functionality needed, it can be added later.

Thanks!

@gabime gabime merged commit 3cab260 into gabime:v1.x Jan 19, 2023
@gabime
Copy link
Owner

gabime commented Jan 19, 2023

Thanks @maghorbani . Merged.

ravimohan1991 added a commit to ravimohan1991/spdlog that referenced this pull request Feb 11, 2023
Minor extension of gabime#2610

Need to if serves the purpose
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