Skip to content

doc: Expose all public headers #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheEvilSkeleton
Copy link

@TheEvilSkeleton TheEvilSkeleton commented Mar 26, 2025

Admittedly, it took me a good 30 minutes to understand why I couldn't use the instance methods for Xdp.Settings, which required me to dig through libportal's source code and realize that these methods aren't part of libportal/portal.h. I believe exposing all the public headers would help developers who want to use libportal.


  • Steps to reproduce: #include <libportal/portal.h> (only) and try to call instance methods on XdpSettings
  • Expected result: can do so
  • Actual result: compiler errors because settings.h wasn't included

I also wrote a small reproducer: TheEvilSkeleton/Settings-Not-Included@8eb0714

@TheEvilSkeleton TheEvilSkeleton force-pushed the expose-all-headers-in-docs branch from 34339b6 to 13d7f2e Compare March 30, 2025 22:25
@ebassi
Copy link

ebassi commented Apr 16, 2025

It's a bit naff, to be honest. I don't understand why there isn't a single header include for libportal, just like every other C library that depends on GLib. Single header inclusion makes it possible to move type declarations around without breaking source compatibility.

@smcv
Copy link
Contributor

smcv commented Apr 16, 2025

I don't understand why there isn't a single header include for libportal, just like every other C library that depends on GLib

There is, <libportal/portal.h>, but from @TheEvilSkeleton's report it seems that it's incomplete.

If fixing this is going to require a libportal source-code change anyway, probably including all of the libportal core headers in portal.h would be a better answer to this than changing the introspection?

(but not the GTK and Qt addons, those need to stay separate!)

@smcv
Copy link
Contributor

smcv commented Apr 16, 2025

Admittedly, it took me a good 30 minutes to understand why I couldn't use the instance methods for Xdp.Settings, which required me to dig through libportal's source code and realize that these methods aren't part of libportal/portal.h. I believe exposing all the public headers would help developers who want to use libportal.

It sounds like you're implicitly reporting a bug. What is the bug? It's usually better to describe the bug than making maintainers reverse-engineer it from a proposed fix, particularly in projects that are limited by maintainer bandwidth.

Presumably something like this:

  • Steps to reproduce: #include <libportal/portal.h> (only) and try to call instance methods on XdpSettings (?)
  • Expected result: can do so
  • Actual result: compiler errors because settings.h wasn't included

@TheEvilSkeleton
Copy link
Author

It sounds like you're implicitly reporting a bug. What is the bug? It's usually better to describe the bug than making maintainers reverse-engineer it from a proposed fix, particularly in projects that are limited by maintainer bandwidth.

True, sorry

TheEvilSkeleton added a commit to TheEvilSkeleton/Settings-Not-Included that referenced this pull request Apr 16, 2025
@TheEvilSkeleton
Copy link
Author

  • Steps to reproduce: #include <libportal/portal.h> (only) and try to call instance methods on XdpSettings (?)

  • Expected result: can do so

  • Actual result: compiler errors because settings.h wasn't included

Yes, it's pretty much that. I also wrote a reproducer in this commit: TheEvilSkeleton/Settings-Not-Included@8eb0714

@smcv
Copy link
Contributor

smcv commented Apr 16, 2025

OK, thanks for the reproducer. I think that confirms my belief that a better fix for this would be to add #include <libportal/settings.h> to portal.h so that you don't have to know about the individual headers.

Are there any other public headers that are not included in portal.h? We should add those too, unless there's a reason they're inappropriate (for example the GTK and Qt headers, which have extra dependencies, and are mutually exclusive between major versions of the same library).

@TheEvilSkeleton
Copy link
Author

Are there any other public headers that are not included in portal.h? We

Not that I'm aware of, no

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