Nuklear
Nuklear copied to clipboard
const correctness
I'm wrapping this library into C++ (mostly for ease of use, reduced verbosity and increased code productivity; might release the code at some point) and I have noticed that const is rarely used.
Now, I know this is an immediate mode library and thus any widget-producing function can not have const nk_context* but many getter-only functions such as xxx_get_scroll for sure can take const context.
There are also minor issues in some specific functions, e.g.:
- https://github.com/Immediate-Mode-UI/Nuklear/issues/700
nk_combo,nk_comboboxand all related functions should takeconst char *const *, notconst char **as neither the strings nor the array is modified
I know very little internals of the library, but this issue is purely on the abstraction level so I should be able to make a PR. This would not be a breaking change. Any thoughts?
I have noticed that a recent commit 0a28e3002be24dad4aa9263817fb96f3ad94a9b5 is basically a revert of older commit 2e4db87ed124f3c4eb098f7e501a2fc0aba4c974. I think this is a bad change. I know that standard functions use char** but it does not make sense to return a non-const pointer to a const string.
struct nk_font* nk_font_atlas_add_from_memory(struct nk_font_atlas *atlas, void *memory, nk_size size, float height, const struct nk_font_config *config); takes a non-const pointer to the memory but looking at the implementation this memory is never modified (actually copied).
Same for nk_font_atlas_add_compressed.
Is this still occuring?
I'm not sure what you mean.
I listed few additional cases that I did not modify for simplicity of my PR - these obviously weren't adressed (unless someone else modified them in the meantime).
My PR was merged (https://github.com/Immediate-Mode-UI/Nuklear/pull/721) so normally I would close this issue but now I see that your own PR (https://github.com/Immediate-Mode-UI/Nuklear/pull/728) reverted my changes. The PR has barebone commit message and there is no comment anywhere why.
Can you tell me what's going on?
Thanks for catching that. Perhaps a merge conflict occured. I'll have another look.
Looks like there was a merge conflict, and I had brought the const attributes back in via a rebase over at https://github.com/Immediate-Mode-UI/Nuklear/commit/a315d25b4c33efa0e112cab9562460e785935a10 . Thanks for having a closer look! :+1:
Good news. Thanks.