Skip to content

Misc minor cleanups#356

Merged
texus merged 4 commits intotexus:1.xfrom
jjuhl:misc-minor-cleanups
May 2, 2026
Merged

Misc minor cleanups#356
texus merged 4 commits intotexus:1.xfrom
jjuhl:misc-minor-cleanups

Conversation

@jjuhl
Copy link
Copy Markdown
Contributor

@jjuhl jjuhl commented May 1, 2026

No description provided.

@texus
Copy link
Copy Markdown
Owner

texus commented May 1, 2026

I don't really like the "Simplify tgui::utf::encodeCharUtf8 slightly" change. I generally prefer to have the code in the else branch, unless the else branch is the one that is usually taken.
If most of the time you needed 4 bytes then the change would make sense to me, but since it can randomly be 2, 3 or 4, I feel like the variables shouldn't be initialized to a value that is often going to be overwritten a moment later.

@jjuhl
Copy link
Copy Markdown
Contributor Author

jjuhl commented May 1, 2026

Fair enough. I don't necessarily agree 100%, but I can respect your point of view, I'll drop that commit.

@jjuhl jjuhl force-pushed the misc-minor-cleanups branch from 740447a to 306e8d6 Compare May 1, 2026 12:02
jjuhl added 3 commits May 1, 2026 14:04
  firstMimeMatch.contains(U"exec")
is much more readable than
  firstMimeMatch.find(U"exec") != String::npos
@jjuhl jjuhl force-pushed the misc-minor-cleanups branch from 306e8d6 to 8eed82c Compare May 1, 2026 12:04
@jjuhl
Copy link
Copy Markdown
Contributor Author

jjuhl commented May 1, 2026

Hmm, Your CI seems to have issues. All I did was drop the commit in question and reword another one.

@texus texus merged commit 0f73e95 into texus:1.x May 2, 2026
26 of 30 checks 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.

2 participants