-
Notifications
You must be signed in to change notification settings - Fork 65
Add core CeedObject to handle common functionality #1917
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
base: main
Are you sure you want to change the base?
Conversation
0d04399 to
4a7536e
Compare
4a7536e to
0ba6602
Compare
|
I should probably add tests or exclude the new viewing functionality from tests? Its not super likely to ever break |
|
Thoughts - Do we also want CeedObjectDestroy ? (Easy to add) And do we want the view tabs to be added to the CeedObject? I'm thinking yes for both |
I would say yes for both |
| int CeedObjectCreate(Ceed ceed, int (*view_function)(CeedObject, FILE *), CeedObject obj) { | ||
| obj->ceed = NULL; | ||
| if (ceed) CeedCall(CeedReferenceCopy(ceed, &obj->ceed)); | ||
| obj->ViewFunction = view_function; | ||
| obj->ref_count = 1; | ||
| return CEED_ERROR_SUCCESS; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I understand why, but this is technically a different interface (taking a CeedObject instead of a CeedObject *) than other *Create methods in libCEED. Do we want to add a note that this function that obj is the address of a CeedObject_private (more accurate than the current wording)?
|
I think another addition here might be to add a |
@zatkins-dev this should simplify some of the logging logic, as we won't have to have different view logic for each Ceed*
We can now:
I'll be doing other stuff for the next few days, so feel free to tinker with this if you like. No worries if not - I'll finish it next week sometime