Skip to content

Conversation

firewave
Copy link
Collaborator

No description provided.

Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm these functions are for usage in the debugger. will I be able to call these during debugging? I guess std::cout symbol is not available while I debug inside the simplecpp.

We don't need to have these functions in release builds? would that solve your itch?

@firewave
Copy link
Collaborator Author

Not directly specifying any stream is good style as you do not want some "random" print something. It would also allow to use it in more contexts.

Not sure about calling it directly from the debugger since I rarely do that. I thought about adding a default parameter but I did not want to have the <iostream> include in the header.

Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can still call it directly in the debugger:

(gdb) call rawtok->printAll(std::cout)

so well this complicates debugging but it works.

Copy link
Contributor

@jcfr jcfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integrating the proposed API while maintaining the possibility to just call the function seems like a reasonable trade-off to finalize this.

void printAll() const;
void printOut() const;
void printAll(std::ostream& ostr) const;
void printOut(std::ostream& ostr) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void printOut(std::ostream& ostr) const;
void printOut(std::ostream& ostr) const;
void printAll() const {
this->printAll(std::cout);
}
void printOut() const {
this->printOut(std::cout);
}

@@ -235,7 +235,7 @@ namespace simplecpp {
}
void push_back(Token *tok);

void dump() const;
void dump(std::ostream& ostr) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void dump(std::ostream& ostr) const;
void dump(std::ostream& ostr) const;
void dump() const {
this->dump(std::cout);
}

@firewave
Copy link
Collaborator Author

Integrating the proposed API while maintaining the possibility to just call the function seems like a reasonable trade-off to finalize this.

Like I said that would introduce the <iostream> header which it is one of the heavier headers that and as it is just debug code that seems unnecessary to spill into user code.

Also we could use default parameters which would require less code.

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