Skip to content

Add Documentation with Sphinx and push to Github Pages - #121

Open
3y3p4tch wants to merge 3 commits into
hyprwm:mainfrom
3y3p4tch:documentation
Open

3y3p4tch wants to merge 3 commits into
hyprwm:mainfrom
3y3p4tch:documentation

Conversation

@3y3p4tch

@3y3p4tch 3y3p4tch commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

For now, only Signal module is documented. Further modules will be documented in future PRs

I've tried to follow existing conventions for github actions. For documentation, I referenced usage in hyprland to understand this well enough, but I wouldn't fully trust my documentation since I am new to the project.

A review from @gulafaran would be nice.

Side Note: Claude was used to generate this documentation and related files. However, I have gone through the generated files myself, have reviewed it and I fully understand the contents being added in this PR. I am a bit unsure on how github pages really work (haven't done this in a long while), so please check that carefully.

Documentation can be checked manually on local - instructions added in README.

For now, only Signal module is documented. Further modules will be
documented in future PRs.
@3y3p4tch 3y3p4tch mentioned this pull request Aug 2, 2026
id-token: write
environment:
name: github-pages
url: ${{ steps.deployment.outputs.page_url }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

where is this gonna push? What's the url?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

(hyprwm.github.io btw. Feel free to suggest changes)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

that was my question, the domain. Taking up hyprwm.github.io is kinda yikes. Can't we do /hyprutils?

Comment thread docs/signal/index.md

// subscribe; keep the returned handle alive for as long as you want events
CHyprSignalListener onChange = valueChanged.listen([](int value) {
printf("value is now %d\n", value);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cmon printf in 2026? std::print xd

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's fine, who cares. It's a documentation on signals, not on the right way to print in 2026.

@3y3p4tch

3y3p4tch commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@vaxerski This documentation is my best guess on how things work. You surely know it much better than I do. Please confirm that it covers any/all edge cases, is exhaustive and correct.

@vaxerski

vaxerski commented Aug 4, 2026

Copy link
Copy Markdown
Member

You surely know it much better than I do

brother most of those edge cases were:

  • caught by me ages ago and I forgot about them
  • fixed by fox

your guess is as good as mine atp, lmao

@vaxerski

vaxerski commented Aug 4, 2026

Copy link
Copy Markdown
Member

@codex review please

maybe mr bot will be able to catch some inconsistencies if there are any

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 759bdd65e9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/signal/index.md
Comment on lines +15 to +17
parties subscribe to it. Emitting calls every registered handler inline, in
registration order, and returns once they have all run. There is no queue, no
dispatch thread, and no allocation per emission.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove the no-allocation emission guarantee

This guarantee is inaccurate for any signal with live listeners: CSignalBase::emitInternal copies the static-listener vector and, for handle-owned listeners, constructs a temporary std::vector and calls reserve(m_vListeners.size()), so emitting can allocate on every call. Keeping this statement in the new public docs misleads performance-sensitive users into relying on an allocation-free emission path that the implementation does not provide.

Useful? React with 👍 / 👎.

Comment thread docs/signal/signals.md
Comment on lines +50 to +52
The practical consequence is that `CSignalT<SButtonEvent>` already does the right
thing: declare the plain type and handlers get `const SButtonEvent&` with no
copies. Write `const&` in the signal's parameter list only when you want it to be

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Qualify the no-copy payload guidance

This says CSignalT<SButtonEvent> handlers get const SButtonEvent& with no copies, but RefArg<T> maps every std::is_trivially_copyable_v<T> type to T by value. For trivially copyable event structs like the SButtonEvent shown earlier, users will actually get a copy unless they declare the signal argument as const SButtonEvent&, so the current guidance can lead to unexpected copies for larger plain structs.

Useful? React with 👍 / 👎.

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