Skip to content

core: add a UUID to every core surface#8200

Closed
jcollie wants to merge 1 commit intoghostty-org:mainfrom
jcollie:core-uuid
Closed

core: add a UUID to every core surface#8200
jcollie wants to merge 1 commit intoghostty-org:mainfrom
jcollie:core-uuid

Conversation

@jcollie
Copy link
Copy Markdown
Member

@jcollie jcollie commented Aug 10, 2025

  • Adds a 128bit UUID to every core surface.
  • Expose that UUID as an environment variable.
  • Add a function to the core app to search for surfaces by UUID.

@jcollie jcollie requested a review from a team as a code owner August 10, 2025 14:56
Copy link
Copy Markdown
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

Just a change requested on the dependency.

Other than that, I'm fine with this as it is. I think we'll take a much harder look at surface IDs in the future when we also make libghostty aware of hierarchy (windows vs tabs vs splits) but for now this is fine.

Comment thread flatpak/zig-packages.json Outdated
Comment thread src/Surface.zig Outdated
Copy link
Copy Markdown
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

I don't want to vendor the UUID. I just want to make our own UUID type that copy and pastes what we need. I can take this over.

@jcollie
Copy link
Copy Markdown
Member Author

jcollie commented Aug 13, 2025

I went ahead and implemented our own UUID type.

@pluiedev
Copy link
Copy Markdown
Member

One downside to an UUID is that you can't fit it inside a glib.Variant by itself (the type system only supports 64-bit integers at most), which means that when you want to pass the surface ID through our GTK actions, you either have to stuff it inside a string (ew, bad) or split it up into two u64s. Neither option sounds enticing, really...

@pluiedev
Copy link
Copy Markdown
Member

pluiedev commented Aug 13, 2025

Plus: u128s are also ill-defined at best in C land (just search "__int128" and the horrors therein), so every time you want to refer to a surface via libghostty you have to specify two u64s (or even worse, a string). Why not just use one single u64 in this case?

@mitchellh
Copy link
Copy Markdown
Contributor

Yeah we may be able to get away with an incrementing u64. Surface creation can grab a lock to increment and you’d have to have an unfathomable amount of surfaces to ever overflow.

I’ve been meaning to look at that as an option.

@jcollie
Copy link
Copy Markdown
Member Author

jcollie commented Aug 13, 2025

Yeah we may be able to get away with an incrementing u64. Surface creation can grab a lock to increment and you’d have to have an unfathomable amount of surfaces to ever overflow.

Incrementing a u64 would def be my least preferred option. So many network protocols have had security issues due to easily predictable identifiers that I just automatically reject that as an option. Plus the need for a mutex.

If we aren't going for "real" UUIDs then std.crypto.random.int(u64) is the best way to go IMHO.

@mitchellh
Copy link
Copy Markdown
Contributor

Yeah sorry saying incrementing was definitely wrong. I mean something closer to Snowflake.

@jcollie
Copy link
Copy Markdown
Member Author

jcollie commented Aug 14, 2025

If UUIDs are overkill because of their size, Snowflake IDs are overkill because of their complexity. Ghostty's not really a distributed system so all of that work constructing that ID is kinda pointless.

@jcollie jcollie force-pushed the core-uuid branch 2 times, most recently from f4984dc to 74c0051 Compare August 14, 2025 00:36
@mitchellh
Copy link
Copy Markdown
Contributor

It’s roughly the same complexity as a UUID, some bits for time, some bits incremented, some bits random. Just 64 instead of 128 bits.

@jcollie
Copy link
Copy Markdown
Member Author

jcollie commented Aug 14, 2025

Switched to a plain 'ol random u64.

Comment thread src/Surface.zig Outdated
- Expose that ID as the environment variable GHOSTTY_SURFACE to
  processes running in Ghostty surfaces.
- Add a function to the core app to search for surfaces by ID.
- ID is randomly generated, it has no other meaning other than as a
  unique identifier for the surface.
@mitchellh
Copy link
Copy Markdown
Contributor

Going to wait on this until we need it.

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