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

Abstract stream support in printer classes #108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

becrux
Copy link

@becrux becrux commented Apr 27, 2018

This patch allows to create new printers based on custom streams.

@becrux
Copy link
Author

becrux commented Apr 27, 2018

This is still a WIP, probably some code is redundant.

The change was originated by my need to use QTextStream class, in order to write the stacktrace in a QFile.

This should also start an implementation for the issue #26 .

This patch allows to create new printers based on custom streams.
@Manu343726
Copy link
Collaborator

Why not keeping the printer API, but changing its backend to pick the right printer under the hood?

Printer printer;
printer.print(st, stdout); // uses legacy FILE backend
printer.print(st, std::cout); //Uses new std::ostream compatible backend

@Manu343726
Copy link
Collaborator

Also, I don't see any benefit of adding a printer singleton.

@becrux
Copy link
Author

becrux commented Apr 29, 2018

I need to use with Qt QTextStream, in order to dump a stacktrace in a QFile, I cannot use console. Anyway, I assume that hardcoding a stream technology, no matter what is, does not allow this library to be adapter to any other non-STL or non-STDIO context.

Regarding QTextStream, you cannot use STL I/O manipulators, and it does not fit as it is, that's why I tried to create an interface layer, without changing too much (well, I tried).

I was able to perform it with templates, but that required a singleton in my implementation to make everything fit each other. Virtual interfaces could be another approach, but it would have requested more changes IMHO.

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.

3 participants