Skip to content

Typos and minor issues#926

Merged
RobLoach merged 3 commits intoImmediate-Mode-UI:masterfrom
rswinkle:typos_and_minor_issues
Apr 17, 2026
Merged

Typos and minor issues#926
RobLoach merged 3 commits intoImmediate-Mode-UI:masterfrom
rswinkle:typos_and_minor_issues

Conversation

@rswinkle
Copy link
Copy Markdown
Contributor

I broke this up into 3 separate commits

vertexes -> vertices

Remove comment about non-existent function nk_window_focus(). I can't find any evidence it ever existed, even in the original repo. All I could find was this which doesn't really make sense since even searching the original repo all I could find is the comment. Very strange. Anyone know what's going on here? Is there a programmatic window focus function by another name?

NK_EDIT_COMMITED -> NK_EDIT_COMMITTED
Every time I haven't used an edit field in a while I forget that it's spelled wrong. I'm fine to appease @sleeptightAnsiC and just do this for now instead:

enum nk_edit_events {
    NK_EDIT_ACTIVE      = NK_FLAG(0), /**!< edit widget is currently being modified */
    NK_EDIT_INACTIVE    = NK_FLAG(1), /**!< edit widget is not active and is not being modified */
    NK_EDIT_ACTIVATED   = NK_FLAG(2), /**!< edit widget went from state inactive to state active */
    NK_EDIT_DEACTIVATED = NK_FLAG(3), /**!< edit widget went from state active to state inactive */
    NK_EDIT_COMMITED   = NK_FLAG(4)  /**!< old deprecated name for NK_EDIT_COMMITTED */
    NK_EDIT_COMMITTED   = NK_FLAG(4)  /**!< edit widget has received an enter and lost focus */
};

but honestly... everyone just needs to accept that breaking changes and major version bumps are both inevitable and even a good thing, not something to be avoided at all costs. Otherwise stuff like this just spreads over time.

@RobLoach, sorry I meant to get both this and my other mini one in over the weekend, and then yesterday but my internet was acting up (or maybe just Github? hard to tell). No hurry.

Comment thread demo/common/overview.c
@sleeptightAnsiC
Copy link
Copy Markdown
Contributor

sleeptightAnsiC commented Mar 24, 2026

Remove comment about non-existent function nk_window_focus(). I can't find any evidence it ever existed, even in the original repo.

This function does exist. The name was simply misspelled in the doc. It should be: nk_window_set_focus

I'm fine to appease sleeptightAnsiC and just do this for now instead [...]

Just add something like

/* for backwards compatibility */
#define NK_EDIT_COMMITED NK_EDIT_COMMITTED

right below nk_edit_events definitions, and no one will have any problems with this.

EDIT: Rob posted his comment like 5sec before mine with the same suggestion.

[...] but honestly... everyone just needs to accept that breaking changes and major version bumps are both inevitable and even a good thing, not something to be avoided at all costs.

Trying to snuck breakage under "fixed typo" is pointless. This is easily avoidable.

Comment thread src/nuklear.h Outdated
Comment on lines +1179 to +1181
*
* To change window position inside the stack occurs either automatically by
* user input by being clicked on or programmatically by calling `nk_window_focus`.
* Changing the window position inside the stack occurs automatically via
* user input, ie by being clicked.

This comment was marked as resolved.

@rswinkle
Copy link
Copy Markdown
Contributor Author

Remove comment about non-existent function nk_window_focus(). I can't find any evidence it ever existed, even in the original repo.

This function does exist. The name was simply misspelled in the doc. It should be: nk_window_set_focus

I swear I search for and didn't find that function years ago when I first noticed the comment. This is what I get for making notes about things with the intention to do something about them eventually, and then when I finally do something, I don't do as much due diligence as I should sometimes because surely I did it earlier. Mea culpa.

I'm fine to appease sleeptightAnsiC and just do this for now instead [...]

Just add something like

/* for backwards compatibility */
#define NK_EDIT_COMMITED NK_EDIT_COMMITTED

right below nk_edit_events definitions, and no one will have any problems with this.

EDIT: Rob posted his comment like 5sec before mine with the same suggestion.

[...] but honestly... everyone just needs to accept that breaking changes and major version bumps are both inevitable and even a good thing, not something to be avoided at all costs.

Trying to snuck breakage under "fixed typo" is pointless. This is easily avoidable.

I wasn't trying to sneak it in otherwise I wouldn't have explicitly called your attention to it. And my comment was not about this issue alone as I said, and it wouldn't matter what my commit was about, your opinion would be the same regarding any breaking change. Am I wrong?

It's a chronic problem that adds up. Sure an extra define/enum here is harmless, especially relative to most of these "do we break things or not" debate. However, we're just making our lives harder because when we finally do start making breaking changes we'll never remember all the things we wanted to break but were just saving to bundle together, and we'll forget any hacks/workarounds we make and any sacrifices in functionality we made on the altar of, "well we can't do that, that would break things."

@rswinkle rswinkle force-pushed the typos_and_minor_issues branch from 0e748e6 to 88380b4 Compare March 24, 2026 21:30
@rswinkle
Copy link
Copy Markdown
Contributor Author

On the plus side, at least I'm getting more practice with interactive rebasing and editing commits.

@RobLoach
Copy link
Copy Markdown
Contributor

Don't worry about rebasing things if you're concerned about conflicts or anything. Github has a squash and merge button if you're worried about the git commits being hairy.
Screenshot from 2026-03-24 19-59-05

@sleeptightAnsiC
Copy link
Copy Markdown
Contributor

@RobLoach No need to squash it here. The separation in this PR is quite good.

You usually want to keep unrelated stuff in different commits when patching something important. Otherwise, doing things like git bisect or git blame becomes problematic.

@sleeptightAnsiC
Copy link
Copy Markdown
Contributor

sleeptightAnsiC commented Mar 25, 2026

remove comment about non-existent function

This function does exist. The name was simply misspelled in the doc.

I swear I search for and didn't find that function [...] Mea culpa.

No worries.

it wouldn't matter what my commit [comment?] was about, your opinion would be the same regarding any breaking change. Am I wrong?

I agreed for a change here, and under #912, and those are still technically "breaking". I only care about old C code to remain compilable. Don't force people to patch their code and I won't complain.

I'm not entirely sure, why are you trying to start discussion about "breaking changes" here, under PR about typos. This is probably the worst place to discuss such things. This time, even Rob was faster than me with requesting the old symbol to remain. Fixing the typo is the weakest argument for breaking anything.

Keeping the old symbol by introducing a macro is a good enough compromise. I get my fake compatibility and you get your pedantic fix. If it was up to me, I wouldn't allow even that, but I'm not in charge here. If someone gets affected by this change anyway, well, it's not in my interest to waste my free time arguing about it.

Copy link
Copy Markdown
Contributor

@sleeptightAnsiC sleeptightAnsiC left a comment

Choose a reason for hiding this comment

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

Thanks.

@rswinkle
Copy link
Copy Markdown
Contributor Author

I agreed for a change here, and under #912, and those are still technically "breaking". I only care about old C code to remain compilable. Don't force people to patch their code and I won't complain.

You agreed to a non-breaking change, same as #912 where we explicitly discussed how it was non-breaking according to the definition you give right here: the developer can update the library and not be forced to change anything. And for nth time, updating the library does not force people to patch their code. The library does not magically update itself in there repos. This isn't libc. Heck, even with dynamic libraries, between flatpaks, appimages, snaps, and plain old-fashioned tarball packages, no one is forced to update anything under almost any circumstances these days.

I'm not entirely sure, why are you trying to start discussion about "breaking changes" here, under PR about typos. This is probably the worst place to discuss such things. This time, even Rob was faster than me with requesting the old symbol to remain. Fixing the typo is the weakest argument for breaking anything.

I'm trying to wear you down. If I can't convince with logic and common sense it's my only option. Looks like I have another opportunity in another PR today. I'm only half joking. And actually I was faster than Rob, considering I listed an equivalent change to keep it backward compatible in my very first message. The typo is not the argument. The argument is the same regardless of the message carrier. Major updates are normal and semver exists for a reason. Your personal preference to both always use the latest version of Nuklear and never have to make any changes to do so doesn't change that.

If it was up to me, I wouldn't allow even that, but I'm not in charge here.

Exactly. Not only would you never agree to any breaking change, but if you were in charge you would resist even non-breaking changes that could've been breaking without a concession. That points to something almost pathological imo.

I am posting this at the same time I comment on the other PR. They sort of go together.

@RobLoach RobLoach merged commit 76d7e8e into Immediate-Mode-UI:master Apr 17, 2026
1 check passed
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