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

feat: Surface::toStream overload without stream argument #3335

Merged
merged 6 commits into from
Jul 1, 2024

Conversation

paulgessinger
Copy link
Member

I'm adding a way that allows you to avoid having to write

surface->toStream(gctx, std::cout);
std::cout << std::endl;

and instead write

std::cout << surface->toStream(gctx) << std::endl;

This is done by returning a temporary helper struct that implements operator<<.

I'm adding a way that allows you to avoid having to write

```cpp
surface->toStream(gctx, std::cout);
std::cout << std::endl;
```

and instead write

```cpp
std::cout << surface->toStream(gctx) << std::endl;
```

This is done by returning a temporary helper struct that implements
`operator<<`.
@paulgessinger paulgessinger added this to the v36.0.0 milestone Jun 28, 2024
@paulgessinger paulgessinger changed the title feat: Surface::toStream overload without stream feat: Surface::toStream overload without stream argument Jun 28, 2024
@github-actions github-actions bot added the Component - Core Affects the Core module label Jun 28, 2024
@andiwand
Copy link
Contributor

I think @benjaminhuth did something similar here #1327 which requires std::tieing the surface and context

@benjaminhuth
Copy link
Member

yeah, maybe this is more intuitive than std::cout << std::tie(surface, ctx) << std::endl. It will definitively give better compiler errors :D

@andiwand
Copy link
Contributor

Should we replace it then and converge on one version?

@benjaminhuth
Copy link
Member

Would be fine for me.

@benjaminhuth
Copy link
Member

Could you add this also to the doxygen description of Acts::Surface, so that people easily find this method @paulgessinger ?

@paulgessinger
Copy link
Member Author

Ha, I had forgotten all about #1327, I was playing around with a tuple argument, but I didn't actually know it would work with tie.

Up to you, I can also close this in favor of just sticking to #1327.

@paulgessinger
Copy link
Member Author

@andiwand That's a breaking change then?

@benjaminhuth
Copy link
Member

benjaminhuth commented Jun 28, 2024

@paulgessinger Really up to you... If your unawareness of std::tie is a hint that this is too non-obvious, we can use this. Otherwise we just have one PR less, which is also not too bad^^

@github-actions github-actions bot added the Component - Plugins Affects one or more Plugins label Jun 29, 2024
Copy link
Member

@benjaminhuth benjaminhuth left a comment

Choose a reason for hiding this comment

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

Let's get it in!

Copy link

sonarqubecloud bot commented Jul 1, 2024

@kodiakhq kodiakhq bot merged commit 7d825f6 into acts-project:main Jul 1, 2024
51 checks passed
@github-actions github-actions bot removed the automerge label Jul 1, 2024
@paulgessinger paulgessinger deleted the feat/surface-to-stream branch July 1, 2024 13:55
@acts-project-service acts-project-service added Breaks Athena build This PR breaks the Athena build Fails Athena tests This PR causes a failure in the Athena tests labels Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaks Athena build This PR breaks the Athena build Component - Core Affects the Core module Component - Plugins Affects one or more Plugins Fails Athena tests This PR causes a failure in the Athena tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants